RESOLVED FIXED73390
DFG local CSE may cause incorrect reference counting for a node 
https://bugs.webkit.org/show_bug.cgi?id=73390
Summary DFG local CSE may cause incorrect reference counting for a node
Yuqiang Xian
Reported 2011-11-29 20:14:50 PST
When performing a node substitution, the ref count of the replaced child will be increased, no matter whether the user node is skipped in code generation or not. This will cause the reference count of the replaced child never get the chance to become zero and so the registers occupied by it cannot be reused simply without spilling, if it's used by a "skipped" node.
Attachments
proposed patch (2.99 KB, patch)
2011-11-29 20:19 PST, Yuqiang Xian 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Yuqiang Xian
Comment 1 2011-11-29 20:19:42 PST
Created attachment 117103 [details] proposed patch Seems 1% gain on V8 benchmark (ia32 Linux) VMs tested: "TipOfTree" at /home/yxian/WebKit_orig/WebKitBuild/Release/Source/JavaScriptCore/shell/jsc_efl "CSEFix" at /mnt/supplement/WebKit/WebKitBuild/Release_efl/Source/JavaScriptCore/shell/jsc_efl 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. TipOfTree CSEFix SunSpider: 3d-cube 8.2594+-0.0316 ^ 8.1417+-0.0347 ^ definitely 1.0145x faster 3d-morph 10.2610+-0.0448 10.2553+-0.0473 3d-raytrace 9.5848+-0.0383 9.5573+-0.0411 access-binary-trees 1.8829+-0.0373 ? 1.8895+-0.0355 ? access-fannkuch 9.5501+-0.0463 9.5283+-0.0410 access-nbody 5.3530+-0.0368 5.3301+-0.0418 access-nsieve 3.7163+-0.0307 ? 3.7211+-0.0207 ? bitops-3bit-bits-in-byte 1.1799+-0.0397 1.1685+-0.0354 bitops-bits-in-byte 4.6766+-0.0356 4.6558+-0.0305 bitops-bitwise-and 4.2498+-0.0221 ? 4.2652+-0.0288 ? bitops-nsieve-bits 6.7169+-0.0651 6.7000+-0.1013 controlflow-recursive 2.9818+-0.0306 2.9471+-0.0449 might be 1.0118x faster crypto-aes 9.7919+-0.0450 ! 9.9482+-0.0964 ! definitely 1.0160x slower crypto-md5 3.6010+-0.0291 3.5986+-0.0437 crypto-sha1 2.8681+-0.0403 2.8331+-0.0422 might be 1.0124x faster date-format-tofte 11.7510+-0.0443 ? 11.7741+-0.1023 ? date-format-xparb 11.3110+-0.0541 11.1694+-0.1267 might be 1.0127x faster math-cordic 9.1597+-0.0334 ? 9.1724+-0.0594 ? math-partial-sums 13.9912+-0.0383 ? 13.9981+-0.0563 ? math-spectral-norm 2.5070+-0.0247 ? 2.5499+-0.0466 ? might be 1.0171x slower regexp-dna 11.9294+-0.0607 11.8607+-0.0831 string-base64 4.9062+-0.0503 4.9043+-0.0540 string-fasta 10.5320+-0.0391 ? 10.5507+-0.0467 ? string-tagcloud 15.7985+-0.0564 15.7406+-0.1381 string-unpack-code 27.1332+-0.1234 26.9051+-0.2348 string-validate-input 7.1923+-0.0487 7.1431+-0.0415 <arithmetic> * 8.1110+-0.0161 8.0888+-0.0239 <geometric> 6.4497+-0.0161 6.4355+-0.0182 <harmonic> 4.9041+-0.0275 4.8927+-0.0313 TipOfTree CSEFix V8: crypto 96.9107+-0.1947 ^ 92.8136+-0.7141 ^ definitely 1.0441x faster deltablue 168.2100+-0.4819 ? 169.3268+-0.8235 ? earley-boyer 134.1887+-0.6886 ^ 132.5359+-0.1425 ^ definitely 1.0125x faster raytrace 61.6394+-0.7935 60.8866+-0.4473 might be 1.0124x faster regexp 128.6398+-0.3146 ? 129.2508+-0.6458 ? richards 179.7654+-0.2484 ^ 174.2936+-0.2297 ^ definitely 1.0314x faster splay 126.1392+-0.6525 ? 126.7273+-1.2038 ? <arithmetic> 127.9276+-0.2567 ^ 126.5478+-0.2979 ^ definitely 1.0109x faster <geometric> * 121.6040+-0.3141 ^ 120.1744+-0.3543 ^ definitely 1.0119x faster <harmonic> 114.3628+-0.4392 ^ 112.8700+-0.4246 ^ definitely 1.0132x faster TipOfTree CSEFix Kraken: ai-astar 782.5097+-0.6275 ? 782.9803+-1.5137 ? audio-beat-detection 391.4235+-8.7912 380.8584+-2.3657 might be 1.0277x faster audio-dft 353.3374+-1.4899 353.2809+-1.6748 audio-fft 246.5186+-0.4713 ^ 243.8529+-0.4220 ^ definitely 1.0109x faster audio-oscillator 343.0105+-3.4689 342.9240+-3.2334 imaging-darkroom 401.8871+-3.7631 401.8615+-3.5517 imaging-desaturate 885.7895+-0.6491 ? 886.7546+-2.1873 ? imaging-gaussian-blur 684.2414+-1.7442 ! 689.4242+-0.6069 ! definitely 1.0076x slower json-parse-financial 76.2471+-0.9816 75.7259+-0.6706 json-stringify-tinderbox 125.0676+-0.2787 124.9324+-0.2194 stanford-crypto-aes 130.7913+-0.2653 ? 131.5297+-0.8921 ? stanford-crypto-ccm 131.3663+-0.5530 130.5726+-0.5559 stanford-crypto-pbkdf2 279.1069+-1.3949 277.7063+-0.6543 stanford-crypto-sha256-iterative 109.0833+-0.2376 ? 109.2401+-0.1017 ? <arithmetic> * 352.8843+-0.7121 352.2603+-0.4665 <geometric> 270.2922+-0.6622 269.5043+-0.4897 <harmonic> 206.3656+-0.6699 205.7393+-0.5289 TipOfTree CSEFix All benchmarks: <arithmetic> 128.6544+-0.2270 ^ 128.2508+-0.1594 ^ definitely 1.0031x faster <geometric> 30.3899+-0.0577 30.2730+-0.0612 <harmonic> 8.6545+-0.0474 8.6331+-0.0537 TipOfTree CSEFix Geomean of preferred means: <scaled-result> 70.3423+-0.1205 ^ 69.9603+-0.1401 ^ definitely 1.0055x faster
Filip Pizlo
Comment 2 2011-11-29 21:18:35 PST
Comment on attachment 117103 [details] proposed patch Good catch! R=me
Filip Pizlo
Comment 3 2011-11-29 21:18:51 PST
Comment on attachment 117103 [details] proposed patch Good catch! R=me
Yuqiang Xian
Comment 4 2011-11-29 21:36:08 PST
Comment on attachment 117103 [details] proposed patch Thanks for the review. Going to land it manually.
Yuqiang Xian
Comment 5 2011-11-29 21:41:08 PST
Landed as r101447: http://trac.webkit.org/changeset/101447
Yuqiang Xian
Comment 6 2011-11-29 21:43:22 PST
Comment on attachment 117103 [details] proposed patch Clear flags for landed patch.
Note You need to log in before you can comment on or make changes to this bug.