Bug 94448

Summary:

Array accesses should remember what kind of array they are predicted to access

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, ggaren, mhahnenberg, msaboff, oliver

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

91933

Attachments:

Description	Flags
work in progress	none
work in progress	none
work in progress	none
almost there	none
getting there	none
it runs simple things	none
more	none
seems to work	none
the patch	barraclough: review+

Filip Pizlo

Reported 2012-08-20 00:24:00 PDT

It used to be that array accesses would know what kind of array they're accessing based on the predictions of their children. But that's no longer the case, since part of the decision process of what kind of array access to compile comes from the ArrayProfile. Hence, array accesses should remember what the ArrayProfile said rather than relying entirely on the predictions of their children.

Attachments
work in progress (37.83 KB, patch) 2012-08-20 00:29 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (72.83 KB, patch) 2012-08-21 16:32 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (100.68 KB, patch) 2012-08-21 19:38 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
almost there (107.21 KB, patch) 2012-08-21 19:46 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
getting there (118.02 KB, patch) 2012-08-21 20:23 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it runs simple things (131.79 KB, patch) 2012-08-22 00:51 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more (133.00 KB, patch) 2012-08-22 14:22 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
seems to work (169.37 KB, patch) 2012-08-22 15:57 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (169.72 KB, patch) 2012-08-22 17:23 PDT, Filip Pizlo	barraclough: review+	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-08-20 00:29:38 PDT

Created attachment 159347 [details] work in progress

Filip Pizlo

Comment 2 2012-08-21 16:32:22 PDT

Created attachment 159797 [details] work in progress

Filip Pizlo

Comment 3 2012-08-21 19:38:15 PDT

Created attachment 159845 [details] work in progress

Filip Pizlo

Comment 4 2012-08-21 19:46:21 PDT

Created attachment 159846 [details] almost there

Filip Pizlo

Comment 5 2012-08-21 20:23:13 PDT

Created attachment 159851 [details] getting there

Filip Pizlo

Comment 6 2012-08-22 00:51:26 PDT

Created attachment 159875 [details] it runs simple things Still more testing to go. And there's also 32-bit to worry about.

Filip Pizlo

Comment 7 2012-08-22 14:22:29 PDT

Created attachment 160010 [details] more

Filip Pizlo

Comment 8 2012-08-22 15:57:29 PDT

Created attachment 160026 [details] seems to work Seems at least perf-neutral and it runs non-trivial workloads, but I'm still testing it more.

Filip Pizlo

Comment 9 2012-08-22 17:23:30 PDT

Created attachment 160041 [details] the patch

Gavin Barraclough

Comment 10 2012-08-22 18:14:20 PDT

Comment on attachment 160041 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=160041&action=review > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:151 > + for (unsigned n = 2; n--;) why? - please to be commenting. > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:328 > + case PutByVal: { case PutByValAlias: > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:344 > if (m_graph[child3].shouldSpeculateInteger()) if (!condition) fixDoubleEdge

Filip Pizlo

Comment 11 2012-08-22 20:38:43 PDT

Landed in http://trac.webkit.org/changeset/126387

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