RFR: 8343214: Fix encoding errors in APX New Data Destination Instructions Support

[Jatin Bhateja]

On Tue, 29 Oct 2024 17:19:20 GMT, Srinivas Vamsi Parasa <sparasa at openjdk.org> wrote:

> The goal of this PR is to fix instruction encoding errors for some of the instructions which support the APX New Data Destination (NDD) and No Flags (NF) features (added in [JDK-8329035](https://bugs.openjdk.org/browse/JDK-8329035))

I think we should first check-in extended gtest asm validation script detecting these issues either before or along with this patch.

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

> 1481: void Assembler::eaddl(Register dst, Register src1, Register src2, bool no_flags) {
> 1482:   InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);
> 1483:   (void) evex_prefix_and_encode_ndd(src2->encoding(), dst->encoding(), src1->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags);

Hi @vamsi-parasa,
NDD is very flexible in terms of argument selection, i.e. 
ADDL  NDD,  SRC1 (ModRM.R/M), SRC2 (ModRM.REG) has opcode 0x01
Whereas,
ADDL  NDD, SRC1 (ModRM.REG),  SRC2 (ModRM.R/M) has opcode 0x03

In this case, we are trying to match GCC encoding scheme.

Can you please add the following comment here since the argument nomenclature does not match with parameter nomenclature?

NDD shares its encoding bits with NDS bits for regular EVEX instruction. Therefore we are passing DST as the second argument to minimize changes in leaf level routine.

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

> 2630:   InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);
> 2631:   attributes.set_address_attributes(/* tuple_type */ EVEX_NOSCALE, /* input_size_in_bits */ EVEX_32bit);
> 2632:   evex_prefix_nf(src, 0, dst->encoding(), VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags);

Could you also replace VEX_OPCODE_OF_3C with the standard naming convention of VEX_OPCODE_MAP4?
I added /*MAP4*/ in the comments after the prefix for the setzuCC instruction, but it's better to make this change consistently in all places.

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

PR Review: https://git.openjdk.org/jdk/pull/21770#pullrequestreview-2403880657
PR Review Comment: https://git.openjdk.org/jdk/pull/21770#discussion_r1822168026
PR Review Comment: https://git.openjdk.org/jdk/pull/21770#discussion_r1822048023