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