Bug 179912 – [DFG] Add NormalizeMapKey DFG IR

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

...

[Yusuke Suzuki]

Created attachment 327410 [details]
Patch

[EWS Watchlist]

Comment on attachment 327410 [details]
Patch

Attachment 327410 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/5321124

New failing tests:
stress/proxy-own-keys.js.dfg-eager-no-cjit-validate
stress/proxy-own-keys.js.ftl-eager-no-cjit
stress/proxy-own-keys.js.ftl-eager

[Yusuke Suzuki]

Created attachment 327417 [details]
Patch

[Saam Barati]

Comment on attachment 327417 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327417&action=review

r=me

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1115
> +        forNode(node).makeHeapTop();

Is it worth doing constant folding here based on child1's type?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:5195
> +            // Give key and bucket's key are already normalized. So if 64-bit compare fails and one is not a cell, they're definitely not equal.

"Give key and bucket's key" => "The input key and bucket's key"

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:8585
> +        m_out.appendTo(passThroughCase, continuation);
> +        ValueFromBlock fastResult = m_out.anchor(key);
> +        m_out.jump(continuation);

This doesn't need to be a block. You can make the upsilon at the top, and make everything that jumps to "passThroughCase" jump to continuation.

[Yusuke Suzuki]

Comment on attachment 327417 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327417&action=review

Thank you for your review!

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1115
>> +        forNode(node).makeHeapTop();
> 
> Is it worth doing constant folding here based on child1's type?

Sounds nice. I think large part of NormalizeMapKey will be converted to Identity in fixup phase. But mixed typed case is not handled well in fixup phase.
Introducing constant foldoing here could increase the coverage.

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:5195
>> +            // Give key and bucket's key are already normalized. So if 64-bit compare fails and one is not a cell, they're definitely not equal.
> 
> "Give key and bucket's key" => "The input key and bucket's key"

Thanks, fixed.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:8585
>> +        m_out.jump(continuation);
> 
> This doesn't need to be a block. You can make the upsilon at the top, and make everything that jumps to "passThroughCase" jump to continuation.

Sounds nice. Fixed.

[Yusuke Suzuki]

Committed r225154: <https://trac.webkit.org/changeset/225154>

[Radar WebKit Bug Importer]

<rdar://problem/35693381>