193094 2019-01-02 17:10:33 -0800 DFG IntegerRangeOptimization phase exceeding loop limit shouldn't ASSERT 2022-02-27 23:29:21 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 msaboff msaboff ews-watchlist keith_miller mark.lam saam webkit-bug-importer oldest_to_newest 1491326 0 msaboff 2019-01-02 17:10:33 -0800 The current value of 50 for giveUpThreshold in DFGIntegerRangeOptimizationPhase.cpp is somewhat arbitrary. It works for all our current tests, including benchmarks with real world code. One can construct test cases that will exceed the threshold. For example the code: const theNumber100 = 100; function foo() { for (var i = 0; i < 1000; ++i) { switch (i + 1000) { case 0: case 2: case 23: case 26: case 29: case 32: case 35: case 38: case 41: case 44: case 46: case 49: case 52: case 55: case 58: case 61: case theNumber100: break; } } } Due to the sequence of compare & branch byte code generated due to the const, this code takes 53 loop iterations to converge. Add a few more case statements and the loop count grows higher. B3 has optimizations to handle this kind of compare and branch code even without running the IntegerRangeOptimization phase. 1491327 1 msaboff 2019-01-02 17:11:09 -0800 <rdar://problem/45838655> 1491338 2 358230 msaboff 2019-01-02 17:30:41 -0800 Created attachment 358230 Patch 1491360 3 358230 saam 2019-01-02 19:17:29 -0800 Comment on attachment 358230 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358230&action=review > Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp:1098 > // If you hit this assertion for a legitimate case, update the giveUpThreshold > // to the smallest values that converges. There is no more assertion. 1491448 4 358230 msaboff 2019-01-03 09:55:12 -0800 Comment on attachment 358230 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358230&action=review >> Source/JavaScriptCore/dfg/DFGIntegerRangeOptimizationPhase.cpp:1098 >> // to the smallest values that converges. > > There is no more assertion. Fixed. 1491452 5 msaboff 2019-01-03 09:58:34 -0800 Committed r239595: <https://trac.webkit.org/changeset/239595> 358230 2019-01-02 17:30:41 -0800 2022-02-27 23:29:21 -0800 Patch 193094.patch text/plain 2325 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjM5NTgyKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBA CisyMDE5LTAxLTAyICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIERGRyBJbnRlZ2VyUmFuZ2VPcHRpbWl6YXRpb24gcGhhc2UgZXhjZWVkaW5nIGxvb3AgbGlt aXQgc2hvdWxkbid0IEFTU0VSVAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTkzMDk0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgUmVtb3ZlZCB0aGlzIGRlYnVnIG9ubHkgQVNTRVJUIGFzIG9uZSBjYW4gY29u c3RydWN0IHRlc3QgY2FzZXMgdGhhdCB3aWxsIGV4Y2VlZCB0aGUgZ2l2ZVVwVGhyZXNob2xkCisg ICAgICAgIGFtb3VudC4gICBUaGlzIGNhbiBiZSBkb25lIHdpdGggYSBsYXJnZSBzd2l0Y2ggc3Rh dGVtZW50IHdpdGggYXQgbGVhc3Qgb25lIGNvbnN0IG9yIHZhcmlhYmxlIGNhc2UKKyAgICAgICAg Y2xhdXNlLiAgU3VjaCBjb2RlIGJ5dGVjb21waWxlcyB0byBjb21wYXJlIC8ganRydWUgc2VxdWVu Y2VzLiAgSW5jcmVhc2luZyB0aGUgZ2l2ZVVwVGhyZXNob2xkIGNvdW50CisgICAgICAgIGRvZXNu J3QgaGVscCB3aXRoIHRoZSBldmVudHVhbCBjb2RlIGdlbmVyYXRlZCBhcyBCMyBoYXMgb3B0aW1p emF0aW9ucyB0byBjb2FsZXNjZSBjb21wYXJlIC8gYnJhbmNoCisgICAgICAgIGNvZGUgc2VxdWVu Y2VzIGV2ZW4gd2hlbiB3ZSBkb24ndCBydW4gdGhlIEludGVnZXJSYW5nZU9wdGltaXphdGlvbiBw aGFzZS4KKworICAgICAgICAqIGRmZy9ERkdJbnRlZ2VyUmFuZ2VPcHRpbWl6YXRpb25QaGFzZS5j cHA6CisKIDIwMTgtMTItMzEgIEtlaXRoIE1pbGxlciAgPGtlaXRoX21pbGxlckBhcHBsZS5jb20+ CiAKICAgICAgICAgU291cmNlUHJvdmlkZXJzIHNob3VsZCB1c2UgYW4gYWN0dWFsIFVSTCBpbnN0 ZWFkIG9mIGEgc3RyaW5nCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0ludGVn ZXJSYW5nZU9wdGltaXphdGlvblBoYXNlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNj cmlwdENvcmUvZGZnL0RGR0ludGVnZXJSYW5nZU9wdGltaXphdGlvblBoYXNlLmNwcAkocmV2aXNp b24gMjM5NTcxKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdJbnRlZ2VyUmFuZ2VP cHRpbWl6YXRpb25QaGFzZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEwOTIsMTMgKzEwOTIsMTIg QEAgcHVibGljOgogICAgICAgICAgICAgKyttX2l0ZXJhdGlvbnM7CiAgICAgICAgICAgICBpZiAo bV9pdGVyYXRpb25zID49IGdpdmVVcFRocmVzaG9sZCkgewogICAgICAgICAgICAgICAgIC8vIFRo aXMgY2FzZSBpcyBub3QgbmVjZXNzYXJpbHkgd3JvbmcgYnV0IGl0IGNhbiBiZSBhIHNpZ24gdGhh dCB0aGlzIHBoYXNlCi0gICAgICAgICAgICAgICAgLy8gZG9lcyBub3QgY29udmVyZ2UuCisgICAg ICAgICAgICAgICAgLy8gZG9lcyBub3QgY29udmVyZ2UuIFRoZSB2YWx1ZSBnaXZlVXBUaHJlc2hv bGQgd2FzIGNob3NlbiBlbXBlcmljYWxseSBiYXNlZCBvbgorICAgICAgICAgICAgICAgIC8vIGN1 cnJlbnQgdGVzdHMgYW5kIHJlYWwgd29ybGQgSlMuCiAgICAgICAgICAgICAgICAgLy8gSWYgeW91 IGhpdCB0aGlzIGFzc2VydGlvbiBmb3IgYSBsZWdpdGltYXRlIGNhc2UsIHVwZGF0ZSB0aGUgZ2l2 ZVVwVGhyZXNob2xkCiAgICAgICAgICAgICAgICAgLy8gdG8gdGhlIHNtYWxsZXN0IHZhbHVlcyB0 aGF0IGNvbnZlcmdlcy4KLSAgICAgICAgICAgICAgICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsKIAot ICAgICAgICAgICAgICAgIC8vIEluIHJlbGVhc2UsIGRvIG5vdCByaXNrIGhvbGRpbmcgdGhlIHRo cmVhZCBmb3IgdG9vIGxvbmcgc2luY2UgdGhpcyBwaGFzZQotICAgICAgICAgICAgICAgIC8vIGlz IHJlYWxseSBzbG93LgorICAgICAgICAgICAgICAgIC8vIERvIG5vdCByaXNrIGhvbGRpbmcgdGhl IHRocmVhZCBmb3IgdG9vIGxvbmcgc2luY2UgdGhpcyBwaGFzZSBpcyByZWFsbHkgc2xvdy4KICAg ICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CiAgICAgICAgICAgICB9CiAK