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

Description Saam Barati 2020-01-07 21:20:18 PST 
...
Comment 1 Saam Barati 2020-01-07 21:20:53 PST 
<rdar://problem/56108519>
Comment 2 Saam Barati 2020-01-07 21:30:55 PST 
Created attachment 387069 [details]
patch
Comment 3 Yusuke Suzuki 2020-01-07 21:35:55 PST 
Comment on attachment 387069 [details]
patch

r=me
Comment 4 WebKit Commit Bot 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>
Comment 5 WebKit Commit Bot 2020-01-07 23:23:39 PST 
All reviewed patches have been landed.  Closing bug.
Comment 6 Caio Lima 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.
Comment 7 Saam Barati 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.
Comment 8 Saam Barati 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.
Comment 9 Saam Barati 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