Bug 205906

Summary: AI rule for ValueMod/ValueDiv produce constants with the wrong format when the result can be an int32
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, guijemont, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch none

Saam Barati
Reported 2020-01-07 21:20:18 PST
...
Attachments
patch (4.90 KB, patch)
2020-01-07 21:30 PST, Saam Barati 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2020-01-07 21:20:53 PST
<rdar://problem/56108519>
Saam Barati
Comment 2 2020-01-07 21:30:55 PST
Created attachment 387069 [details] patch
Yusuke Suzuki
Comment 3 2020-01-07 21:35:55 PST
Comment on attachment 387069 [details] patch r=me
WebKit Commit Bot
Comment 4 2020-01-07 23:23:38 PST
Comment on attachment 387069 [details] patch Clearing flags on attachment: 387069 Committed r254188: <https://trac.webkit.org/changeset/254188>
WebKit Commit Bot
Comment 5 2020-01-07 23:23:39 PST
All reviewed patches have been landed. Closing bug.
Caio Lima
Comment 6 2020-01-08 01:46:46 PST
Comment on attachment 387069 [details] 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.
Saam Barati
Comment 7 2020-01-08 18:09:35 PST
Comment on attachment 387069 [details] 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.
Saam Barati
Comment 8 2020-01-08 18:12:29 PST
Comment on attachment 387069 [details] 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.
Saam Barati
Comment 9 2020-01-08 18:32:59 PST
Comment on attachment 387069 [details] 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
Note You need to log in before you can comment on or make changes to this bug.