73390 2011-11-29 20:14:50 -0800 DFG local CSE may cause incorrect reference counting for a node 2011-11-29 21:43:22 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 yuqiang.xian webkit-unassigned barraclough fpizlo oldest_to_newest 511321 0 yuqiang.xian 2011-11-29 20:14:50 -0800 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. 511324 1 117103 yuqiang.xian 2011-11-29 20:19:42 -0800 Created attachment 117103 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 511354 2 117103 fpizlo 2011-11-29 21:18:35 -0800 Comment on attachment 117103 proposed patch Good catch! R=me 511355 3 117103 fpizlo 2011-11-29 21:18:51 -0800 Comment on attachment 117103 proposed patch Good catch! R=me 511365 4 117103 yuqiang.xian 2011-11-29 21:36:08 -0800 Comment on attachment 117103 proposed patch Thanks for the review. Going to land it manually. 511373 5 yuqiang.xian 2011-11-29 21:41:08 -0800 Landed as r101447: http://trac.webkit.org/changeset/101447 511375 6 117103 yuqiang.xian 2011-11-29 21:43:22 -0800 Comment on attachment 117103 proposed patch Clear flags for landed patch. 117103 2011-11-29 20:19:42 -0800 2011-11-29 21:43:22 -0800 proposed patch 73390.patch text/plain 3058 yuqiang.xian SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTAxNDQ0KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBA CisyMDExLTExLTI5ICBZdXFpYW5nIFhpYW4gIDx5dXFpYW5nLnhpYW5AaW50ZWwuY29tPgorCisg ICAgICAgIERGRyBsb2NhbCBDU0UgbWF5IGNhdXNlIGluY29ycmVjdCByZWZlcmVuY2UgY291bnRp bmcgZm9yIGEgbm9kZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9NzMzOTAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBXaGVuIHBlcmZvcm1pbmcgYSBub2RlIHN1YnN0aXR1dGlvbiwgdGhlIHJlZiBjb3VudCBv ZiB0aGUgcmVwbGFjZWQKKyAgICAgICAgY2hpbGQgd2lsbCBiZSBpbmNyZWFzZWQsIG5vIG1hdHRl ciB3aGV0aGVyIHRoZSB1c2VyIG5vZGUgaXMgc2tpcHBlZCBpbgorICAgICAgICBjb2RlIGdlbmVy YXRpb24gb3Igbm90LiBUaGlzIHdpbGwgY2F1c2UgdGhlIHJlZmVyZW5jZSBjb3VudCBvZiB0aGUK KyAgICAgICAgcmVwbGFjZWQgY2hpbGQgbmV2ZXIgZ2V0IHRoZSBjaGFuY2UgdG8gYmVjb21lIHpl cm8gYW5kIHNvIHRoZQorICAgICAgICByZWdpc3RlcnMgb2NjdXBpZWQgYnkgaXQgY2Fubm90IGJl IHJldXNlZCBzaW1wbHkgd2l0aG91dCBzcGlsbGluZywgaWYKKyAgICAgICAgaXQncyB1c2VkIGJ5 IGEgInNraXBwZWQiIG5vZGUuCisgICAgICAgIFRoaXMgaXMgYSAxJSBnYWluIG9uIFY4IGJlbmNo bWFyaywgdGVzdGVkIG9uIElBMzIgTGludXguCisKKyAgICAgICAgKiBkZmcvREZHUHJvcGFnYXRv ci5jcHA6CisgICAgICAgIChKU0M6OkRGRzo6UHJvcGFnYXRvcjo6cGVyZm9ybVN1YnN0aXR1dGlv bik6CisgICAgICAgIChKU0M6OkRGRzo6UHJvcGFnYXRvcjo6cGVyZm9ybU5vZGVDU0UpOgorCiAy MDExLTExLTI5ICBTYW0gV2VpbmlnICA8c2FtQHdlYmtpdC5vcmc+CiAKICAgICAgICAgQWRkIENP TVBJTEVSX1NVUFBPUlRTIG1hY3JvIHRvIGFsbG93IGZvciBjb21waWxlciBmZWF0dXJlIHRlc3Rp bmcKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHUHJvcGFnYXRvci5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdQcm9wYWdhdG9yLmNwcAko cmV2aXNpb24gMTAxNDQzKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdQcm9wYWdh dG9yLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTE5MCw3ICsxMTkwLDcgQEAgcHJpdmF0ZToKICAg ICAgICAgcmV0dXJuIE5vTm9kZTsKICAgICB9CiAgICAgCi0gICAgdm9pZCBwZXJmb3JtU3Vic3Rp dHV0aW9uKE5vZGVJbmRleCYgY2hpbGQpCisgICAgdm9pZCBwZXJmb3JtU3Vic3RpdHV0aW9uKE5v ZGVJbmRleCYgY2hpbGQsIGJvb2wgYWRkUmVmID0gdHJ1ZSkKICAgICB7CiAgICAgICAgIC8vIENo ZWNrIGlmIHRoaXMgb3BlcmFuZCBpcyBhY3R1YWxseSB1bnVzZWQuCiAgICAgICAgIGlmIChjaGls ZCA9PSBOb05vZGUpCkBAIC0xMjA3LDcgKzEyMDcsOCBAQCBwcml2YXRlOgogICAgICAgICAvLyBo YXZlIGEgcmVwbGFjZW1lbnQuCiAgICAgICAgIEFTU0VSVChtX3JlcGxhY2VtZW50c1tjaGlsZF0g PT0gTm9Ob2RlKTsKICAgICAgICAgCi0gICAgICAgIG1fZ3JhcGhbY2hpbGRdLnJlZigpOworICAg ICAgICBpZiAoYWRkUmVmKQorICAgICAgICAgICAgbV9ncmFwaFtjaGlsZF0ucmVmKCk7CiAgICAg fQogICAgIAogICAgIHZvaWQgc2V0UmVwbGFjZW1lbnQoTm9kZUluZGV4IHJlcGxhY2VtZW50KQpA QCAtMTI0NiwxNiArMTI0NywxOCBAQCBwcml2YXRlOgogICAgIAogICAgIHZvaWQgcGVyZm9ybU5v ZGVDU0UoTm9kZSYgbm9kZSkKICAgICB7CisgICAgICAgIGJvb2wgc2hvdWxkR2VuZXJhdGUgPSBu b2RlLnNob3VsZEdlbmVyYXRlKCk7CisKICAgICAgICAgaWYgKG5vZGUub3AgJiBOb2RlSGFzVmFy QXJncykgewogICAgICAgICAgICAgZm9yICh1bnNpZ25lZCBjaGlsZElkeCA9IG5vZGUuZmlyc3RD aGlsZCgpOyBjaGlsZElkeCA8IG5vZGUuZmlyc3RDaGlsZCgpICsgbm9kZS5udW1DaGlsZHJlbigp OyBjaGlsZElkeCsrKQotICAgICAgICAgICAgICAgIHBlcmZvcm1TdWJzdGl0dXRpb24obV9ncmFw aC5tX3ZhckFyZ0NoaWxkcmVuW2NoaWxkSWR4XSk7CisgICAgICAgICAgICAgICAgcGVyZm9ybVN1 YnN0aXR1dGlvbihtX2dyYXBoLm1fdmFyQXJnQ2hpbGRyZW5bY2hpbGRJZHhdLCBzaG91bGRHZW5l cmF0ZSk7CiAgICAgICAgIH0gZWxzZSB7Ci0gICAgICAgICAgICBwZXJmb3JtU3Vic3RpdHV0aW9u KG5vZGUuY2hpbGRyZW4uZml4ZWQuY2hpbGQxKTsKLSAgICAgICAgICAgIHBlcmZvcm1TdWJzdGl0 dXRpb24obm9kZS5jaGlsZHJlbi5maXhlZC5jaGlsZDIpOwotICAgICAgICAgICAgcGVyZm9ybVN1 YnN0aXR1dGlvbihub2RlLmNoaWxkcmVuLmZpeGVkLmNoaWxkMyk7CisgICAgICAgICAgICBwZXJm b3JtU3Vic3RpdHV0aW9uKG5vZGUuY2hpbGRyZW4uZml4ZWQuY2hpbGQxLCBzaG91bGRHZW5lcmF0 ZSk7CisgICAgICAgICAgICBwZXJmb3JtU3Vic3RpdHV0aW9uKG5vZGUuY2hpbGRyZW4uZml4ZWQu Y2hpbGQyLCBzaG91bGRHZW5lcmF0ZSk7CisgICAgICAgICAgICBwZXJmb3JtU3Vic3RpdHV0aW9u KG5vZGUuY2hpbGRyZW4uZml4ZWQuY2hpbGQzLCBzaG91bGRHZW5lcmF0ZSk7CiAgICAgICAgIH0K ICAgICAgICAgCi0gICAgICAgIGlmICghbm9kZS5zaG91bGRHZW5lcmF0ZSgpKQorICAgICAgICBp ZiAoIXNob3VsZEdlbmVyYXRlKQogICAgICAgICAgICAgcmV0dXJuOwogICAgICAgICAKICNpZiBE RkdfRU5BQkxFKERFQlVHX1BST1BBR0FUSU9OX1ZFUkJPU0UpCg==