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

[Steve Dohrmann]

On Fri, 26 Apr 2024 23:04:46 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

> Should is_src_gpr be set to true for the additional following instructions as well:

Thanks @sviswa7, yes some of these should also have src_is_gpr = true. 
Fixed:
void Assembler::extractps(Register dst, XMMRegister src, uint8_t imm8)
void Assembler::pextrd(Register dst, XMMRegister src, int imm8)
void Assembler::pextrq(Register dst, XMMRegister src, int imm8)
void Assembler::pextrb(Register dst, XMMRegister src, int imm8)
void Assembler::movdq(Register dst, XMMRegister src)

Use a different prefix function that does use the src_if_gpr flag:
void Assembler::pextl(Register dst, Register src1, Address src2)
void Assembler::pdepl(Register dst, Register src1, Address src2)
void Assembler::pextq(Register dst, Register src1, Address src2)
void Assembler::pdepq(Register dst, Register src1, Address src2)

> src/hotspot/cpu/x86/assembler_x86.cpp line 11717:
> 
>> 11715: 
>> 11716: void Assembler::vex_prefix(Address adr, int nds_enc, int xreg_enc, VexSimdPrefix pre, VexOpcode opc, InstructionAttr *attributes) {
>> 11717:   bool is_extended = adr.base_needs_rex2() || adr.index_needs_rex2() || nds_enc >= 16 || xreg_enc >= 16;
> 
> We could add an assert here:
> if  (adr.base_needs_rex2() || adr.index_needs_rex2()) {
>   assert(UseAPX, "APX features not enabled");
> }

Thanks, done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 11769:
> 
>> 11767: }
>> 11768: 
>> 11769: int Assembler::vex_prefix_and_encode(int dst_enc, int nds_enc, int src_enc, VexSimdPrefix pre, VexOpcode opc, InstructionAttr *attributes, bool src_is_gpr) {
> 
> We could add an assert here:
> if (src_is_gpr && src_enc >= 16) {
>   assert(UseAPX, "APX features not enabled");
> }

Thanks, done.

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

PR Comment: https://git.openjdk.org/jdk/pull/18476#issuecomment-2080248834
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1581630063
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1581630162