Bug 67670 – DFG JIT does not optimize booleans

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

[Filip Pizlo]

The DFG JIT tries to perform type-specific optimizations, particularly for arrays, cells, integers, and doubles.  But it does nothing for booleans, even though some obvious optimizations are possible, like simplifying the code emitted for Branch() if the input is known or predicted boolean.

[Filip Pizlo]

Created attachment 106483 [details]
the patch

Still have more testing to do, but I think it's ready for review.

[WebKit Review Bot]

Attachment 106483 [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.h:221:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1211:  Should have a space between // and comment  [whitespace/comments] [4]
Source/JavaScriptCore/bytecode/ValueProfile.h:184:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/bytecode/ValueProfile.h:188:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/dfg/DFGNode.h:293:  Should have a space between // and comment  [whitespace/comments] [4]
Source/JavaScriptCore/dfg/DFGPredictionTracker.h:90:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Source/JavaScriptCore/dfg/DFGJITCodeGenerator.h:414:  The parameter name "nodeIndex" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/dfg/DFGPropagator.cpp:82:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/JavaScriptCore/dfg/DFGPropagator.cpp:160:  Should have a space between // and comment  [whitespace/comments] [4]
Source/JavaScriptCore/dfg/DFGPropagator.cpp:163:  Should have a space between // and comment  [whitespace/comments] [4]
Source/JavaScriptCore/dfg/DFGGraph.h:213:  The parameter name "codeBlock" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 11 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Here's the current performance of this patch relative to just dynamic optimization alone:



Benchmark report for V8.

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc
"DynamicOpt" at /Volumes/Data/pizlo/quartary/OpenSource/WebKitBuild/Release/jsc
"DynamicBoolOpt" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc

Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. Used 1 benchmark iteration per VM
invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting
benchmark execution times with 95% confidence intervals in milliseconds.

                     TipOfTree               DynamicOpt            DynamicBoolOpt       DynamicBoolOpt v. TipOfTree

crypto            92.9087+-1.0076    ^    82.0337+-0.5620    ?    82.9391+-1.3857       ^ definitely 1.1202x faster
deltablue        273.9975+-5.3832    ^   261.2656+-1.0780    ?   261.7145+-2.2611       ^ definitely 1.0469x faster
earley-boyer     101.8557+-1.8443    !   108.5937+-2.2791    ^   102.9614+-0.9825       ? might be 1.0109x slower
raytrace          81.4509+-1.0405    ?    83.5420+-1.2135    ^    81.4757+-0.7785       ?
regexp           111.8391+-1.3009        110.3129+-1.7970    ?   111.0009+-1.0489       
richards         245.8331+-1.3454    ^   236.6249+-1.3738    ^   220.2786+-2.1472       ^ definitely 1.1160x faster
splay            104.5783+-1.5713    ?   105.3847+-0.8156    ?   105.7928+-2.3796       ? might be 1.0116x slower

<arithmetic>     144.6376+-1.0918    ^   141.1082+-0.3229    ^   138.0233+-0.5582       ^ definitely 1.0479x faster
<geometric>      129.3673+-0.6707    ^   127.0473+-0.3857    ^   124.7610+-0.6602       ^ definitely 1.0369x faster
<harmonic>       118.4370+-0.4619    ^   116.6713+-0.4381    ^   114.9704+-0.7147       ^ definitely 1.0302x faster

[Filip Pizlo]

Created attachment 106500 [details]
the patch - fix windows, style

[Filip Pizlo]

Created attachment 106518 [details]
the patch - undo accidental Platform.h change

[Filip Pizlo]

Created attachment 106531 [details]
the patch - fixed a silly bug

Holding off commit until tests pass.

[Filip Pizlo]

Comment on attachment 106531 [details]
the patch - fixed a silly bug

OK, this appears ready.

[WebKit Review Bot]

Comment on attachment 106531 [details]
the patch - fixed a silly bug

Clearing flags on attachment: 106531

Committed r94629: <http://trac.webkit.org/changeset/94629>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.