Bug 210870 – [JSC] branchIfBigInt32 can use BigInt32Mask and remove branchIfNumber filter

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] branchIfBigInt32 can use BigInt32Mask and remove branchIfNumber filter

[Yusuke Suzuki]

Created attachment 397232 [details]
Patch

[Robin Morisset]

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.

[Yusuke Suzuki]

Created attachment 397238 [details]
Patch

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Created attachment 397244 [details]
Patch

[Yusuke Suzuki]

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));
}

[Yusuke Suzuki]

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));
}

[Yusuke Suzuki]

Comment on attachment 397244 [details]
Patch

I'll do it.

[Saam Barati]

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++

[Yusuke Suzuki]

Created attachment 397255 [details]
Patch

[Yusuke Suzuki]

Created attachment 397259 [details]
Patch

[Yusuke Suzuki]

Created attachment 397271 [details]
Patch

[Yusuke Suzuki]

Committed r260551: <https://trac.webkit.org/changeset/260551>

[Radar WebKit Bug Importer]

<rdar://problem/62228618>