Summary: | GetMapBucket node should speculate on the type of its 'key' child | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||
Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | benjamin, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, oliver, sukolsak, ticaiolima, ysuzuki | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Local Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Saam Barati
2016-09-06 12:13:59 PDT
Looks like this is worth 2-3% on ES6SampleBench/Basic. Created attachment 290532 [details]
patch
Comment on attachment 290532 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=290532&action=review > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6606 > + m_out.insertNewBlocksBefore(notBitEqual); It's scary to me that you're not capturing the return value here and resetting it... > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6648 > + m_out.appendTo(bucketKeyIsInt32, loopAround); ... down here. Any reason why you don't do that? Comment on attachment 290532 [details]
patch
I think that aside from that minor thing, this code is right.
Created attachment 290537 [details]
patch for landing
Comment on attachment 290537 [details] patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=290537&action=review > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6612 > + m_out.appendTo(notBitEqual, bucketKeyIsCell); This order: notBitEqual -> bucketKeyIsCell is inconsistent with the order of allocation, above: notBitEqual -> bucketKeyNotCell -> bucketKeyIsCell I think that might confuse Output, at worst leading to a wonky block order. Comment on attachment 290537 [details] patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=290537&action=review >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6612 >> + m_out.appendTo(notBitEqual, bucketKeyIsCell); > > This order: notBitEqual -> bucketKeyIsCell > > is inconsistent with the order of allocation, above: > > notBitEqual -> bucketKeyNotCell -> bucketKeyIsCell > > I think that might confuse Output, at worst leading to a wonky block order. Ok. I'll re-order the allocations. Good catch. landed in: https://trac.webkit.org/changeset/206763 |