Skip to content

8371259: ML-DSA AVX2 and AVX512 intrinsics and improvements #28136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
+1,572 −703
Open

8371259: ML-DSA AVX2 and AVX512 intrinsics and improvements #28136

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
35841c7
AVX2 and AVX512 intrinsics for MLDSA
vpaprotsk Jul 9, 2025
2ff3b82
Fixes and comments from Anas
vpaprotsk Oct 6, 2025
f4f84b6
add copyright, whitespace and test jtreg tags
vpaprotsk Nov 4, 2025
6d3f779
Merge remote-tracking branch 'origin/master' into avx2-ntt
vpaprotsk Nov 4, 2025
050312f
address first comments
vpaprotsk Nov 17, 2025
e913340
whitespace
vpaprotsk Nov 17, 2025
b04f4f0
next set of comments
vpaprotsk Nov 20, 2025
cefa021
Merge remote-tracking branch 'origin/master' into avx2-ntt
vpaprotsk Nov 24, 2025
691e1df
comments from Ferenc
vpaprotsk Nov 24, 2025
bfc16f1
spelling
vpaprotsk Nov 24, 2025
094051e
comments from Jatin
vpaprotsk Nov 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/hotspot/cpu/x86/assembler_x86.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3860,6 +3860,46 @@ void Assembler::evmovdquq(Address dst, KRegister mask, XMMRegister src, bool mer
emit_operand(src, dst, 0);
}

void Assembler::vmovsldup(XMMRegister dst, XMMRegister src, int vector_len) {
assert(vector_len == AVX_512bit ? VM_Version::supports_evex() : VM_Version::supports_avx(), "");
InstructionAttr attributes(vector_len, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ true);
Copy link
Member

@jatin-bhateja jatin-bhateja Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you check for AVX512-VL you allow accessing 128/256 bit registers from the higher register bank [X/Y]MM(16-31)

But your assertions are nowhere checking this.

Copy link
Contributor Author

@vpaprotsk vpaprotsk Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe those asserts are in vex_prefix_and_encode (

jdk/src/hotspot/cpu/x86/assembler_x86.cpp

Line 13164 in 6d3f779

assert(((!is_extended) || (!attributes->is_legacy_mode())),"XMM register should be 0-15");
) and vex_prefix (

jdk/src/hotspot/cpu/x86/assembler_x86.cpp

Line 13047 in 6d3f779

assert((!is_extended || (!attributes->is_legacy_mode())),"XMM register should be 0-15");
)

I also haven't found any other instruction that does this check so I could emulate the style.

int encode = vex_prefix_and_encode(dst->encoding(), 0, src->encoding(), VEX_SIMD_F3, VEX_OPCODE_0F, &attributes);
emit_int16(0x12, (0xC0 | encode));
}

void Assembler::vmovshdup(XMMRegister dst, XMMRegister src, int vector_len) {
assert(vector_len == AVX_512bit ? VM_Version::supports_evex() : VM_Version::supports_avx(), "");
InstructionAttr attributes(vector_len, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ true, /* uses_vl */ true);
Copy link
Member

@jatin-bhateja jatin-bhateja Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you check for AVX512-VL you allow accessing 128/256 bit registers from the higher register bank [X/Y]MM(16-31)

But your assertions are nowhere checking this.

int encode = vex_prefix_and_encode(dst->encoding(), 0, src->encoding(), VEX_SIMD_F3, VEX_OPCODE_0F, &attributes);
emit_int16(0x16, (0xC0 | encode));
}

void Assembler::evmovsldup(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len) {
assert(VM_Version::supports_evex(), "");
Copy link
Member

@jatin-bhateja jatin-bhateja Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(VM_Version::supports_evex(), "");
assert(vector_len == AVX_512 || VM_Version::supports_avx512vl), "");

Copy link
Contributor Author

@vpaprotsk vpaprotsk Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took the patch, but also kept the supports_evex() assert

assert(vector_len == AVX_512bit || VM_Version::supports_avx512vl(), "");
InstructionAttr attributes(vector_len, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ false, /* uses_vl */ true);
attributes.set_embedded_opmask_register_specifier(mask);
attributes.set_is_evex_instruction();
if (merge) {
attributes.reset_is_clear_context();
}
int encode = vex_prefix_and_encode(dst->encoding(), 0, src->encoding(), VEX_SIMD_F3, VEX_OPCODE_0F, &attributes);
emit_int16(0x12, (0xC0 | encode));
}

void Assembler::evmovshdup(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len) {
assert(VM_Version::supports_evex(), "");
Copy link
Member

@jatin-bhateja jatin-bhateja Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

assert(vector_len == AVX_512bit || VM_Version::supports_avx512vl(), "");
InstructionAttr attributes(vector_len, /* vex_w */ false, /* legacy_mode */ false, /* no_mask_reg */ false, /* uses_vl */ true);
attributes.set_embedded_opmask_register_specifier(mask);
attributes.set_is_evex_instruction();
if (merge) {
attributes.reset_is_clear_context();
}
int encode = vex_prefix_and_encode(dst->encoding(), 0, src->encoding(), VEX_SIMD_F3, VEX_OPCODE_0F, &attributes);
emit_int16(0x16, (0xC0 | encode));
}

// Uses zero extension on 64bit

void Assembler::movl(Register dst, int32_t imm32) {
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/cpu/x86/assembler_x86.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,11 @@ class Assembler : public AbstractAssembler {
void evmovdqaq(XMMRegister dst, Address src, int vector_len);
void evmovdqaq(XMMRegister dst, KRegister mask, Address src, bool merge, int vector_len);

void vmovsldup(XMMRegister dst, XMMRegister src, int vector_len);
void vmovshdup(XMMRegister dst, XMMRegister src, int vector_len);
void evmovsldup(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);
void evmovshdup(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);

// Move lower 64bit to high 64bit in 128bit register
void movlhps(XMMRegister dst, XMMRegister src);

Expand Down
1 change: 1 addition & 0 deletions src/hotspot/cpu/x86/macroAssembler_x86.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1368,6 +1368,7 @@ class MacroAssembler: public Assembler {

void vpcmpeqw(XMMRegister dst, XMMRegister nds, Address src, int vector_len);
void vpcmpeqw(XMMRegister dst, XMMRegister nds, XMMRegister src, int vector_len);
using Assembler::evpcmpeqd;
void evpcmpeqd(KRegister kdst, KRegister mask, XMMRegister nds, AddressLiteral src, int vector_len, Register rscratch = noreg);

// Vector compares
Expand Down
Loading