205906 2020-01-07 21:20:18 -0800 AI rule for ValueMod/ValueDiv produce constants with the wrong format when the result can be an int32 2020-01-08 18:32:59 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 saam saam benjamin commit-queue fpizlo ggaren gskachkov guijemont keith_miller mark.lam msaboff rmorisset ticaiolima tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1603937 0 saam 2020-01-07 21:20:18 -0800 ... 1603938 1 saam 2020-01-07 21:20:53 -0800 <rdar://problem/56108519> 1603940 2 387069 saam 2020-01-07 21:30:55 -0800 Created attachment 387069 patch 1603943 3 387069 ysuzuki 2020-01-07 21:35:55 -0800 Comment on attachment 387069 patch r=me 1603976 4 387069 commit-queue 2020-01-07 23:23:38 -0800 Comment on attachment 387069 patch Clearing flags on attachment: 387069 Committed r254188: <https://trac.webkit.org/changeset/254188> 1603977 5 commit-queue 2020-01-07 23:23:39 -0800 All reviewed patches have been landed. Closing bug. 1604005 6 387069 ticaiolima 2020-01-08 01:46:46 -0800 Comment on attachment 387069 patch View in context: https://bugs.webkit.org/attachment.cgi?id=387069&action=review > JSTests/stress/ai-value-mod-should-result-in-constant-int-where-possible.js:14 > + const c = z / 1.0 + 0; I think you meant "%" here. > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:330 > + setConstant(node, jsDoubleNumber(left.asNumber() / right.asNumber())); How does this issue not affect `ArithDiv/ArithMod` as well? IIUC, we can have the same scenario for ArithDiv/ArithMod. 1604437 7 387069 saam 2020-01-08 18:09:35 -0800 Comment on attachment 387069 patch View in context: https://bugs.webkit.org/attachment.cgi?id=387069&action=review >> JSTests/stress/ai-value-mod-should-result-in-constant-int-where-possible.js:14 >> + const c = z / 1.0 + 0; > > I think you meant "%" here. yes. will fix. 1604439 8 387069 saam 2020-01-08 18:12:29 -0800 Comment on attachment 387069 patch View in context: https://bugs.webkit.org/attachment.cgi?id=387069&action=review >> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:330 >> + setConstant(node, jsDoubleNumber(left.asNumber() / right.asNumber())); > > How does this issue not affect `ArithDiv/ArithMod` as well? IIUC, we can have the same scenario for ArithDiv/ArithMod. No, it's not the same. If you read the code for ArithDiv/ArithMod over Double binary use kind, they both return the value in double format. 1604462 9 387069 saam 2020-01-08 18:32:59 -0800 Comment on attachment 387069 patch View in context: https://bugs.webkit.org/attachment.cgi?id=387069&action=review >>> JSTests/stress/ai-value-mod-should-result-in-constant-int-where-possible.js:14 >>> + const c = z / 1.0 + 0; >> >> I think you meant "%" here. > > yes. will fix. thanks for calling this out. Fixed in: https://trac.webkit.org/changeset/254247/webkit 387069 2020-01-07 21:30:55 -0800 2020-01-07 23:23:38 -0800 patch c-backup.diff text/plain 5019 saam SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNTQxODYpCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDIwIEBACisyMDIwLTAxLTA3ICBTYWFtIEJhcmF0aSAgPHNiYXJhdGlAYXBwbGUu Y29tPgorCisgICAgICAgIEFJIHJ1bGUgZm9yIFZhbHVlTW9kL1ZhbHVlRGl2IHByb2R1Y2UgY29u c3RhbnRzIHdpdGggdGhlIHdyb25nIGZvcm1hdCB3aGVuIHRoZSByZXN1bHQgY2FuIGJlIGFuIGlu dDMyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDU5 MDYKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzU2MTA4NTE5PgorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogc3RyZXNzL2FpLXZhbHVlLWRpdi1zaG91 bGQtcmVzdWx0LWluLWNvbnN0YW50LWludC13aGVyZS1wb3NzaWJsZS5qczogQWRkZWQuCisgICAg ICAgIChmb28uYmFyLmYpOgorICAgICAgICAoZm9vLik6CisgICAgICAgIChmb28pOgorICAgICAg ICAqIHN0cmVzcy9haS12YWx1ZS1tb2Qtc2hvdWxkLXJlc3VsdC1pbi1jb25zdGFudC1pbnQtd2hl cmUtcG9zc2libGUuanM6IEFkZGVkLgorICAgICAgICAoZm9vLmJhci5mKToKKyAgICAgICAgKGZv by4pOgorICAgICAgICAoZm9vKToKKwogMjAyMC0wMS0wNiAgQWxleGV5IFNodmF5a2EgIDxzaHZh aWthbGVzaEBnbWFpbC5jb20+CiAKICAgICAgICAgU3RyaW5nLnByb3RvdHlwZS5yZXBsYWNlKCkg aW5jb3JyZWN0bHkgaGFuZGxlcyBuYW1lZCByZWZlcmVuY2VzIG9uIFJlZ0V4cCB3L28gbmFtZWQg Z3JvdXBzCkluZGV4OiBKU1Rlc3RzL3N0cmVzcy9haS12YWx1ZS1kaXYtc2hvdWxkLXJlc3VsdC1p bi1jb25zdGFudC1pbnQtd2hlcmUtcG9zc2libGUuanMKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSlNUZXN0cy9z dHJlc3MvYWktdmFsdWUtZGl2LXNob3VsZC1yZXN1bHQtaW4tY29uc3RhbnQtaW50LXdoZXJlLXBv c3NpYmxlLmpzCShub25leGlzdGVudCkKKysrIEpTVGVzdHMvc3RyZXNzL2FpLXZhbHVlLWRpdi1z aG91bGQtcmVzdWx0LWluLWNvbnN0YW50LWludC13aGVyZS1wb3NzaWJsZS5qcwkod29ya2luZyBj b3B5KQpAQCAtMCwwICsxLDIxIEBACisvL0AgcnVuRGVmYXVsdCgiLS11c2VSYW5kb21pemluZ0Z1 enplckFnZW50PTEiLCAiLS12YWxpZGF0ZUFic3RyYWN0SW50ZXJwcmV0ZXJTdGF0ZT0xIiwgIi0t aml0UG9saWN5U2NhbGU9MCIsICItLXVzZUNvbmN1cnJlbnRKSVQ9MCIpCisKK2Z1bmN0aW9uIGZv bygpIHsKKyAgICBmb3IgKGxldCBpID0gMDsgaSA8IDM7IGkrKykgeworICAgICAgICBjb25zdCBv ID0ge307CisgICAgICAgIGZ1bmN0aW9uIGJhcihhMCkgeworICAgICAgICAgICAgbGV0IHg7Cisg ICAgICAgICAgICBkbyB7CisgICAgICAgICAgICAgICAgZnVuY3Rpb24gZigpIHsgejsgfQorICAg ICAgICAgICAgICAgIHggPSBvOworICAgICAgICAgICAgICAgIGNvbnN0IHkgPSB0eXBlb2YgeCA9 PT0gYTA7CisgICAgICAgICAgICAgICAgW2EwLCAwLjFdOworICAgICAgICAgICAgICAgIGNvbnN0 IHogPSAwICsgeTsKKyAgICAgICAgICAgICAgICBjb25zdCBjID0geiAvIDEuMCArIDA7CisgICAg ICAgICAgICB9IHdoaWxlICgheCk7CisgICAgICAgIH0KKyAgICAgICAgYmFyKDApOworICAgIH0K K30KKworZm9vKCk7CkluZGV4OiBKU1Rlc3RzL3N0cmVzcy9haS12YWx1ZS1tb2Qtc2hvdWxkLXJl c3VsdC1pbi1jb25zdGFudC1pbnQtd2hlcmUtcG9zc2libGUuanMKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSlNU ZXN0cy9zdHJlc3MvYWktdmFsdWUtbW9kLXNob3VsZC1yZXN1bHQtaW4tY29uc3RhbnQtaW50LXdo ZXJlLXBvc3NpYmxlLmpzCShub25leGlzdGVudCkKKysrIEpTVGVzdHMvc3RyZXNzL2FpLXZhbHVl LW1vZC1zaG91bGQtcmVzdWx0LWluLWNvbnN0YW50LWludC13aGVyZS1wb3NzaWJsZS5qcwkod29y a2luZyBjb3B5KQpAQCAtMCwwICsxLDIxIEBACisvL0AgcnVuRGVmYXVsdCgiLS11c2VSYW5kb21p emluZ0Z1enplckFnZW50PTEiLCAiLS12YWxpZGF0ZUFic3RyYWN0SW50ZXJwcmV0ZXJTdGF0ZT0x IiwgIi0taml0UG9saWN5U2NhbGU9MCIsICItLXVzZUNvbmN1cnJlbnRKSVQ9MCIpCisKK2Z1bmN0 aW9uIGZvbygpIHsKKyAgICBmb3IgKGxldCBpID0gMDsgaSA8IDM7IGkrKykgeworICAgICAgICBj b25zdCBvID0ge307CisgICAgICAgIGZ1bmN0aW9uIGJhcihhMCkgeworICAgICAgICAgICAgbGV0 IHg7CisgICAgICAgICAgICBkbyB7CisgICAgICAgICAgICAgICAgZnVuY3Rpb24gZigpIHsgejsg fQorICAgICAgICAgICAgICAgIHggPSBvOworICAgICAgICAgICAgICAgIGNvbnN0IHkgPSB0eXBl b2YgeCA9PT0gYTA7CisgICAgICAgICAgICAgICAgW2EwLCAwLjFdOworICAgICAgICAgICAgICAg IGNvbnN0IHogPSAwICsgeTsKKyAgICAgICAgICAgICAgICBjb25zdCBjID0geiAvIDEuMCArIDA7 CisgICAgICAgICAgICB9IHdoaWxlICgheCk7CisgICAgICAgIH0KKyAgICAgICAgYmFyKDApOwor ICAgIH0KK30KKworZm9vKCk7CkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9n Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9u IDI1NDE4MikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cJKHdvcmtpbmcgY29w eSkKQEAgLTEsMyArMSwxOCBAQAorMjAyMC0wMS0wNyAgU2FhbSBCYXJhdGkgIDxzYmFyYXRpQGFw cGxlLmNvbT4KKworICAgICAgICBBSSBydWxlIGZvciBWYWx1ZU1vZC9WYWx1ZURpdiBwcm9kdWNl IGNvbnN0YW50cyB3aXRoIHRoZSB3cm9uZyBmb3JtYXQgd2hlbiB0aGUgcmVzdWx0IGNhbiBiZSBh biBpbnQzMgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MjA1OTA2CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS81NjEwODUxOT4KKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgcnVudGltZSBjb2RlIGZvciBW YWx1ZU1vZCBhbmQgVmFsdWVEaXYgcHJvZHVjZXMgYW4gaW50MzIgd2hlbiB0aGUgcmVzdWx0Cisg ICAgICAgIGlzIG9mIGludDMyIHZhbHVlLiBIb3dldmVyLCB0aGUgQUkgd2FzIHNheWluZyB0aGUg cmVzdWx0IGlzIGluIGRvdWJsZSBmb3JtYXQuCisgICAgICAgIFRoaXMgcGF0Y2ggZml4ZXMgQUkg dG8gcHJvZHVjZSBhIEpTVmFsdWUgaW4gdGhlIHJpZ2h0IGZvcm1hdC4KKworICAgICAgICAqIGRm Zy9ERkdBYnN0cmFjdEludGVycHJldGVySW5saW5lcy5oOgorICAgICAgICAoSlNDOjpERkc6OkFi c3RyYWN0SW50ZXJwcmV0ZXI8QWJzdHJhY3RTdGF0ZVR5cGU+OjpoYW5kbGVDb25zdGFudERpdk9w KToKKwogMjAyMC0wMS0wNyAgUm9zcyBLaXJzbGluZyAgPHJvc3Mua2lyc2xpbmdAc29ueS5jb20+ CiAKICAgICAgICAgVW5yZXZpZXdlZCByZXN0YWJpbGl6YXRpb24gb2Ygbm9uLXVuaWZpZWQgYnVp bGQuCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0Fic3RyYWN0SW50ZXJwcmV0 ZXJJbmxpbmVzLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdB YnN0cmFjdEludGVycHJldGVySW5saW5lcy5oCShyZXZpc2lvbiAyNTQxODIpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvZGZnL0RGR0Fic3RyYWN0SW50ZXJwcmV0ZXJJbmxpbmVzLmgJKHdvcmtp bmcgY29weSkKQEAgLTMyMywxMCArMzIzLDE3IEBAIGJvb2wgQWJzdHJhY3RJbnRlcnByZXRlcjxB YnN0cmFjdFN0YXRlVHkKICAgICAgICAgICAgIGlmIChpc0Nsb2JiZXJpbmcpCiAgICAgICAgICAg ICAgICAgZGlkRm9sZENsb2JiZXJXb3JsZCgpOwogCi0gICAgICAgICAgICBpZiAoaXNEaXZPcGVy YXRpb24pCi0gICAgICAgICAgICAgICAgc2V0Q29uc3RhbnQobm9kZSwganNEb3VibGVOdW1iZXIo bGVmdC5hc051bWJlcigpIC8gcmlnaHQuYXNOdW1iZXIoKSkpOwotICAgICAgICAgICAgZWxzZQot ICAgICAgICAgICAgICAgIHNldENvbnN0YW50KG5vZGUsIGpzRG91YmxlTnVtYmVyKGZtb2QobGVm dC5hc051bWJlcigpLCByaWdodC5hc051bWJlcigpKSkpOworICAgICAgICAgICAgaWYgKGlzRGl2 T3BlcmF0aW9uKSB7CisgICAgICAgICAgICAgICAgaWYgKG9wID09IFZhbHVlRGl2KQorICAgICAg ICAgICAgICAgICAgICBzZXRDb25zdGFudChub2RlLCBqc051bWJlcihsZWZ0LmFzTnVtYmVyKCkg LyByaWdodC5hc051bWJlcigpKSk7CisgICAgICAgICAgICAgICAgZWxzZQorICAgICAgICAgICAg ICAgICAgICBzZXRDb25zdGFudChub2RlLCBqc0RvdWJsZU51bWJlcihsZWZ0LmFzTnVtYmVyKCkg LyByaWdodC5hc051bWJlcigpKSk7CisgICAgICAgICAgICB9IGVsc2UgeworICAgICAgICAgICAg ICAgIGlmIChvcCA9PSBWYWx1ZU1vZCkKKyAgICAgICAgICAgICAgICAgICAgc2V0Q29uc3RhbnQo bm9kZSwganNOdW1iZXIoZm1vZChsZWZ0LmFzTnVtYmVyKCksIHJpZ2h0LmFzTnVtYmVyKCkpKSk7 CisgICAgICAgICAgICAgICAgZWxzZQorICAgICAgICAgICAgICAgICAgICBzZXRDb25zdGFudChu b2RlLCBqc0RvdWJsZU51bWJlcihmbW9kKGxlZnQuYXNOdW1iZXIoKSwgcmlnaHQuYXNOdW1iZXIo KSkpKTsKKyAgICAgICAgICAgIH0KIAogICAgICAgICAgICAgcmV0dXJuIHRydWU7CiAgICAgICAg IH0K