Bug 227201 – Add a new pattern to instruction selector to use BFI supported by ARM64

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yijia Huang]

...

[Yijia Huang]

Created attachment 432140 [details]
Patch

[Filip Pizlo]

Comment on attachment 432140 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:28
> +        Pattern 2:
> +            mask1 = (1 << width) - 1
> +            mask2 = ~(mask1 << lsb)
> +            d = ((n & mask1) << lsb) | (d & mask2)
> +
> +        Pattern 3:
> +            mask1 = (1 << width) - 1
> +            d = ((n & mask1) << lsb) | d

Is this right?

Say that d = 0x42424242
Say that mask1 = 0xFF
Say that mask2 = 0xFF00FFFF
Say that lsb = 16
Say that n = 0xBC

Note that 0x42 | 0xBC = 0xFE

With pattern 2, we will get:

((0xBC & 0xFF) << 16) | (0x42424242 & 0xFF00FFFF)
= 0xBC0000 | 0x42004242
= 0x42BC4242

With pattern 3, we will get:

((0xBC & 0xFF) << 16) | 0x42424242
= 0xBC0000 | 0x42424242
= 0x42FE4242

I don't think they're equivalent patterns.

[Yijia Huang]

Created attachment 432184 [details]
Patch

[Yijia Huang]

Created attachment 432185 [details]
Patch

[Yijia Huang]

(In reply to Filip Pizlo from comment #2)
> Comment on attachment 432140 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=432140&action=review
> 
> > Source/JavaScriptCore/ChangeLog:28
> > +        Pattern 2:
> > +            mask1 = (1 << width) - 1
> > +            mask2 = ~(mask1 << lsb)
> > +            d = ((n & mask1) << lsb) | (d & mask2)
> > +
> > +        Pattern 3:
> > +            mask1 = (1 << width) - 1
> > +            d = ((n & mask1) << lsb) | d
> 
> Is this right?
> 
> Say that d = 0x42424242
> Say that mask1 = 0xFF
> Say that mask2 = 0xFF00FFFF
> Say that lsb = 16
> Say that n = 0xBC
> 
> Note that 0x42 | 0xBC = 0xFE
> 
> With pattern 2, we will get:
> 
> ((0xBC & 0xFF) << 16) | (0x42424242 & 0xFF00FFFF)
> = 0xBC0000 | 0x42004242
> = 0x42BC4242
> 
> With pattern 3, we will get:
> 
> ((0xBC & 0xFF) << 16) | 0x42424242
> = 0xBC0000 | 0x42424242
> = 0x42FE4242
> 
> I don't think they're equivalent patterns.

Thanks. I have fixed that.

[Yijia Huang]

Created attachment 432205 [details]
Patch

[EWS]

Committed r279249 (239133@main): <https://commits.webkit.org/239133@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432205 [details].

[Yusuke Suzuki]

Committed r279253 (239137@main): <https://commits.webkit.org/239137@main>