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

[Sandhya Viswanathan]

On Fri, 19 Apr 2024 21:51:45 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 incrementally with one additional commit since the last revision:
> 
>   bug fix in other ::prefix_rex2

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

> 668:     } else {
> 669:       // [base + disp]
> 670:       // !(rbp | r12 | r20 | r28) were handled above

comment should be:
// (rsp | r12 | r20 | r28) were handled above

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

> 3618:   InstructionAttr attributes(AVX_128bit, /* rex_w */ true, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);
> 3619:   // swap src/dst to get correct prefix
> 3620:   int encode = simd_prefix_and_encode(src, xnoreg, as_XMMRegister(dst->encoding()), VEX_SIMD_66, VEX_OPCODE_0F, &attributes, true);

Here the last argument to simd_prefix_and_encode shouldn't be true as src is not gpr?

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

> 12895:   if (adr.index_needs_rex2()) {
> 12896:     assert(false, "prefix(Register dst, Address adr) does not support handling of an X");
> 12897:   }

this could be written as:
assert(!adr.index_needs_rex2(), "prefix(Register dst, Address adr) does not support handling of an X");

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

> 13837: void Assembler::movsbq(Register dst, Address src) {
> 13838:   InstructionMark im(this);
> 13839:   int prefix = get_prefixq(src, dst, true /* page1 */);

We are not consistent in the comment is_map1, M0, page1 all refer to the same thing. Also some places there is no comment that the true is for is_map1.

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

> 14477:     _input_size_in_bits = input_size_in_bits;
> 14478:   }
> 14479: }

New line missing at the end of file.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1579945658
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1580219101
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1580246261
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1580258716
PR Review Comment: https://git.openjdk.org/jdk/pull/18476#discussion_r1580260295