[webkit-reviews] review denied: [Bug 227201] Add a new pattern to instruction selector to use BFI supported by ARM64 : [Attachment 432140] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied Yijia Huang <yijia_huang at apple.com>'s
request for review:
Bug 227201: Add a new pattern to instruction selector to use BFI supported by
ARM64
https://bugs.webkit.org/show_bug.cgi?id=227201

Attachment 432140: Patch

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




--- Comment #2 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 432140
  --> https://bugs.webkit.org/attachment.cgi?id=432140
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.