Bug 205906 - AI rule for ValueMod/ValueDiv produce constants with the wrong format when the result can be an int32
Summary: AI rule for ValueMod/ValueDiv produce constants with the wrong format when th...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-07 21:20 PST by Saam Barati
Modified: 2020-01-08 18:32 PST (History)
14 users (show)

See Also:

Attachments
patch (4.90 KB, patch)
2020-01-07 21:30 PST, Saam Barati 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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