RFR: 8329035: New Data Destination instructions support [v2]

[Steve Dohrmann]

On Wed, 28 Aug 2024 15:22:09 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Steve Dohrmann has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   function name changes based on review comments
>
> src/hotspot/cpu/x86/assembler_x86.cpp line 1579:
> 
>> 1577: 
>> 1578: void Assembler::eaddl(Register dst, Register src1, Register src2, bool no_flags) {
>> 1579:   InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);
> 
> Should we not auto demote these instruction to use legacy MAP0 encoding, if dst and src1 / src2 are same and does not belong to EGPR set?
> We do REX to VEX promotion and EVEX to VEX demotions at assembler level if the required criteria is met.

The thinking is that the user would only use these extended functions if they want either NDD or NF semantics, or both.  If they want only NF they can use the same register for dst / src and set no_flags to true; this case requires evex encoding.  In other cases, they can call the non-"e" function to get prefix-optimized use of the larger register set.

> src/hotspot/cpu/x86/assembler_x86.cpp line 2647:
> 
>> 2645:   InstructionAttr attributes(AVX_128bit, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ false);
>> 2646:   attributes.set_address_attributes(/* tuple_type */ EVEX_NOSCALE, /* input_size_in_bits */ EVEX_32bit);
>> 2647:   vex_prefix_ndd(src, dst->encoding(), 0, VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags);
> 
> Suggestion:
> 
>   eevex_prefix_ndd(src, dst->encoding(), 0, VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags);

Done.

> src/hotspot/cpu/x86/assembler_x86.hpp line 794:
> 
>> 792:                        bool eevex_x, int nds_enc, VexSimdPrefix pre, VexOpcode opc, bool no_flags = false);
>> 793: 
>> 794:   void vex_prefix_ndd(Address adr, int ndd_enc, int xreg_enc, VexSimdPrefix pre, VexOpcode opc, InstructionAttr *attributes, bool no_flags = false) {
> 
> Suggestion:
> 
>   void eevx_prefix_ndd(Address adr, int ndd_enc, int xreg_enc, VexSimdPrefix pre, VexOpcode opc, InstructionAttr *attributes, bool no_flags = false) {
> 
> 
> NDD is only supported with 4 byte extended evex encoding.

Done.  Assume "evex_prefix_ndd" was intended.

> src/hotspot/cpu/x86/assembler_x86.hpp line 798:
> 
>> 796:   }
>> 797: 
>> 798:   void vex_prefix_nf(Address adr, int ndd_enc, int xreg_enc, VexSimdPrefix pre, VexOpcode opc, InstructionAttr *attributes, bool no_flags = false) {
> 
> Same as above.

Done.

> src/hotspot/cpu/x86/assembler_x86.hpp line 809:
> 
>> 807:                              InstructionAttr *attributes, bool src_is_gpr = false, bool nds_is_ndd = false, bool force_evex = false, bool no_flags = false);
>> 808: 
>> 809:   int  vex_prefix_and_encode_ndd(int dst_enc, int nds_enc, int src_enc, VexSimdPrefix pre, VexOpcode opc,
> 
> Suggestion:
> 
>   int  vex_prefix_and_encode_ndd(int ndd_enc, int nds_enc, int src_enc, VexSimdPrefix pre, VexOpcode opc,

The second argument (nds_enc) carries the ndd encoding.  I kept the original parameter names since they are used in the implementation, which now serves both the ndd and nds features.  The added nds_is_ndd flag distinguishes the two uses.  

The two encoding functions (vex_prefix and vex_prefix_and_encode) were fairly complex to start with.  Changes to them to get ndd and nf features are pretty simple, so reusing them and keeping the existing parameter names in the implementation seemed a good idea.  Using these same parameter names in the header took away the need to translate names if going between the header and impl.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1742768595
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1742767859
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1742767513
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1742767666
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1742769333