Summary: | DFG should hoist structure checks | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||||||
Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | barraclough, fpizlo, ggaren, gustavo, mhahnenberg, oliver, philn, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | 92728 | ||||||||||||
Bug Blocks: | 91933 | ||||||||||||
Attachments: |
|
Description
Filip Pizlo
2012-07-30 17:34:17 PDT
Created attachment 155408 [details]
work in progress
Created attachment 155443 [details]
almost works
It turns out that to do this right you need the CFA to track the last CheckStructure performed on a variable even if it was subsequently clobbered, so that OSR entry can validate whether the variable still has that structure. Otherwise the subsequent transition watchpoints would not work.
Created attachment 155447 [details]
runs benchmarks (on 64-bit)
Created attachment 155893 [details]
the patch
Comment on attachment 155893 [details] the patch Attachment 155893 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13419235 Comment on attachment 155893 [details] the patch Attachment 155893 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13411585 Comment on attachment 155893 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=155893&action=review r+, unsure about the lack of any kind of enum / name for the ballot constants. > Source/JavaScriptCore/dfg/DFGStructureCheckHoistingPhase.cpp:412 > + noticeStructureCheck(variable, set.singletonStructure()); I feel like I've seen this idiom before - I could be wrong? - maybe we want a singletonStructureOrNull, or just make singletonStructure() provide this behavior by default? > Source/JavaScriptCore/dfg/DFGVariableAccessData.h:156 > + m_votes[1] = 0; o_O 0 & 1 - are you sure? Comment on attachment 155893 [details] the patch Attachment 155893 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13421225 Landed in http://trac.webkit.org/changeset/124404 |