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

RESOLVED FIXED 81805

DFG::compileValueToInt32 Sometime Generates GPR to FPR reg back to GPR

https://bugs.webkit.org/show_bug.cgi?id=81805

Summary DFG::compileValueToInt32 Sometime Generates GPR to FPR reg back to GPR

Michael Saboff

Reported 2012-03-21 11:16:39 PDT

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.

Attachments
Draft Patch - No 32_64 work (7.96 KB, patch) 2012-03-21 11:33 PDT, Michael Saboff	webkit-ews: commit-queue-	Details Formatted Diff Diff
Updated patch with 32 bit case added (8.70 KB, patch) 2012-03-23 14:18 PDT, Michael Saboff	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Michael Saboff

Comment 1 2012-03-21 11:33:12 PDT

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

Comment 2 2012-03-21 11:35:09 PDT

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 3 2012-03-21 11:37:42 PDT

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 4 2012-03-21 11:38:07 PDT

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 5 2012-03-21 12:38:54 PDT

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 6 2012-03-21 13:09:04 PDT

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

Comment 7 2012-03-21 14:15:24 PDT

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 8 2012-03-21 14:28:22 PDT

Comment on attachment 133081 [details] Draft Patch - No 32_64 work Looks good to me so far.

Michael Saboff

Comment 9 2012-03-23 14:18:44 PDT

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

WebKit Review Bot

Comment 10 2012-03-23 14:22:03 PDT

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

Comment 11 2012-03-23 14:27:43 PDT

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

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware All

OS All

Product WebKit

Component JavaScriptCore

Assignee

Michael Saboff

Reported

2012-03-21 11:16 PDT

Modified

2012-03-24 12:11 PDT History

CC List

5 users Show

URL

Keywords

Depends on

Blocks