Bug 64969 – DFG JIT generates inefficient code for speculation failures

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

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.

[Filip Pizlo]

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.

[WebKit Review Bot]

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.

[Filip Pizlo]

Created attachment 101629 [details]
the patch (fix style)

[WebKit Review Bot]

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.

[Filip Pizlo]

Created attachment 101631 [details]
the patch (fix more review)

[Filip Pizlo]

Created attachment 101950 [details]
the patch (improved assertion checking)

This improves the patch with more assertion checking, and encapsulation of the GeneralizedRegister class.

[WebKit Review Bot]

Comment on attachment 101950 [details]
the patch (improved assertion checking)

Clearing flags on attachment: 101950

Committed r91804: <http://trac.webkit.org/changeset/91804>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.