Bug 64969 - DFG JIT generates inefficient code for speculation failures
Summary: DFG JIT generates inefficient code for speculation failures
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: 2011-07-21 12:07 PDT by Filip Pizlo
Modified: 2011-07-26 17:55 PDT (History)
3 users (show)

See Also:


Attachments
the patch (32.13 KB, patch)
2011-07-21 12:21 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (fix style) (31.93 KB, patch)
2011-07-21 12:40 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (fix more review) (31.90 KB, patch)
2011-07-21 12:44 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (improved assertion checking) (32.23 KB, patch)
2011-07-25 17:32 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 2011-07-21 12:07:36 PDT
When the speculative version of the DFG-generated code encounters a value that violates speculation, it jumps to the non-speculative version of the same code.  The two code sequences are generated mostly independently, with different register allocation and spill decisions, but with the invariant that the spill slots are the same and have the same format.  Currently the speculation failure code that is emitted to jump from one path to the other spills all registers used by the speculative path, and then refills the ones used by the non-speculative path.  In most cases, both paths will have succeeded in allocating registers to roughly the same nodes - so a more efficient approach would be to emit code that simply shuffles the the values in registers rather than going to memory, and to only spill and fill if one path had spilled but the other hadn't.
Comment 1 Filip Pizlo 2011-07-21 12:21:04 PDT
Created attachment 101626 [details]
the patch

This results in a 1.2% speed-up on SunSpider, is performance neutral on v8-v4 in the SunSpider harness, and a 1% speed-up (mean of score over ten runs, interleaved) in V8 version 6 using the V8 harness.
Comment 2 WebKit Review Bot 2011-07-21 12:23:47 PDT
Attachment 101626 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:133:  Missing space after ,  [whitespace/comma] [3]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:135:  Missing space after ,  [whitespace/comma] [3]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:248:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:256:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:292:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:298:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:518:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:521:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 8 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Filip Pizlo 2011-07-21 12:40:52 PDT
Created attachment 101629 [details]
the patch (fix style)
Comment 4 WebKit Review Bot 2011-07-21 12:42:47 PDT
Attachment 101629 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:282:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:288:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Filip Pizlo 2011-07-21 12:44:53 PDT
Created attachment 101631 [details]
the patch (fix more review)
Comment 6 Filip Pizlo 2011-07-25 17:32:00 PDT
Created attachment 101950 [details]
the patch (improved assertion checking)

This improves the patch with more assertion checking, and encapsulation of the GeneralizedRegister class.
Comment 7 WebKit Review Bot 2011-07-26 17:55:46 PDT
Comment on attachment 101950 [details]
the patch (improved assertion checking)

Clearing flags on attachment: 101950

Committed r91804: <http://trac.webkit.org/changeset/91804>
Comment 8 WebKit Review Bot 2011-07-26 17:55:51 PDT
All reviewed patches have been landed.  Closing bug.