Bug 87929 - DFG CSE should be able to eliminate unnecessary flushes of arguments and captured variables
Summary: DFG CSE should be able to eliminate unnecessary flushes of arguments and capt...
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:
Blocks:
 
Reported: 2012-05-30 23:44 PDT by Filip Pizlo
Modified: 2012-06-02 15:59 PDT (History)
0 users

See Also:

Attachments
work in progress (11.81 KB, patch)
2012-05-30 23:47 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
more (22.86 KB, patch)
2012-06-01 23:05 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
moar (27.65 KB, patch)
2012-06-01 23:49 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
bigger patch = better patch... ?... (37.81 KB, patch)
2012-06-02 02:01 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (47.14 KB, patch)
2012-06-02 15:15 PDT, Filip Pizlo 		ggaren: review+
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-05-30 23:44:09 PDT 
We currently flush SetLocals to captured variables and arguments to inline functions.  But this is not necessary, if the code between the SetLocal and the Flush is speculated to not be clobbering.
Comment 1 Filip Pizlo 2012-05-30 23:47:50 PDT 
Created attachment 144997 [details]
work in progress

It appears to do things, but needs a lot more testing.  Putting up for EWS.
Comment 2 Filip Pizlo 2012-06-01 23:05:53 PDT 
Created attachment 145429 [details]
more

Making progress.

But it still crashes on some of my more awesome tests.
Comment 3 Filip Pizlo 2012-06-01 23:49:50 PDT 
Created attachment 145434 [details]
moar
Comment 4 Filip Pizlo 2012-06-02 02:01:58 PDT 
Created attachment 145439 [details]
bigger patch = better patch... ?...
Comment 5 Filip Pizlo 2012-06-02 14:03:57 PDT 
Comment on attachment 145439 [details]
bigger patch = better patch... ?...

This appears to pass all of the tests on 64-bit.  Now I need to port it to 32-bit.
Comment 6 Filip Pizlo 2012-06-02 15:15:56 PDT 
Created attachment 145460 [details]
the patch

Both 64-bit and 32-bit work.
Comment 7 Geoffrey Garen 2012-06-02 15:46:57 PDT 
Comment on attachment 145460 [details]
the patch

r=me
Comment 8 Filip Pizlo 2012-06-02 15:59:34 PDT 
Landed in http://trac.webkit.org/changeset/119342