Bug 92696

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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

92728

Bug Blocks:

91933

Attachments:

Description	Flags
work in progress	none
almost works	none
runs benchmarks (on 64-bit)	none
the patch	barraclough: review+, webkit-ews: commit-queue-

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	Details Formatted Diff Diff
almost works (57.04 KB, patch) 2012-07-30 22:58 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
runs benchmarks (on 64-bit) (64.16 KB, patch) 2012-07-30 23:31 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (80.95 KB, patch) 2012-08-01 15:03 PDT, Filip Pizlo	barraclough: review+ webkit-ews: commit-queue-	Details Formatted Diff Diff
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.