Bug 228057 – Add ARM64 EON opcode and select it in AIR

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

[Yijia Huang]

...

[Yijia Huang]

Created attachment 433749 [details]
Patch

[Yijia Huang]

Created attachment 433762 [details]
Patch

[Saam Barati]

Comment on attachment 433762 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:53
> +        // Old optimized AIR
> +        Lshift64 %x1, amount, %x1, @4
> +        Not      %x1,    %x1,      @5
> +        Xor      %x0,    %x1, %x0, @6
> +        Ret      %x0,              @7
> +
> +        // New optimized AIR
> +        XorNotLeftShift %x0, %x1, $63, %x0, @6

Nice

> Source/JavaScriptCore/b3/B3LowerToAir.cpp:829
> +    Air::Opcode lowerB3toAirInOpcode(B3::Opcode b3Opcode, 

I feel like we need a much more specific name here. The current one makes is not suggestive of what it's doing.

maybe "opcodeBasedOnShiftKind"? or something similar

> Source/JavaScriptCore/b3/B3LowerToAir.cpp:3144
> +            auto tryAppendEON = [&] (Value* left, Value* right) -> bool {

this looks good 👍

> Source/JavaScriptCore/b3/air/AirOpcode.opcodes:884
> +arm64: XorNotLeftShift64 U:G:64, U:G:64, U:G:64, D:G:64
> +    Tmp, Tmp, Imm, Tmp
> +
> +arm64: XorNotRightShift64 U:G:64, U:G:64, U:G:64, D:G:64
> +    Tmp, Tmp, Imm, Tmp
> +
> +arm64: XorNotUnsignedRightShift64 U:G:64, U:G:64, U:G:64, D:G:64
> +    Tmp, Tmp, Imm, Tmp

Why is the use of the Imm 64 bit instead of 32? I think 32 is more correct and consistent w/ how we do it elsewhere

[Yijia Huang]

Created attachment 433904 [details]
Patch

[Saam Barati]

Comment on attachment 433904 [details]
Patch

Nice. r=me

[EWS]

Committed r280111 (239828@main): <https://commits.webkit.org/239828@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/80864899>