Bug 67670 - DFG JIT does not optimize booleans
Summary: DFG JIT does not optimize booleans
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-09-06 13:52 PDT by Filip Pizlo
Modified: 2011-09-06 19:47 PDT (History)
4 users (show)

See Also:


Attachments
the patch (75.30 KB, patch)
2011-09-06 14:06 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch - fix windows, style (73.68 KB, patch)
2011-09-06 15:32 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch - undo accidental Platform.h change (73.23 KB, patch)
2011-09-06 16:52 PDT, Filip Pizlo
barraclough: review+
Details | Formatted Diff | Diff
the patch - fixed a silly bug (75.19 KB, patch)
2011-09-06 18:44 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-09-06 13:52:19 PDT
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.
Comment 1 Filip Pizlo 2011-09-06 14:06:23 PDT
Created attachment 106483 [details]
the patch

Still have more testing to do, but I think it's ready for review.
Comment 2 WebKit Review Bot 2011-09-06 14:07:50 PDT
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.
Comment 3 Filip Pizlo 2011-09-06 15:04:14 PDT
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
Comment 4 Filip Pizlo 2011-09-06 15:32:49 PDT
Created attachment 106500 [details]
the patch - fix windows, style
Comment 5 Filip Pizlo 2011-09-06 16:52:45 PDT
Created attachment 106518 [details]
the patch - undo accidental Platform.h change
Comment 6 Filip Pizlo 2011-09-06 18:44:38 PDT
Created attachment 106531 [details]
the patch - fixed a silly bug

Holding off commit until tests pass.
Comment 7 Filip Pizlo 2011-09-06 18:57:52 PDT
Comment on attachment 106531 [details]
the patch - fixed a silly bug

OK, this appears ready.
Comment 8 WebKit Review Bot 2011-09-06 19:46:59 PDT
Comment on attachment 106531 [details]
the patch - fixed a silly bug

Clearing flags on attachment: 106531

Committed r94629: <http://trac.webkit.org/changeset/94629>
Comment 9 WebKit Review Bot 2011-09-06 19:47:04 PDT
All reviewed patches have been landed.  Closing bug.