On Tue, 20 Oct 2026 12:43:07 GMT, Larry <[email protected]> wrote: >> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 8488: >> >>> 8486: pxor(xmm5, xmm5); >>> 8487: pcmpeqd(xmm5, xmm5); >>> 8488: pslldq(xmm5, 4); >> >> I don't understand what these 2 instructions do. I assume `pxor` clears >> `xmm5`. > > `pxor` clears `xmm5` may be redundant. I'll remove `pxor` in the next patch. > > The next 2 instructions build the 128-bit mask used by the following `pand`: > > - `pcmpeqd xmm5, xmm5` sets every 32-bit lane to `0xffffffff`; > - `pslldq xmm5, 4` shifts the value left by four bytes, leaving > `0xffffffff_ffffffff_ffffffff_00000000` (low dword cleared, upper 96 bits > set). > > This is the same mask that `kernel_crc32_avx512` loads from > `_crc_by128_masks_avx512[1]`; the later `vpsrldq xmm5, 8` derives > `_crc_by128_masks_avx512[0]` `0x00000000_00000000_ffffffff_ffffffff` for the > second `pand`. > > The corresponding code in `kernel_crc32_avx512` is: > > // barrett reduction > bind(L_barrett); > // [masks + 1 * 16] = 0xffffffff_ffffffff_ffffffff_00000000: > // keep the high 96 bits. > vpand(xmm7, xmm7, > ExternalAddress(StubRoutines::x86::crc_by128_masks_avx512_addr() + 1 * 16), > Assembler::AVX_128bit, tmp2); > movdqu(xmm1, xmm7); > movdqu(xmm2, xmm7); > movdqu(xmm10, Address(table, 4 * 16)); > > pclmulqdq(xmm7, xmm10, 0x0); > pxor(xmm7, xmm2); > // [masks + 0] = 0x00000000_00000000_ffffffff_ffffffff: > // keep the low 64 bits. > vpand(xmm7, xmm7, > ExternalAddress(StubRoutines::x86::crc_by128_masks_avx512_addr()), > Assembler::AVX_128bit, tmp2); > movdqu(xmm2, xmm7); > pclmulqdq(xmm7, xmm10, 0x10); > pxor(xmm7, xmm2); > pxor(xmm7, xmm1); > pextrd(crc, xmm7, 2);
Add comments similar to kernel_crc32_avx512 code comments to show state of mask. Looking on your AVX vs non-AVX code. It is unfortunate that you have to duplicate exactly the same code but with different prefix encoding. I have few comments. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33342#discussion_r4364795889
