Bug 86552 - DFG should optimize aliased uses of the Arguments object of the current call frame
Summary: DFG should optimize aliased uses of the Arguments object of the current call ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 87378
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-15 17:31 PDT by Filip Pizlo
Modified: 2012-05-24 05:36 PDT (History)
0 users

See Also:


Attachments
work in progress (39.78 KB, patch)
2012-05-15 17:35 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (52.43 KB, patch)
2012-05-15 18:21 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
fixed some issues (53.21 KB, patch)
2012-05-15 18:29 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
it's starting to do things (65.84 KB, patch)
2012-05-15 23:04 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (66.71 KB, patch)
2012-05-17 15:06 PDT, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff
almost ready (81.52 KB, patch)
2012-05-17 18:45 PDT, Filip Pizlo
no flags 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-05-15 17:31:21 PDT
We currently optimize the heck out of:

arguments[i]
arguments.length

But we fail to do the same optimizations if we have:

var x = arguments;
x[i]
x.length

In particular, we will presently allocate (and tear off) an arguments object, and then optimize x[i] and x.length to be array-like accesses to that arguments object. While that is faster than what the baseline JIT and LLInt would do, it's still rather redundant, since the DFG has most of the machinery in place to realize that this aliased form of arguments access is equivalent to the non-aliased form.
Comment 1 Filip Pizlo 2012-05-15 17:35:14 PDT
Created attachment 142107 [details]
work in progress

Still need to implement the OSR side of this optimization.

And check that the code even compiles. ;-)
Comment 2 Filip Pizlo 2012-05-15 18:21:54 PDT
Created attachment 142114 [details]
more

Wrote the 64-bit side of the OSR part of this optimization.

Still haven't even tried compiling this.
Comment 3 Filip Pizlo 2012-05-15 18:29:32 PDT
Created attachment 142115 [details]
fixed some issues

Fixed some issues that I found through code inspection.
Comment 4 Filip Pizlo 2012-05-15 23:04:06 PDT
Created attachment 142158 [details]
it's starting to do things

Still more work to be done though.
Comment 5 Filip Pizlo 2012-05-17 15:06:08 PDT
Created attachment 142563 [details]
the patch
Comment 6 Geoffrey Garen 2012-05-17 15:53:10 PDT
Comment on attachment 142563 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142563&action=review

Test cases and 32bit, please.

> Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp:442
> +                    // we should remote the phantom reference, since:

s/remote/remove/
Comment 7 Filip Pizlo 2012-05-17 18:45:55 PDT
Created attachment 142606 [details]
almost ready

I just need to turn some of my ad-hoc tests into LayoutTests.
Comment 8 Filip Pizlo 2012-05-17 22:30:16 PDT
Landed with a bunch of LayoutTests in http://trac.webkit.org/changeset/117542
Comment 9 Filip Pizlo 2012-05-17 22:32:52 PDT
> s/remote/remove/

Forgot to fix in the main commit.  Fix landed in http://trac.webkit.org/changeset/117543
Comment 10 Filip Pizlo 2012-05-23 22:32:42 PDT
Merged in http://trac.webkit.org/changeset/118323