RESOLVED FIXED Bug 92696
DFG should hoist structure checks 
https://bugs.webkit.org/show_bug.cgi?id=92696
Summary DFG should hoist structure checks
Filip Pizlo
Reported 2012-07-30 17:34:17 PDT
Patch forthcoming.
Attachments
work in progress (25.61 KB, patch)
2012-07-30 17:43 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
almost works (57.04 KB, patch)
2012-07-30 22:58 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
runs benchmarks (on 64-bit) (64.16 KB, patch)
2012-07-30 23:31 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (80.95 KB, patch)
2012-08-01 15:03 PDT, Filip Pizlo 		barraclough: review+
webkit-ews: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2012-07-30 17:43:15 PDT
Created attachment 155408 [details] work in progress
Filip Pizlo
Comment 2 2012-07-30 22:58:56 PDT
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.
Filip Pizlo
Comment 3 2012-07-30 23:31:44 PDT
Created attachment 155447 [details] runs benchmarks (on 64-bit)
Filip Pizlo
Comment 4 2012-08-01 15:03:37 PDT
Created attachment 155893 [details] the patch
Early Warning System Bot
Comment 5 2012-08-01 15:16:36 PDT
Comment on attachment 155893 [details] the patch Attachment 155893 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13419235
Early Warning System Bot
Comment 6 2012-08-01 15:21:29 PDT
Comment on attachment 155893 [details] the patch Attachment 155893 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13411585
Gavin Barraclough
Comment 7 2012-08-01 17:18:01 PDT
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?
Gyuyoung Kim
Comment 8 2012-08-01 17:21:34 PDT
Comment on attachment 155893 [details] the patch Attachment 155893 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13421225
Filip Pizlo
Comment 9 2012-08-01 21:33:41 PDT
Landed in http://trac.webkit.org/changeset/124404
Note You need to log in before you can comment on or make changes to this bug.