Bug 227201

Summary: Add a new pattern to instruction selector to use BFI supported by ARM64
Product: WebKit Reporter: Yijia Huang <yijia_huang>
Component: JavaScriptCoreAssignee: Yijia Huang <yijia_huang>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, kdaveport7, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Yijia Huang 2021-06-20 21:51:58 PDT 
...
Comment 1 Yijia Huang 2021-06-24 01:50:57 PDT 
Created attachment 432140 [details]
Patch
Comment 2 Filip Pizlo 2021-06-24 09:22:17 PDT 
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.
Comment 3 Yijia Huang 2021-06-24 10:20:12 PDT 
Created attachment 432184 [details]
Patch
Comment 4 Yijia Huang 2021-06-24 10:29:30 PDT 
Created attachment 432185 [details]
Patch
Comment 5 Yijia Huang 2021-06-24 10:47:17 PDT 
(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.
Comment 6 Yijia Huang 2021-06-24 13:31:16 PDT 
Created attachment 432205 [details]
Patch
Comment 7 EWS 2021-06-24 15:50:26 PDT 
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].
Comment 8 Yusuke Suzuki 2021-06-24 16:47:48 PDT 
Committed r279253 (239137@main): <https://commits.webkit.org/239137@main>