RFR: 8330844: Add aliases for conditional jumps and additional instruction forms for x86 [v4]

Sandhya Viswanathan sviswanathan at openjdk.org
Tue Apr 23 18:31:31 UTC 2024


On Tue, 23 Apr 2024 16:18:42 GMT, Scott Gibbons <sgibbons at openjdk.org> wrote:

>> Adding infrastructure for JDK-8320448.  Aliasing conditional jump instructions; adding some x86 instructions.
>
> Scott Gibbons has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Comment indentation

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

> 1833:   prefix(dst, reg);
> 1834:   emit_int8((unsigned char)0x39);
> 1835:   emit_operand(reg, dst, 1);

This should be emit_operand(reg, dst, 0);

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

> 4457: }
> 4458: 
> 4459: void Assembler::vpcmpeqb(XMMRegister dst, XMMRegister src1, Address src2, int vector_len) {

InstructionMark missing in this instruction as well.

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

> 4574: // In this context, the dst vector contains the components that are equal, non equal components are zeroed in dst
> 4575: void Assembler::vpcmpeqw(XMMRegister dst, XMMRegister nds, Address src, int vector_len) {
> 4576:   assert(vector_len == AVX_128bit ? VM_Version::supports_avx() : VM_Version::supports_avx2(), "");

InstructionMark missing in this instruction which takes Address as operand?

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 3573:

> 3571: }
> 3572: 
> 3573: void MacroAssembler::vpcmpeqb(XMMRegister dst, XMMRegister src1, Address src2, int vector_len) {

The assert is missing here:
assert(((dst->encoding() < 16 && src1->encoding() < 16) || VM_Version::supports_avx512vlbw()),"XMM register should be 0-15");

src/hotspot/cpu/x86/macroAssembler_x86.hpp line 961:

> 959:   void ALWAYSINLINE jo(Label& L, bool maybe_short = true) { jcc(Assembler::overflow, L, maybe_short); }
> 960:   void ALWAYSINLINE jno(Label& L, bool maybe_short = true) { jcc(Assembler::noOverflow, L, maybe_short); }
> 961:   void ALWAYSINLINE js(Label& L, bool maybe_short = true) { jcc(Assembler::positive, L, maybe_short); }

Isn't js -> jump is sign flag is set -> Assembler::negative?
Correspondingly jns, js_b, jns_b should also be corrected.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18893#discussion_r1576695214
PR Review Comment: https://git.openjdk.org/jdk/pull/18893#discussion_r1576714792
PR Review Comment: https://git.openjdk.org/jdk/pull/18893#discussion_r1576711681
PR Review Comment: https://git.openjdk.org/jdk/pull/18893#discussion_r1576666734
PR Review Comment: https://git.openjdk.org/jdk/pull/18893#discussion_r1576673354


More information about the hotspot-compiler-dev mailing list