150961 2015-11-05 15:12:58 -0800 B3 should have CSE 2015-12-11 15:21:07 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED P2 Normal --- 150507 1 fpizlo fpizlo commit-queue keith_miller mark.lam msaboff saam oldest_to_newest 1139799 0 fpizlo 2015-11-05 15:12:58 -0800 Fun! 1148520 1 267183 fpizlo 2015-12-11 13:23:04 -0800 Created attachment 267183 the patch 1148522 2 commit-queue 2015-12-11 13:25:20 -0800 Attachment 267183 did not pass style-queue: ERROR: Source/JavaScriptCore/b3/B3ReduceStrength.cpp:41: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style. 1148558 3 267183 benjamin 2015-12-11 15:00:27 -0800 Comment on attachment 267183 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=267183&action=review > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > + m_dominators = &m_proc.dominators(); // Recompute if necessary. This is a tiny bit scary :) > Source/JavaScriptCore/b3/B3Value.cpp:448 > + case BitwiseCast: Oops 1148559 4 fpizlo 2015-12-11 15:02:53 -0800 (In reply to comment #3) > Comment on attachment 267183 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=267183&action=review > > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > > + m_dominators = &m_proc.dominators(); // Recompute if necessary. > > This is a tiny bit scary :) > > > Source/JavaScriptCore/b3/B3Value.cpp:448 > > + case BitwiseCast: > > Oops Why oops? BitwiseCast is a pure value, and so should have a value key. 1148565 5 fpizlo 2015-12-11 15:16:58 -0800 (In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 267183 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=267183&action=review > > > > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > > > + m_dominators = &m_proc.dominators(); // Recompute if necessary. > > > > This is a tiny bit scary :) > > > > > Source/JavaScriptCore/b3/B3Value.cpp:448 > > > + case BitwiseCast: > > > > Oops > > Why oops? BitwiseCast is a pure value, and so should have a value key. Never mind. :-) 1148566 6 fpizlo 2015-12-11 15:19:00 -0800 (In reply to comment #3) > Comment on attachment 267183 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=267183&action=review > > > Source/JavaScriptCore/b3/B3ReduceStrength.cpp:112 > > + m_dominators = &m_proc.dominators(); // Recompute if necessary. > > This is a tiny bit scary :) What is scary about it? The cost? It's true that CSE adds cost to reduceStrength. About half of that cost is dominators. Interestingly, this code doesn't need a dominator tree; it only wants dominance queries. For most CFGs, computing dominance using the data flow fixpoint is cheaper than the Lengauer-Tarjan algorithm that we use, so long as you just want dominance queries and not the tree. So, we could reduce that cost if we felt that we needed to. But half of the cost of the CSE is just the HashMap, of course. ;-) > > > Source/JavaScriptCore/b3/B3Value.cpp:448 > > + case BitwiseCast: > > Oops 1148567 7 fpizlo 2015-12-11 15:21:07 -0800 Landed in http://trac.webkit.org/changeset/193987 267183 2015-12-11 13:23:04 -0800 2015-12-11 15:00:27 -0800 the patch blah.patch text/plain 6609 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTkzOTczKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBA CisyMDE1LTEyLTExICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg QjMgc2hvdWxkIGhhdmUgQ1NFCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xNTA5NjEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBUaGlzIGltcGxlbWVudHMgYSB2ZXJ5IHNpbXBsZSBDU0UgZm9yIHB1cmUgdmFs dWVzLiBJIG5lZWQgdGhpcyBhcyBhIHByZXJlcXVpc2l0ZSBmb3Igb3RoZXIKKyAgICAgICAgb3B0 aW1pemF0aW9ucyB0aGF0IEknbSBpbXBsZW1lbnRpbmcuIEZvciBub3csIHRoaXMgaXMgbmV1dHJh bCBvbiBpbWFnaW5nLWdhdXNzaWFuLWJsdXIgYnV0IGEKKyAgICAgICAgc2xvdy1kb3duIG9uIGFz bS5qcyBjb2RlLiBJIHN1c3BlY3QgdGhhdCB0aGUgYXNtLmpzIHNsb3ctZG93biBpcyBiZWNhdXNl IG9mIG90aGVyIHRoaW5ncyB0aGF0IGFyZQorICAgICAgICBzdGlsbCBnb2luZyB3cm9uZywgYW5k IGFueXdheSwgSSBuZWVkIENTRSB0byBiZSBhYmxlIHRvIGRvIGV2ZW4gdGhlIG1vc3QgYmFzaWMg YXNtLmpzIHN0cmVuZ3RoCisgICAgICAgIHJlZHVjdGlvbnMuCisKKyAgICAgICAgKiBiMy9CM1Jl ZHVjZVN0cmVuZ3RoLmNwcDoKKyAgICAgICAgKiBiMy9CM1JlZHVjZVN0cmVuZ3RoLmg6CisgICAg ICAgICogYjMvQjNWYWx1ZS5jcHA6CisgICAgICAgIChKU0M6OkIzOjpWYWx1ZTo6cmVwbGFjZVdp dGhJZGVudGl0eSk6CisgICAgICAgIChKU0M6OkIzOjpWYWx1ZTo6a2V5KToKKwogMjAxNS0xMi0x MSAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUuY29tPgogCiAgICAgICAgIGNoZWNr LWZvci1pbmFwcHJvcHJpYXRlLW9iamMtY2xhc3MtbmFtZXMgc2hvdWxkIGNoZWNrIGFsbCBjbGFz cyBuYW1lcywgbm90IGp1c3QgZXh0ZXJuYWxseSB2aXNpYmxlIG9uZXMKSW5kZXg6IFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9iMy9CM1JlZHVjZVN0cmVuZ3RoLmNwcAo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvYjMvQjNSZWR1Y2VTdHJlbmd0aC5jcHAJKHJldmlzaW9uIDE5Mzk0 NikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9iMy9CM1JlZHVjZVN0cmVuZ3RoLmNwcAkod29y a2luZyBjb3B5KQpAQCAtMzAsNiArMzAsNyBAQAogCiAjaW5jbHVkZSAiQjNCYXNpY0Jsb2NrSW5s aW5lcy5oIgogI2luY2x1ZGUgIkIzQ29udHJvbFZhbHVlLmgiCisjaW5jbHVkZSAiQjNEb21pbmF0 b3JzLmgiCiAjaW5jbHVkZSAiQjNJbmRleFNldC5oIgogI2luY2x1ZGUgIkIzSW5zZXJ0aW9uU2V0 SW5saW5lcy5oIgogI2luY2x1ZGUgIkIzTWVtb3J5VmFsdWUuaCIKQEAgLTM3LDggKzM4LDEwIEBA CiAjaW5jbHVkZSAiQjNQcm9jZWR1cmVJbmxpbmVzLmgiCiAjaW5jbHVkZSAiQjNVcHNpbG9uVmFs dWUuaCIKICNpbmNsdWRlICJCM1VzZUNvdW50cy5oIgorI2luY2x1ZGUgIkIzVmFsdWVLZXkuaCIK ICNpbmNsdWRlICJCM1ZhbHVlSW5saW5lcy5oIgogI2luY2x1ZGUgPHd0Zi9HcmFwaE5vZGVXb3Jr bGlzdC5oPgorI2luY2x1ZGUgPHd0Zi9IYXNoTWFwLmg+CiAKIG5hbWVzcGFjZSBKU0MgeyBuYW1l c3BhY2UgQjMgewogCkBAIC0xMDUsMTEgKzEwOCwyMCBAQCBwdWJsaWM6CiAgICAgICAgICAgICAg ICAgZGF0YUxvZyhtX3Byb2MpOwogICAgICAgICAgICAgfQogCisgICAgICAgICAgICBtX3Byb2Mu cmVzZXRWYWx1ZU93bmVycygpOworICAgICAgICAgICAgbV9kb21pbmF0b3JzID0gJm1fcHJvYy5k b21pbmF0b3JzKCk7IC8vIFJlY29tcHV0ZSBpZiBuZWNlc3NhcnkuCisgICAgICAgICAgICBtX3B1 cmVWYWx1ZXMuY2xlYXIoKTsKKwogICAgICAgICAgICAgZm9yIChCYXNpY0Jsb2NrKiBibG9jayA6 IG1fcHJvYy5ibG9ja3NJblByZU9yZGVyKCkpIHsKICAgICAgICAgICAgICAgICBtX2Jsb2NrID0g YmxvY2s7CiAgICAgICAgICAgICAgICAgCi0gICAgICAgICAgICAgICAgZm9yIChtX2luZGV4ID0g MDsgbV9pbmRleCA8IGJsb2NrLT5zaXplKCk7ICsrbV9pbmRleCkKLSAgICAgICAgICAgICAgICAg ICAgcHJvY2VzcygpOworICAgICAgICAgICAgICAgIGZvciAobV9pbmRleCA9IDA7IG1faW5kZXgg PCBibG9jay0+c2l6ZSgpOyArK21faW5kZXgpIHsKKyAgICAgICAgICAgICAgICAgICAgbV92YWx1 ZSA9IG1fYmxvY2stPmF0KG1faW5kZXgpOworICAgICAgICAgICAgICAgICAgICBtX3ZhbHVlLT5w ZXJmb3JtU3Vic3RpdHV0aW9uKCk7CisgICAgICAgICAgICAgICAgICAgIAorICAgICAgICAgICAg ICAgICAgICByZWR1Y2VWYWx1ZVN0cmVuZ3RoKCk7CisgICAgICAgICAgICAgICAgICAgIHJlcGxh Y2VJZlJlZHVuZGFudCgpOworICAgICAgICAgICAgICAgIH0KICAgICAgICAgICAgICAgICBtX2lu c2VydGlvblNldC5leGVjdXRlKG1fYmxvY2spOwogICAgICAgICAgICAgfQogCkBAIC0xMjksMTEg KzE0MSw4IEBAIHB1YmxpYzoKICAgICB9CiAgICAgCiBwcml2YXRlOgotICAgIHZvaWQgcHJvY2Vz cygpCisgICAgdm9pZCByZWR1Y2VWYWx1ZVN0cmVuZ3RoKCkKICAgICB7Ci0gICAgICAgIG1fdmFs dWUgPSBtX2Jsb2NrLT5hdChtX2luZGV4KTsKLSAgICAgICAgbV92YWx1ZS0+cGVyZm9ybVN1YnN0 aXR1dGlvbigpOwotICAgICAgICAKICAgICAgICAgc3dpdGNoIChtX3ZhbHVlLT5vcGNvZGUoKSkg ewogICAgICAgICBjYXNlIEFkZDoKICAgICAgICAgICAgIGhhbmRsZUNvbW11dGF0aXZpdHkoKTsK QEAgLTEwMTQsNiArMTAyMywzNSBAQCBwcml2YXRlOgogICAgICAgICByZXR1cm4gZmFsc2U7CiAg ICAgfQogCisgICAgdm9pZCByZXBsYWNlSWZSZWR1bmRhbnQoKQorICAgIHsKKyAgICAgICAgLy8g VGhpcyBkb2VzIGEgdmVyeSBzaW1wbGUgcHVyZSBkb21pbmF0b3ItYmFzZWQgQ1NFLiBJbiB0aGUg ZnV0dXJlIHdlIGNvdWxkIGFkZCBsb2FkIGVsaW1pbmF0aW9uLgorICAgICAgICAvLyBOb3RlIHRo YXQgaWYgd2UgYWRkIGxvYWQgZWxpbWluYXRpb24sIHdlIHNob3VsZCBkbyBpdCBieSBkaXJlY3Rs eSBtYXRjaGluZyBsb2FkIGFuZCBzdG9yZQorICAgICAgICAvLyBpbnN0cnVjdGlvbnMgaW5zdGVh ZCBvZiB1c2luZyB0aGUgVmFsdWVLZXkgZnVuY3Rpb25hbGl0eSBvciBkb2luZyBERkcgSGVhcExv Y2F0aW9uLWxpa2UKKyAgICAgICAgLy8gdGhpbmdzLgorCisgICAgICAgIC8vIERvbid0IGJvdGhl ciB3aXRoIGlkZW50aXRpZXMuIFdlIGtpbGwgdGhvc2UgYW55d2F5LgorICAgICAgICBpZiAobV92 YWx1ZS0+b3Bjb2RlKCkgPT0gSWRlbnRpdHkpCisgICAgICAgICAgICByZXR1cm47CisKKyAgICAg ICAgVmFsdWVLZXkga2V5ID0gbV92YWx1ZS0+a2V5KCk7CisgICAgICAgIGlmICgha2V5KQorICAg ICAgICAgICAgcmV0dXJuOworICAgICAgICAKKyAgICAgICAgVmVjdG9yPFZhbHVlKiwgMT4mIG1h dGNoZXMgPSBtX3B1cmVWYWx1ZXMuYWRkKGtleSwgVmVjdG9yPFZhbHVlKiwgMT4oKSkuaXRlcmF0 b3ItPnZhbHVlOworCisgICAgICAgIC8vIFJlcGxhY2UgdGhpcyB2YWx1ZSB3aXRoIHdoaWNoZXZl ciB2YWx1ZSBkb21pbmF0ZXMgdXMuCisgICAgICAgIGZvciAoVmFsdWUqIG1hdGNoIDogbWF0Y2hl cykgeworICAgICAgICAgICAgaWYgKG1fZG9taW5hdG9ycy0+ZG9taW5hdGVzKG1hdGNoLT5vd25l ciwgbV92YWx1ZS0+b3duZXIpKSB7CisgICAgICAgICAgICAgICAgbV92YWx1ZS0+cmVwbGFjZVdp dGhJZGVudGl0eShtYXRjaCk7CisgICAgICAgICAgICAgICAgbV9jaGFuZ2VkID0gdHJ1ZTsKKyAg ICAgICAgICAgICAgICByZXR1cm47CisgICAgICAgICAgICB9CisgICAgICAgIH0KKworICAgICAg ICBtYXRjaGVzLmFwcGVuZChtX3ZhbHVlKTsKKyAgICB9CisKICAgICB2b2lkIHNpbXBsaWZ5Q0ZH KCkKICAgICB7CiAgICAgICAgIGlmICh2ZXJib3NlKSB7CkBAIC0xMjEyLDExICsxMjUwLDEzIEBA IHByaXZhdGU6CiAKICAgICBQcm9jZWR1cmUmIG1fcHJvYzsKICAgICBJbnNlcnRpb25TZXQgbV9p bnNlcnRpb25TZXQ7Ci0gICAgQmFzaWNCbG9jayogbV9ibG9jazsKLSAgICB1bnNpZ25lZCBtX2lu ZGV4OwotICAgIFZhbHVlKiBtX3ZhbHVlOwotICAgIGJvb2wgbV9jaGFuZ2VkOwotICAgIGJvb2wg bV9jaGFuZ2VkQ0ZHOworICAgIEJhc2ljQmxvY2sqIG1fYmxvY2sgeyBudWxscHRyIH07CisgICAg dW5zaWduZWQgbV9pbmRleCB7IDAgfTsKKyAgICBWYWx1ZSogbV92YWx1ZSB7IG51bGxwdHIgfTsK KyAgICBEb21pbmF0b3JzKiBtX2RvbWluYXRvcnMgeyBudWxscHRyIH07CisgICAgSGFzaE1hcDxW YWx1ZUtleSwgVmVjdG9yPFZhbHVlKiwgMT4+IG1fcHVyZVZhbHVlczsKKyAgICBib29sIG1fY2hh bmdlZCB7IGZhbHNlIH07CisgICAgYm9vbCBtX2NoYW5nZWRDRkcgeyBmYWxzZSB9OwogfTsKIAog fSAvLyBhbm9ueW1vdXMgbmFtZXNwYWNlCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvYjMv QjNSZWR1Y2VTdHJlbmd0aC5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9i My9CM1JlZHVjZVN0cmVuZ3RoLmgJKHJldmlzaW9uIDE5Mzk0NikKKysrIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9iMy9CM1JlZHVjZVN0cmVuZ3RoLmgJKHdvcmtpbmcgY29weSkKQEAgLTMyLDggKzMy LDExIEBAIG5hbWVzcGFjZSBKU0MgeyBuYW1lc3BhY2UgQjMgewogCiBjbGFzcyBQcm9jZWR1cmU7 CiAKLS8vIERvZXMgc3RyZW5ndGggcmVkdWN0aW9uLCBjb25zdGFudCBmb2xkaW5nLCBjYW5vbmlj YWxpemF0aW9uLCBDRkcgc2ltcGxpZmljYXRpb24sIGFuZCBEQ0UuIEluIHRoZQotLy8gZnV0dXJl IHdlIG1heSBhbHNvIGhhdmUgaXQgZG8gQ1NFLgorLy8gRG9lcyBzdHJlbmd0aCByZWR1Y3Rpb24s IGNvbnN0YW50IGZvbGRpbmcsIGNhbm9uaWNhbGl6YXRpb24sIENGRyBzaW1wbGlmaWNhdGlvbiwg RENFLCBhbmQgQ1NFLiBUaGlzCisvLyBwaGFzZSBydW5zIHRob3NlIG9wdGltaXphdGlvbnMgdG8g Zml4cG9pbnQuIFRoZSBnb2FsIG9mIHRoZSBwaGFzZSBpcyB0byBkcmFtYXRpY2FsbHkgcmVkdWNl IHRoZQorLy8gY29tcGxleGl0eSBvZiB0aGUgY29kZS4gSW4gdGhlIGZ1dHVyZSwgaXQncyBwcmVm ZXJhYmxlIHRvIGFkZCBvcHRpbWl6YXRpb25zIHRvIHRoaXMgcGhhc2UgcmF0aGVyIHRoYW4KKy8v IGNyZWF0aW5nIG5ldyBvcHRpbWl6YXRpb25zIGJlY2F1c2UgdGhlbiB0aGUgb3B0aW1pemF0aW9u cyBjYW4gcGFydGljaXBhdGUgaW4gdGhlIGZpeHBvaW50LiBIb3dldmVyLAorLy8gdGhpcyBwaGFz ZSBzaG91bGRuJ3QgYmVjb21lIHRvbyBleHBlbnNpdmUsIHNvIGV4cGVuc2l2ZSBvcHRpbWl6YXRp b25zIHNob3VsZCBiZSBzZXBhcmF0ZS4KIAogYm9vbCByZWR1Y2VTdHJlbmd0aChQcm9jZWR1cmUm KTsKIApJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2IzL0IzVmFsdWUuY3BwCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9iMy9CM1ZhbHVlLmNwcAkocmV2aXNpb24gMTkz OTQ2KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2IzL0IzVmFsdWUuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC01NCw2ICs1NCwxMyBAQCB2b2lkIFZhbHVlOjpyZXBsYWNlV2l0aElkZW50aXR5KFZh bHVlKiB2CiAgICAgLy8gYSBwbGFpbiBJZGVudGl0eSBWYWx1ZS4gV2UgZmlyc3QgY29sbGVjdCBh bGwgb2YgdGhlIGluZm9ybWF0aW9uIHdlIG5lZWQsIHRoZW4gd2UgZGVzdHJ1Y3QgdGhlCiAgICAg Ly8gcHJldmlvdXMgdmFsdWUgaW4gcGxhY2UsIGFuZCB0aGVuIHdlIGNvbnN0cnVjdCB0aGUgSWRl bnRpdHkgVmFsdWUgaW4gcGxhY2UuCiAKKyAgICBBU1NFUlQobV90eXBlID09IHZhbHVlLT5tX3R5 cGUpOworCisgICAgaWYgKG1fdHlwZSA9PSBWb2lkKSB7CisgICAgICAgIHJlcGxhY2VXaXRoTm9w KCk7CisgICAgICAgIHJldHVybjsKKyAgICB9CisKICAgICB1bnNpZ25lZCBpbmRleCA9IG1faW5k ZXg7CiAgICAgVHlwZSB0eXBlID0gbV90eXBlOwogICAgIE9yaWdpbiBvcmlnaW4gPSBtX29yaWdp bjsKQEAgLTQzOCw2ICs0NDUsNyBAQCBWYWx1ZUtleSBWYWx1ZTo6a2V5KCkgY29uc3QKICAgICBj YXNlIEZsb2F0VG9Eb3VibGU6CiAgICAgY2FzZSBEb3VibGVUb0Zsb2F0OgogICAgIGNhc2UgQ2hl Y2s6CisgICAgY2FzZSBCaXR3aXNlQ2FzdDoKICAgICAgICAgcmV0dXJuIFZhbHVlS2V5KG9wY29k ZSgpLCB0eXBlKCksIGNoaWxkKDApKTsKICAgICBjYXNlIEFkZDoKICAgICBjYXNlIFN1YjoK