Bug 210839 - [JSC] AI results of BigInt32 Bitwise shift operation does not match to runtime results
Summary: [JSC] AI results of BigInt32 Bitwise shift operation does not match to runtim...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-21 19:16 PDT by Yusuke Suzuki
Modified: 2020-04-22 08:56 PDT (History)
7 users (show)

See Also:

Attachments
Patch (610.08 KB, patch)
2020-04-21 22:52 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (610.98 KB, patch)
2020-04-21 23:16 PDT, Yusuke Suzuki 		saam: review+
Details | Formatted Diff | Diff
Patch for landing (610.35 KB, patch)
2020-04-21 23:36 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Yusuke Suzuki 2020-04-21 19:16:30 PDT 
...
Comment 1 Yusuke Suzuki 2020-04-21 22:18:03 PDT 
AI says it is BigInt32, but at runtime, we are returning HeapBigInt!
Comment 2 Yusuke Suzuki 2020-04-21 22:52:48 PDT 
Created attachment 397169 [details]
Patch
Comment 3 Yusuke Suzuki 2020-04-21 23:16:09 PDT 
Created attachment 397170 [details]
Patch
Comment 4 Saam Barati 2020-04-21 23:21:19 PDT 
Comment on attachment 397170 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397170&action=review

> Source/JavaScriptCore/ChangeLog:12
> +        And we found that FTL BigInt32 predicate is not correctly checking state. This patch fixes it too.

You should say what the test for this was. A JSValue number might have any lower bits set.

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:546
> +            // FIXME: We should have inlined implementation that always returns BigInt32.

link to a bug

> Source/JavaScriptCore/jit/JITOperations.cpp:3152
> +void JIT_OPERATION operationDataLog(EncodedJSValue value)
> +{
> +    dataLogLn(JSValue::decode(value));
> +}

why? Let's remove IMO
Comment 5 Yusuke Suzuki 2020-04-21 23:25:55 PDT 
Comment on attachment 397170 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397170&action=review

>> Source/JavaScriptCore/ChangeLog:12
>> +        And we found that FTL BigInt32 predicate is not correctly checking state. This patch fixes it too.
> 
> You should say what the test for this was. A JSValue number might have any lower bits set.

Yes, I'm hitting this bug with JSTests/stress/v8-bigint32-sar.js's CompareStrictEq.

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:546
>> +            // FIXME: We should have inlined implementation that always returns BigInt32.
> 
> link to a bug

Fixed.

>> Source/JavaScriptCore/jit/JITOperations.cpp:3152
>> +}
> 
> why? Let's remove IMO

Removed.
Comment 6 Yusuke Suzuki 2020-04-21 23:36:52 PDT 
Created attachment 397172 [details]
Patch for landing
Comment 7 Yusuke Suzuki 2020-04-22 08:40:15 PDT 
Committed r260512: <https://trac.webkit.org/changeset/260512>
Comment 8 Radar WebKit Bug Importer 2020-04-22 08:41:16 PDT 
<rdar://problem/62187118>
Comment 9 Yusuke Suzuki 2020-04-22 08:56:41 PDT 
https://bugs.webkit.org/show_bug.cgi?id=210860