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

[Jatin Bhateja]

On Thu, 2 May 2024 20:31:17 GMT, Steve Dohrmann <duke at openjdk.org> wrote:

>> Add instruction encoding support for Intel APX extended general-purpose registers:
>> 
>> Intel Advanced Performance Extensions (APX) doubles the number of general-purpose registers, from 16 to 32. For more information about APX, see https://www.intel.com/content/www/us/en/developer/articles/technical/advanced-performance-extensions-apx.html.
>> 
>> By specification, instruction encoding remains unchanged for instructions using only the lower 16 GPRs. For cases where one or more instruction operands reference extended GPRs (Egprs), encoding targets either REX2, an extension of REX encoding, or an extended version of EVEX encoding. These new encoding schemes extend or modify existing instruction prefixes only when Egprs are used.
>
> Steve Dohrmann has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 20 additional commits since the last revision:
> 
>  - update for egpr use: bzhil(R,R,R), btq(R,R), btq(R,imm)
>  - Merge branch 'master' into apx-encoding-pr
>  - Update full name
>  - simplification and fix asserts in ldmxcsr, stmxcsr, and emit_prefix_and_int8
>  - remove is_map1 comment for addb, andb, movb, orb, testb, xchgb, xorb
>  - fix stmxcrs REX2 branch, add asserts to SHA instructions
>  - fixes: pp bits in crc32, REX2 branch in ldmxcsr
>  - add egpr support for popcntq(R,A), cvttsd2siq(R,A), popq(R)
>  - fix 4 more src_is_gpr = true cases, add asserts to check for UseAPX
>  - fix is_gpr arg on two functions with reversed src / dst operands
>  - ... and 10 more: https://git.openjdk.org/jdk/compare/6985920c...7b3e8ec7

src/hotspot/cpu/x86/assembler_x86.cpp line 2839:

> 2837: void Assembler::kmovwl(KRegister dst, KRegister src) {
> 2838:   assert(VM_Version::supports_evex(), "");
> 2839:   InstructionAttr attributes(AVX_128bit, /* rex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);

Suggestion:

  InstructionAttr attributes(AVX_128bit, /* rex_w */ false, /* legacy_mode */ true, /* no_mask_reg */ true, /* uses_vl */ false);


No GPR operand here.

src/hotspot/cpu/x86/assembler_x86.cpp line 2846:

> 2844: void Assembler::kmovdl(KRegister dst, Register src) {
> 2845:   assert(VM_Version::supports_avx512bw(), "");
> 2846:   InstructionAttr attributes(AVX_128bit, /* rex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);

FTR,  We are doing a legacy demotions in downstream code after checking actual register encoding.

src/hotspot/cpu/x86/assembler_x86.cpp line 2860:

> 2858: void Assembler::kmovql(KRegister dst, KRegister src) {
> 2859:   assert(VM_Version::supports_avx512bw(), "");
> 2860:   InstructionAttr attributes(AVX_128bit, /* rex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);

Suggestion:

  InstructionAttr attributes(AVX_128bit, /* rex_w */ true, /* legacy_mode */ true, /* no_mask_reg */ true, /* uses_vl */ false);

src/hotspot/cpu/x86/assembler_x86.cpp line 6556:

> 6554:   assert(VM_Version::supports_bmi1(), "tzcnt instruction not supported");
> 6555:   emit_int8((unsigned char)0xF3);
> 6556:   int encode = prefixq_and_encode(dst->encoding(), src->encoding(), true /* is_map1 */);

FTR, Quoting relevant except from section 3.1.2.1 of APX specification.
“REX2 must be the last prefix. The byte following it is interpreted as the main opcode byte in the opcode map indicated by M0. The 0x0F escape byte is neither needed nor allowed.”

src/hotspot/cpu/x86/assembler_x86.hpp line 536:

> 534:     REXBIT_X  = 0x02,
> 535:     REXBIT_R  = 0x04,
> 536:     REXBIT_W  = 0x08,

Suggestion:

    REX2BIT_B  = 0x01,
    REX2BIT_X  = 0x02,
    REX2BIT_R  = 0x04,
    REX2BIT_W  = 0x08,

Name change suggestion since these bits are part of REX2 prefix.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1589185356
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1589195694
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1589193598
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1589207336
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1588761975