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

[Steve Dohrmann]

On Tue, 10 Sep 2024 07:31:25 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Steve Dohrmann has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   refactoring and fixes based on review comments
>
> src/hotspot/cpu/x86/assembler_x86.cpp line 1420:
> 
>> 1418:   emit_arith(0x11, 0xC0, src1, src2);
>> 1419: }
>> 1420: 
> 
> Encodings for ADC looks good, but I could not find uses for these instructions beyond stubs. Do you think we should add them lazily on need basis.

Done. Removed the eadc instruction functions.

> src/hotspot/cpu/x86/assembler_x86.cpp line 1452:
> 
>> 1450:   emit_int8(imm8);
>> 1451: }
>> 1452: 
> 
> Again I could not find any use of scalar 16 bit addition and only one use of 8 bit add. Should be consider adding them lazily.

Done. Removed the addb and eaddw functions.

> src/hotspot/cpu/x86/assembler_x86.cpp line 1783:
> 
>> 1781:   emit_arith(0x23, 0xC0, src1, src2);
>> 1782: }
>> 1783: 
> 
> We can skip adding 8bit and 16bit flavors of AND instructions since they are not being used by compiler / stubs.

Done. Removed the eandb and eandw functions.

> src/hotspot/cpu/x86/assembler_x86.cpp line 1795:
> 
>> 1793:   attributes.set_address_attributes(/* tuple_type */ EVEX_NOSCALE, /* input_size_in_bits */ EVEX_32bit);
>> 1794:   evex_prefix_ndd(src, dst->encoding(), 0, VEX_SIMD_NONE, VEX_OPCODE_0F_3C, &attributes, no_flags);
>> 1795:   emit_arith_operand(0x81, as_Register(4), src, imm32);
> 
> Suggestion:
> 
>   emit_arith_operand(0x81, rsp, src, imm32);

Done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 1840:
> 
>> 1838:   emit_operand(src1, src2, 0);
>> 1839: }
>> 1840: 
> 
> Should we only keep one _eandl (REG, REG, ADR)_ and remove other one _eandl(REG, ADR, REG)_, apart from source operand encodings there is no other difference b/w the two instructions.

Done. Removed eandl(reg adr reg)

> src/hotspot/cpu/x86/assembler_x86.cpp line 2016:
> 
>> 2014: void Assembler::ecmovl(Condition cc, Register dst, Register src1, Address src2) {
>> 2015:   InstructionMark im(this);
>> 2016:   NOT_LP64(guarantee(VM_Version::supports_cmov(), "illegal instruction"));
> 
> APX is only supported by 64 bit (IA-32e mode) targets.

Done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 2696:
> 
>> 2694: }
>> 2695: 
>> 2696: void Assembler::eidivl(Register src, bool no_flags) { // Unsigned
> 
> Comment should be signed division.

Done.

> src/hotspot/cpu/x86/assembler_x86.cpp line 6801:
> 
>> 6799:   emit_arith_operand(0x81, rbx, src, imm32);
>> 6800: }
>> 6801: 
> 
> Don't find any usage of SUB with borrow in existing code.  I think we can defer adding it till we have a use.

The ::esall(Reg, Reg, imm) is the ndd version of the ::sall(Reg, imm) function just above it.  The ::eshll(Reg, Reg, imm) follows later in the file as the ndd verison of ::shll(Reg, imm).  I understand that sal and shl encode the same semantics  for a given operand pattern, but both were present in non-ndd form so I added ndd forms.

Done. Removed esbb functions.

> src/hotspot/cpu/x86/assembler_x86.cpp line 12723:
> 
>> 12721:   byte3 |= pre;
>> 12722: 
>> 12723:   // P2: byte 4 as zL'Lbv'aaa or 00L0VF00 where V = V4 and F = NF (no flags)
> 
> Suggestion:
> 
>   // P2: byte 4 as zL'Lbv'aaa or 00LXVF00 where V = V4, X(extended context) = ND and F = NF (no flags)

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752874266
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752874475
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752874916
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752875481
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752876605
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752876765
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752876877
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752877186
PR Review Comment: https://git.openjdk.org/jdk/pull/20698#discussion_r1752872867