Bug 86552

Summary: DFG should optimize aliased uses of the Arguments object of the current call frame
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 87378    
Bug Blocks:    
Attachments:
Description Flags
work in progress
none
more
none
fixed some issues
none
it's starting to do things
none
the patch
ggaren: review+
almost ready none

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