Bug 156567

Summary:

implement dynamic scope accesses in the DFG/FTL

Product:

WebKit

Reporter:

Saam Barati <saam>

Component:

JavaScriptCore

Assignee:

Saam Barati <saam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, commit-queue, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, oliver, sukolsak, ysuzuki

Priority:

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
WIP	none
WIP	none
patch	none
patch	ggaren: review-, buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-yosemite	none
patch	none
patch	none
patch	ggaren: review+
patch for landing	none
patch for landing	none
patch for landing	none

Saam Barati

Reported 2016-04-13 23:24:13 PDT

...

Attachments
WIP (24.04 KB, patch) 2016-04-13 23:39 PDT, Saam Barati	no flags	Details Formatted Diff Diff
WIP (25.92 KB, patch) 2016-04-13 23:43 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (73.95 KB, patch) 2016-04-17 15:42 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (74.06 KB, patch) 2016-04-17 19:02 PDT, Saam Barati	ggaren: review- buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews115 for mac-yosemite (931.62 KB, application/zip) 2016-04-17 20:21 PDT, Build Bot	no flags	Details
patch (74.23 KB, patch) 2016-04-18 12:30 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (73.95 KB, patch) 2016-04-18 12:32 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch (73.07 KB, patch) 2016-04-18 12:36 PDT, Saam Barati	ggaren: review+	Details Formatted Diff Diff
patch for landing (73.97 KB, patch) 2016-04-18 13:51 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch for landing (73.81 KB, patch) 2016-04-18 14:25 PDT, Saam Barati	no flags	Details Formatted Diff Diff
patch for landing (73.86 KB, patch) 2016-04-18 16:40 PDT, Saam Barati	no flags	Details Formatted Diff Diff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Saam Barati

Comment 1 2016-04-13 23:39:39 PDT

Created attachment 276374 [details] WIP this might work. I haven't tested it thoroughly. Still needs FTL support.

Saam Barati

Comment 2 2016-04-13 23:43:42 PDT

Created attachment 276375 [details] WIP

Saam Barati

Comment 3 2016-04-17 15:42:06 PDT

Created attachment 276601 [details] patch

WebKit Commit Bot

Comment 4 2016-04-17 15:45:01 PDT

Attachment 276601 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGOperations.h:154: The parameter name "scope" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7837: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:997: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:1001: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:69: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:71: One space before end of line comments [whitespace/comments] [5] Total errors found: 6 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 5 2016-04-17 19:02:42 PDT

Created attachment 276619 [details] patch fix 64bit rebase compilation

WebKit Commit Bot

Comment 6 2016-04-17 19:03:46 PDT

Attachment 276619 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGOperations.h:154: The parameter name "scope" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7837: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:997: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:1001: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:69: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:71: One space before end of line comments [whitespace/comments] [5] Total errors found: 6 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 7 2016-04-17 20:21:01 PDT

Comment on attachment 276619 [details] patch Attachment 276619 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1178305 New failing tests: js/dom/dfg-put-to-readonly-property.html

Build Bot

Comment 8 2016-04-17 20:21:07 PDT

Created attachment 276620 [details] Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5

Geoffrey Garen

Comment 9 2016-04-18 11:48:04 PDT

Comment on attachment 276619 [details] patch r- due to EWS failure

Saam Barati

Comment 10 2016-04-18 12:30:39 PDT

Created attachment 276656 [details] patch I think this patch should work now. There was a subtle bug where I flushed registers before asking a JSValueOperand for it's GPR which confused the DFG register allocator into thinking a particular register still held a particular value. I also fixed armv7 setupArgumentsWithExecState

Saam Barati

Comment 11 2016-04-18 12:32:37 PDT

Created attachment 276657 [details] patch fix a comment.

WebKit Commit Bot

Comment 12 2016-04-18 12:34:07 PDT

Attachment 276657 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGOperations.h:154: The parameter name "scope" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/parser/Parser.h:997: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/parser/Parser.h:1001: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:69: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:71: One space before end of line comments [whitespace/comments] [5] Total errors found: 5 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 13 2016-04-18 12:36:24 PDT

Created attachment 276658 [details] patch remove commented out code.

WebKit Commit Bot

Comment 14 2016-04-18 12:39:04 PDT

Attachment 276658 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:69: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:71: One space before end of line comments [whitespace/comments] [5] Total errors found: 2 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Geoffrey Garen

Comment 15 2016-04-18 12:52:45 PDT

Comment on attachment 276658 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=276658&action=review r=me > Source/JavaScriptCore/ChangeLog:17 > + There was a subtle bug where we used to never compile the var injection vartiant of the variant > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2655 > + case UnresolvedProperty: > + case UnresolvedPropertyWithVarInjectionChecks: I think we want to OSR exit on these two types of access because the baseline JIT can gather profiling data that resolves them. To be conservative, you could say "OSR exit on these types, but only if I haven't done so already." Our hope is that the baseline JIT will produce profiling on first access. But if it fails, there's no reason to try again.

Saam Barati

Comment 16 2016-04-18 13:51:46 PDT

Created attachment 276663 [details] patch for landing I've implemented Geoff's suggested heuristic.

Saam Barati

Comment 17 2016-04-18 14:25:23 PDT

Created attachment 276669 [details] patch for landing rebased

WebKit Commit Bot

Comment 18 2016-04-18 14:45:30 PDT

Attachment 276669 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:69: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:71: One space before end of line comments [whitespace/comments] [5] Total errors found: 2 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

Saam Barati

Comment 19 2016-04-18 16:40:18 PDT

Created attachment 276680 [details] patch for landing efl build fix.

WebKit Commit Bot

Comment 20 2016-04-18 16:42:19 PDT

Attachment 276680 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:69: One space before end of line comments [whitespace/comments] [5] ERROR: Source/JavaScriptCore/runtime/GetPutInfo.h:71: One space before end of line comments [whitespace/comments] [5] Total errors found: 2 in 33 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Commit Bot

Comment 21 2016-04-18 18:38:16 PDT

Comment on attachment 276680 [details] patch for landing Clearing flags on attachment: 276680 Committed r199699: <http://trac.webkit.org/changeset/199699>

WebKit Commit Bot

Comment 22 2016-04-18 18:38:24 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.