RFR: 8328998: Encoding support for Intel APX extended general-purpose registers [v14]

[Steve Dohrmann]

On Tue, 7 May 2024 05:51:22 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Steve Dohrmann has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   revert unneeded legacy flag change for kmovwl(K,K) and kmovql(K,K)
>
> src/hotspot/cpu/x86/assembler_x86.cpp line 11754:
> 
>> 11752: 
>> 11753: // This is a 4 byte encoding
>> 11754: void Assembler::evex_prefix(bool vex_r, bool vex_b, bool vex_x, bool evex_r, bool evex_b, bool evex_v,
> 
> Suggestion:
> 
> void Assembler::evex_prefix(bool vex_r, bool vex_b, bool vex_x, bool evex_r, bool eevex_b, bool evex_v,

Thanks, done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 11766:
> 
>> 11764:   // P0: byte 2, initialized to RXBR`00mm
>> 11765:   // instead of not'd
>> 11766:   int byte2 = (vex_r ? VEX_R : 0) | (vex_x ? VEX_X : 0) | (vex_b ? VEX_B : 0) | (evex_r ? EVEX_Rb : 0);
> 
> Comment at [L#11765 
> ](https://github.com/openjdk/jdk/pull/18476/files#diff-e3576e9c22db89236cdb906f032ff00748ff6d1c21b05277d991d80af75daf3aL11686)
> `// P0: byte 2, initialized to RXBR'00mm  => // P0: byte 2, initialized to RXBR'0mmm`

Thanks, done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 11768:
> 
>> 11766:   int byte2 = (vex_r ? VEX_R : 0) | (vex_x ? VEX_X : 0) | (vex_b ? VEX_B : 0) | (evex_r ? EVEX_Rb : 0);
>> 11767:   byte2 = (~byte2) & 0xF0;
>> 11768:   byte2 |= evex_b ? EEVEX_B : 0;
> 
> Suggestion:
> 
>   byte2 |= eevex_b ? EEVEX_B : 0;
> 
> 
> This corresponds to B4 bit which is specific to EEVEX encoding.

Thanks, done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 11846:
> 
>> 11844:     }
>> 11845:     bool eevex_x = adr.index_needs_rex2();
>> 11846:     bool evex_b = adr.base_needs_rex2();
> 
> Suggestion:
> 
>     bool eevex_b = adr.base_needs_rex2();

Thanks, done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 11848:
> 
>> 11846:     bool evex_b = adr.base_needs_rex2();
>> 11847:     attributes->set_is_evex_instruction();
>> 11848:     evex_prefix(vex_r, vex_b, vex_x, evex_r, evex_b, evex_v, eevex_x, nds_enc, pre, opc);
> 
> Suggestion:
> 
>     evex_prefix(vex_r, vex_b, vex_x, evex_r, eevex_b, evex_v, eevex_x, nds_enc, pre, opc);

Thanks, done.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1592799351
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1592800035
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1592799078
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1592799595
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1592799825