On Tue, 3 Nov 2026 18:03:25 GMT, Volodymyr Paprotski <[email protected]>
wrote:
>> Larry has updated the pull request incrementally with one additional commit
>> since the last revision:
>>
>> 8394260: Refine Adler32 AVX2 loop
>>
>> Reduce the AVX2 hot loop to one conditional backedge and remove the
>> redundant yprev_a copy by accumulating ya before updating it.
>>
>> Factor the modulo BASE sequence into a helper and extend the Adler32 JMH
>> parameters to cover the small sizes.
>
> src/hotspot/cpu/x86/stubGenerator_x86_64_adler.cpp line 95:
>
>> 93: auto reduce_mod_base = [&](const Register reg) {
>> 94: __ movl(rax, reg);
>> 95: __ imulq(rax, rcx);
>
> `// rcx = DIV_BASE_MAGIC`
done;
> src/hotspot/cpu/x86/stubGenerator_x86_64_adler.cpp line 114:
>
>> 112: const XMMRegister ya = xmm0;
>> 113: const XMMRegister yb = xmm1;
>> 114: const XMMRegister yb_0 = xmm2;
>
> This is a bit nitpicky, but variable names really threw me for a loop. "Is
> `_0` from a summation operation?", "Is x, y prefix for column vs row
> operations?". There are overlaps/overloads in registers that are not
> 'trivial' to see that they are just fine..
>
> To convince myself, I rearranged some lines, ydot2v, ydot2v_0 -> ydot2v0,
> ydot2v1, yb_0 -> yb1
>
> Much of this was like it before you, but some cleanup is deserved I think
>
>
> const XMMRegister ya = xmm0;
> const XMMRegister yb0 = xmm1;
> const XMMRegister yb1 = xmm2;
> const XMMRegister ysum_a = xmm4;
> const XMMRegister yzero = xmm5;
> const XMMRegister yones = xmm6;
> const XMMRegister ydot2v0 = xmm11; // weights 64..33
> const XMMRegister ydot2v1 = xmm12; // weights 32..1
> const XMMRegister ydata0 = xmm13;
> const XMMRegister ydata1 = xmm14;
> const XMMRegister ytmp0 = xmm7;
> const XMMRegister ytmp1 = xmm15;
>
> // GPR save area
> const XMMRegister xtmp3 = xmm8;
> const XMMRegister xtmp4 = xmm9;
> const XMMRegister xtmp5 = xmm10;
>
> // 128-bit views of registers
> const XMMRegister xa = ya;
> const XMMRegister xb = yb0;
>
> // AVX512 only
> const XMMRegister xsa = xmm3; // vphaddd needs xmm0-15
> const XMMRegister ysa = xmm16;
> const XMMRegister xtmp0 = xmm17;
> const XMMRegister xtmp1 = xmm18;
> const XMMRegister xtmp2 = xmm19;
done
> src/hotspot/cpu/x86/stubGenerator_x86_64_adler.cpp line 300:
>
>> 298: __ subl(s, 64);
>> 299:
>> 300: __ vpsadbw(ytmp0, ydata0, yzero, Assembler::AVX_256bit);
>
> This could use a comment... feel free to adapt from mine (I annotate when get
> stuck/confused)
>
>
> // ysum_a += ya // "clever": accumulate the accumulator gives rest of yb
> scale
> // ya += sum bytes into quadwords across ydata0 and ydata1
> // ydata(01) = multiply and sum bytes into words (ydata(01) * [64..1])
> // yb(01) += sum words into doublewords across ydata(01)
done;
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/33338#discussion_r4466798387
PR Review Comment: https://git.openjdk.org/jdk/pull/33338#discussion_r4466803594
PR Review Comment: https://git.openjdk.org/jdk/pull/33338#discussion_r4466799637
