On Mon, 2 Nov 2026 09:03:19 GMT, Larry <[email protected]> wrote: >> ### Description >> >> The AVX2 loop in generate_updateBytesAdler32() processed 16 bytes per >> iteration using two vpshufb and four vpaddd instructions, and reduced the >> accumulators with vpmulld followed by a vphaddd chain. >> >> VPHADDD is not a cheap single-µop operation: it decodes to three µops on >> Intel P-cores — two in-lane shuffle µops plus one add — and those shuffle >> µops contend for the shuffle pipes; the AVX forms are also microcoded on AMD >> Zen and Intel E-cores >> ([uops.info](https://uops.info/html-instr/VPHADDD_YMM_YMM_YMM.html)). In a >> hot reduction loop this makes the vphaddd chain throughput-limited by the >> shuffle pipes, hence relatively costly. >> >> This change rewrites the AVX2 loop as a dot product: >> - vpsadbw computes the byte sum for a; >> - vpmaddubsw/vpmaddwd compute the weighted byte sums for b, using two new >> weight tables; >> - the loop processes 64 bytes per iteration with two independent >> accumulators to shorten the dependency chain; >> - the AVX2 horizontal reduction uses 128-bit adds and shuffles instead of >> the vphaddd chain; >> - the 16/32-byte tails are handled with SIMD blocks. >> - the reduction mod BASE (65521) uses the gcc magic multiply (q = (x * >> 2147975281) >> 47) instead of two divl instructions. >> >> ### TEST >> >> - env: Intel Xeon 6980P (Granite Rapids), single socket, 128 cores / 256 >> threads (SMT2), 3 NUMA nodes, 755 GiB RAM, Ubuntu 22.04.4 LTS, glibc 2.35 >> - JMH: test/micro/org/openjdk/bench/java/util/TestAdler32.java; >> - jvmArgs: "-Xms4g -Xmx4g -XX:UseAVX=2" >> - results (ops/ms, mean): >> | size | baseline | patched | delta | >> | ---: | ---: | ---: | ---: | >> | 1 | 104097.5 | 167073.8 | +60.5% | >> | 2 | 101692.6 | 159382.4 | +56.7% | >> | 4 | 94645.6 | 145376.2 | +53.6% | >> | 8 | 84128.9 | 122917.0 | +46.1% | >> | 15 | 69304.3 | 91653.2 | +32.2% | >> | 16 | 66870.0 | 161618.9 | +141.7% | >> | 24 | 56286.5 | 111292.6 | +97.7% | >> | 31 | 50346.7 | 84490.9 | +67.8% | >> | 32 | 56756.3 | 71429.1 | +25.9% | >> | 64 | 52268 | 70605 | +35% | >> | 256 | 30809 | 59958 | +95% | >> | 1024 | 14318 | 33684 | +135% | >> | 4096 | 4475 | 10687 | +139% | >> | 8192 | 2268 | 5487 | +142% | >> | 65536 | 291.3 | 564.9 | +94% | >> >> Sizes below 16 bytes use the scalar loop in both versions; the only relevant >> difference is the final reduction modulo BASE, which baseline performs with >> two divl instructions and the patched version with the magic multiply (q = >> (x * 2147975281) >> 47). These rows therefore isolate the magic-mul... > > Larry has updated the pull request incrementally with one additional commit > since the last revision: > > 8394260: Address Adler32 AVX2 review comments > > Use an unsigned 32-bit type for the division magic constant and update the > LIMIT overflow comment to refer to the unsigned 32-bit range. > > Add an AVX2-specific jtreg run using all-0xFF input around LIMIT.
The `juint` change and the new 0xFF run are fine, and both TestAdler32 runs pass on a 9950X3D2. A few small things inline. The 1 to 31 byte sizes from your tables also aren't in the benchmark's `@Param` list, could you add them? src/hotspot/cpu/x86/stubGenerator_x86_64_adler.cpp line 305: > 303: __ vpaddd(yb, yb, ydata0, Assembler::AVX_256bit); > 304: __ vpaddd(yb_0, yb_0, ydata1, Assembler::AVX_256bit); > 305: __ vmovdqu(yprev_a, ya); `yprev_a` is always `ya` at the top of the loop, so adding `ya` to `ysum_a` before updating `ya` drops the register and this copy. Same at L327. src/hotspot/cpu/x86/stubGenerator_x86_64_adler.cpp line 306: > 304: __ vpaddd(yb_0, yb_0, ydata1, Assembler::AVX_256bit); > 305: __ vmovdqu(yprev_a, ya); > 306: __ jmp(SLOOP1A_AVX2); Two branches per iteration. Testing `s` once before the loop and ending with `jcc(aboveEqual, SLOOP1A_AVX2)` here drops the `jmp`. src/hotspot/cpu/x86/stubGenerator_x86_64_adler.cpp line 350: > 348: __ movdl(b_d, xb); > 349: > 350: __ movl(rax, a_d); This sequence appears four times now (also L356, L409, L415). A small lambda would cover them. ------------- PR Review: https://git.openjdk.org/jdk/pull/33338#pullrequestreview-5736360681 PR Review Comment: https://git.openjdk.org/jdk/pull/33338#discussion_r4442234181 PR Review Comment: https://git.openjdk.org/jdk/pull/33338#discussion_r4442234190 PR Review Comment: https://git.openjdk.org/jdk/pull/33338#discussion_r4442234202
