Bug 227201 - Add a new pattern to instruction selector to use BFI supported by ARM64
Summary: Add a new pattern to instruction selector to use BFI supported by ARM64
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yijia Huang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-06-20 21:51 PDT by Yijia Huang
Modified: 2021-06-24 16:47 PDT (History)
10 users (show)

See Also:

Attachments
Patch (30.46 KB, patch)
2021-06-24 01:50 PDT, Yijia Huang 		no flags Details | Formatted Diff | Diff
Patch (23.51 KB, patch)
2021-06-24 10:20 PDT, Yijia Huang 		no flags Details | Formatted Diff | Diff
Patch (23.34 KB, patch)
2021-06-24 10:29 PDT, Yijia Huang 		no flags Details | Formatted Diff | Diff
Patch (25.79 KB, patch)
2021-06-24 13:31 PDT, Yijia Huang 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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 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>