Bug 81805 – DFG::compileValueToInt32 Sometime Generates GPR to FPR reg back to GPR

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

SpeculativeJIT::compileValueToInt32() will generate code that converts an integer value to a FP value back to an int value when the source operand is already a JSValue.

This code can be restructured to check the format of the operand before blindly asking for a double operand which will quietly convert the JSValue to a double register.

[Michael Saboff]

Created attachment 133081 [details]
Draft Patch - No 32_64 work

Patch for review with 64 bit work.

This improves several SunSpider and Kraken benchmarks.


Benchmark report for SunSpider, V8, and Kraken on msaboff-pro (MacPro5,1).

VMs tested:
"Baseline" at /Volumes/Data/src/webkit.baseline/WebKitBuild/Release/jsc (r111579)
"DirectToInt" at /Volumes/Data/src/webkit/WebKitBuild/Release/jsc (r111579)

Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc() between sample
measurements. 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.

                                             Baseline              DirectToInt                                   
SunSpider:
   3d-cube                                7.2786+-0.0544    ?     7.2893+-0.0661       ?
   3d-morph                               7.3513+-0.0567          7.3068+-0.0414       
   3d-raytrace                            9.3693+-0.0710          9.3604+-0.0705       
   access-binary-trees                    1.7592+-0.0373          1.7495+-0.0209       
   access-fannkuch                        7.6170+-0.0372          7.5977+-0.0210       
   access-nbody                           3.8707+-0.0187          3.8516+-0.0117       
   access-nsieve                          3.5423+-0.0335    ?     3.5511+-0.0328       ?
   bitops-3bit-bits-in-byte               1.3404+-0.0143          1.3399+-0.0125       
   bitops-bits-in-byte                    5.4447+-0.0462          5.4287+-0.0352       
   bitops-bitwise-and                     3.3502+-0.0189          3.3345+-0.0163       
   bitops-nsieve-bits                     3.6733+-0.0285    ^     3.2235+-0.0229       ^ definitely 1.1395x faster
   controlflow-recursive                  2.3282+-0.0292    ?     2.3446+-0.0372       ?
   crypto-aes                             7.7984+-0.0881          7.7708+-0.0785       
   crypto-md5                             3.2133+-0.0129    ?     3.2383+-0.0436       ?
   crypto-sha1                            2.5430+-0.0371          2.5164+-0.0290         might be 1.0106x faster
   date-format-tofte                     11.3623+-0.2336    ^    10.8698+-0.1175       ^ definitely 1.0453x faster
   date-format-xparb                     10.7303+-0.3135         10.4070+-0.1848         might be 1.0311x faster
   math-cordic                            4.0551+-0.0201          4.0387+-0.0119       
   math-partial-sums                      8.7762+-0.0245    ?     8.8244+-0.0367       ?
   math-spectral-norm                     2.7284+-0.0217          2.7130+-0.0056       
   regexp-dna                            10.3735+-0.3020    ^     9.8641+-0.0927       ^ definitely 1.0517x faster
   string-base64                          4.5375+-0.0277    ^     4.4934+-0.0162       ^ definitely 1.0098x faster
   string-fasta                           7.1314+-0.0520    ?     7.1377+-0.0416       ?
   string-tagcloud                       12.8483+-0.0639    ?    12.9168+-0.1127       ?
   string-unpack-code                    22.4526+-0.5192         21.8236+-0.1268         might be 1.0288x faster
   string-validate-input                  6.5903+-0.3045    ^     6.1881+-0.0692       ^ definitely 1.0650x faster

   <arithmetic> *                         6.6179+-0.0241    ^     6.5069+-0.0165       ^ definitely 1.0171x faster
   <geometric>                            5.3628+-0.0170    ^     5.2863+-0.0140       ^ definitely 1.0145x faster
   <harmonic>                             4.3154+-0.0189    ^     4.2659+-0.0157       ^ definitely 1.0116x faster

                                             Baseline              DirectToInt                                   
V8:
   crypto                                75.4345+-0.1879         75.4171+-0.2407       
   deltablue                            158.7217+-1.0601        157.1792+-0.9181       
   earley-boyer                          98.0935+-2.5741         98.0639+-2.2742       
   raytrace                              56.4682+-0.2416         56.1317+-0.2425       
   regexp                               101.3459+-0.2197    !   102.2162+-0.6182       ! definitely 1.0086x slower
   richards                             143.8106+-0.9807        143.2084+-0.9522       
   splay                                 61.2865+-1.1657         60.8484+-0.7583       

   <arithmetic>                          99.3087+-0.4707         99.0093+-0.4319         might be 1.0030x faster
   <geometric> *                         92.7781+-0.4433         92.5298+-0.4058         might be 1.0027x faster
   <harmonic>                            86.8219+-0.4214         86.5768+-0.3587         might be 1.0028x faster

                                             Baseline              DirectToInt                                   
Kraken:
   ai-astar                             831.1022+-3.9854    ^   799.9239+-11.1972      ^ definitely 1.0390x faster
   audio-beat-detection                 195.6483+-1.9955        194.7191+-0.4076       
   audio-dft                            285.8591+-2.2126        284.9669+-2.0853       
   audio-fft                            119.4775+-0.2170    ?   119.5792+-0.1905       ?
   audio-oscillator                     304.8577+-2.3525        304.6109+-2.0223       
   imaging-darkroom                     295.9842+-6.8677        295.4626+-7.4871       
   imaging-desaturate                   236.2551+-0.2371    ?   236.5313+-0.4194       ?
   imaging-gaussian-blur                456.7690+-1.6551        456.0693+-1.0253       
   json-parse-financial                  64.6038+-0.3371    ^    64.0770+-0.1706       ^ definitely 1.0082x faster
   json-stringify-tinderbox              78.5745+-0.2858    ^    77.8383+-0.1922       ^ definitely 1.0095x faster
   stanford-crypto-aes                   86.7475+-0.3436    ^    83.3839+-0.3132       ^ definitely 1.0403x faster
   stanford-crypto-ccm                   77.9236+-0.9315         77.0288+-0.5462         might be 1.0116x faster
   stanford-crypto-pbkdf2               192.9524+-0.6025    ^   190.7468+-0.7171       ^ definitely 1.0116x faster
   stanford-crypto-sha256-iterative      91.6525+-0.4246    ^    89.3379+-0.1357       ^ definitely 1.0259x faster

   <arithmetic> *                       237.0291+-0.6198    ^   233.8769+-1.0040       ^ definitely 1.0135x faster
   <geometric>                          178.2848+-0.4278    ^   176.3345+-0.4250       ^ definitely 1.0111x faster
   <harmonic>                           140.0909+-0.3096    ^   138.3153+-0.2273       ^ definitely 1.0128x faster

                                             Baseline              DirectToInt                                   
All benchmarks:
   <arithmetic>                          89.0560+-0.2244    ^    88.0111+-0.3551       ^ definitely 1.0119x faster
   <geometric>                           23.2840+-0.0511    ^    23.0150+-0.0560       ^ definitely 1.0117x faster
   <harmonic>                             7.5739+-0.0324    ^     7.4877+-0.0271       ^ definitely 1.0115x faster

                                             Baseline              DirectToInt                                   
Geomean of preferred means:
   <scaled-result>                       52.6000+-0.1290    ^    52.0251+-0.1701       ^ definitely 1.0111x faster

[WebKit Review Bot]

Attachment 133081 [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/DFGSpeculativeJIT64.cpp:1469:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1503:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 2 in 4 files


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

[Early Warning System Bot]

Comment on attachment 133081 [details]
Draft Patch - No 32_64 work

Attachment 133081 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12066539

[Early Warning System Bot]

Comment on attachment 133081 [details]
Draft Patch - No 32_64 work

Attachment 133081 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12086558

[Gustavo Noronha (kov)]

Comment on attachment 133081 [details]
Draft Patch - No 32_64 work

Attachment 133081 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12086561

[Geoffrey Garen]

Comment on attachment 133081 [details]
Draft Patch - No 32_64 work

View in context: https://bugs.webkit.org/attachment.cgi?id=133081&action=review

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:1443
> +GeneratedOperandType SpeculativeJIT::checkGeneratedType(NodeIndex nodeIndex)

Why does this function live in the 64bit-only file?

[Gavin Barraclough]

Michael, yep - looks like you're on the right track here to me, cc'ing filip in case he has any input, but looks good.

(In reply to comment #6)
> Why does this function live in the 64bit-only file?

Yes, looks like this could be cross-platform.  This is just a WIP patch for an early pre-review, Michael plans to port this code to 32_64 before landing.

G.

[Filip Pizlo]

Comment on attachment 133081 [details]
Draft Patch - No 32_64 work

Looks good to me so far.

[Michael Saboff]

Created attachment 133559 [details]
Updated patch with 32 bit case added

[WebKit Review Bot]

Attachment 133559 [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/DFGSpeculativeJIT.cpp:1492:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1531:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 2 in 3 files


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

[Michael Saboff]

Committed r111906: <http://trac.webkit.org/changeset/111906>