Bug 210870 - [JSC] branchIfBigInt32 can use BigInt32Mask and remove branchIfNumber filter
Summary: [JSC] branchIfBigInt32 can use BigInt32Mask and remove branchIfNumber filter
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-22 12:12 PDT by Yusuke Suzuki
Modified: 2020-04-22 20:12 PDT (History)
8 users (show)

See Also:

Attachments
Patch (17.58 KB, patch)
2020-04-22 12:15 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (17.58 KB, patch)
2020-04-22 12:38 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (19.67 KB, patch)
2020-04-22 13:13 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (20.93 KB, patch)
2020-04-22 13:56 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (20.93 KB, patch)
2020-04-22 14:09 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (21.11 KB, patch)
2020-04-22 15:07 PDT, Yusuke Suzuki 		saam: review+
Details | Formatted Diff | Diff
Show Obsolete (5) 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-22 12:12:03 PDT 
[JSC] branchIfBigInt32 can use BigInt32Mask and remove branchIfNumber filter
Comment 1 Yusuke Suzuki 2020-04-22 12:15:01 PDT 
Created attachment 397232 [details]
Patch
Comment 2 Robin Morisset 2020-04-22 12:36:20 PDT 
Comment on attachment 397232 [details]
Patch

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

> Source/JavaScriptCore/jit/AssemblyHelpers.h:938
> +        and64(TrustedImm32(JSValue::BigInt32Mask), tempGPR);

It cannot be TrustedImm32, since it is a 64 bit mask. It has to be materialized one way or another.
Comment 3 Yusuke Suzuki 2020-04-22 12:38:33 PDT 
Created attachment 397238 [details]
Patch
Comment 4 Yusuke Suzuki 2020-04-22 12:40:06 PDT 
Comment on attachment 397232 [details]
Patch

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

>> Source/JavaScriptCore/jit/AssemblyHelpers.h:938
>> +        and64(TrustedImm32(JSValue::BigInt32Mask), tempGPR);
> 
> It cannot be TrustedImm32, since it is a 64 bit mask. It has to be materialized one way or another.

Fixed.
Comment 5 Yusuke Suzuki 2020-04-22 13:13:52 PDT 
Created attachment 397244 [details]
Patch
Comment 6 Yusuke Suzuki 2020-04-22 13:23:07 PDT 
Comment on attachment 397244 [details]
Patch

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

> Source/JavaScriptCore/jit/AssemblyHelpers.h:934
> +    Jump branchIfBigInt32(GPRReg gpr, GPRReg tempGPR)

I would extend it further by using number-tag-register later.

if (mode == HaveTagRegisters) {
move(GPRInfo::numberTagRegister, tempGPR);
add64(TrustedImm32(JSValue::BigInt32Tag), tempGPR);
and64(gpr, tempGPR);
return branch64(Equal, tempGPR, TrustedImm32(JSValue::BigInt32Tag));
}
Comment 7 Yusuke Suzuki 2020-04-22 13:26:38 PDT 
Comment on attachment 397244 [details]
Patch

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

>> Source/JavaScriptCore/jit/AssemblyHelpers.h:934
>> +    Jump branchIfBigInt32(GPRReg gpr, GPRReg tempGPR)
> 
> I would extend it further by using number-tag-register later.
> 
> if (mode == HaveTagRegisters) {
> move(GPRInfo::numberTagRegister, tempGPR);
> add64(TrustedImm32(JSValue::BigInt32Tag), tempGPR);
> and64(gpr, tempGPR);
> return branch64(Equal, tempGPR, TrustedImm32(JSValue::BigInt32Tag));
> }

Or further,

if (mode == HaveTagRegisters) {
    add64(TrustedImm32(JSValue::BigInt32Tag), GPRInfo::numberTagRegister, tempGPR);
    and64(gpr, tempGPR);
    return branch64(Equal, tempGPR, TrustedImm32(JSValue::BigInt32Tag));
}
Comment 8 Yusuke Suzuki 2020-04-22 13:27:35 PDT 
Comment on attachment 397244 [details]
Patch

I'll do it.
Comment 9 Saam Barati 2020-04-22 13:31:29 PDT 
Comment on attachment 397244 [details]
Patch

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

>>> Source/JavaScriptCore/jit/AssemblyHelpers.h:934
>>> +    Jump branchIfBigInt32(GPRReg gpr, GPRReg tempGPR)
>> 
>> I would extend it further by using number-tag-register later.
>> 
>> if (mode == HaveTagRegisters) {
>> move(GPRInfo::numberTagRegister, tempGPR);
>> add64(TrustedImm32(JSValue::BigInt32Tag), tempGPR);
>> and64(gpr, tempGPR);
>> return branch64(Equal, tempGPR, TrustedImm32(JSValue::BigInt32Tag));
>> }
> 
> Or further,
> 
> if (mode == HaveTagRegisters) {
>     add64(TrustedImm32(JSValue::BigInt32Tag), GPRInfo::numberTagRegister, tempGPR);
>     and64(gpr, tempGPR);
>     return branch64(Equal, tempGPR, TrustedImm32(JSValue::BigInt32Tag));
> }

Let's also static assert that this produce the same value in C++
Comment 10 Yusuke Suzuki 2020-04-22 13:56:56 PDT 
Created attachment 397255 [details]
Patch
Comment 11 Yusuke Suzuki 2020-04-22 14:09:32 PDT 
Created attachment 397259 [details]
Patch
Comment 12 Yusuke Suzuki 2020-04-22 15:07:24 PDT 
Created attachment 397271 [details]
Patch
Comment 13 Yusuke Suzuki 2020-04-22 20:11:08 PDT 
Committed r260551: <https://trac.webkit.org/changeset/260551>
Comment 14 Radar WebKit Bug Importer 2020-04-22 20:12:20 PDT 
<rdar://problem/62228618>