Bug 94448 - Array accesses should remember what kind of array they are predicted to access
Summary: Array accesses should remember what kind of array they are predicted to access
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 91933
  Show dependency treegraph
 
Reported: 2012-08-20 00:24 PDT by Filip Pizlo
Modified: 2012-08-22 20:38 PDT (History)
5 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 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.
Comment 1 Filip Pizlo 2012-08-20 00:29:38 PDT
Created attachment 159347 [details]
work in progress
Comment 2 Filip Pizlo 2012-08-21 16:32:22 PDT
Created attachment 159797 [details]
work in progress
Comment 3 Filip Pizlo 2012-08-21 19:38:15 PDT
Created attachment 159845 [details]
work in progress
Comment 4 Filip Pizlo 2012-08-21 19:46:21 PDT
Created attachment 159846 [details]
almost there
Comment 5 Filip Pizlo 2012-08-21 20:23:13 PDT
Created attachment 159851 [details]
getting there
Comment 6 Filip Pizlo 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.
Comment 7 Filip Pizlo 2012-08-22 14:22:29 PDT
Created attachment 160010 [details]
more
Comment 8 Filip Pizlo 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.
Comment 9 Filip Pizlo 2012-08-22 17:23:30 PDT
Created attachment 160041 [details]
the patch
Comment 10 Gavin Barraclough 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
Comment 11 Filip Pizlo 2012-08-22 20:38:43 PDT
Landed in http://trac.webkit.org/changeset/126387