zlib-ng PR #1977 follow-up: enable adler32_copy_avx2_vnni with unroll x4 / four VPDPBUSD accumulators to pop the data-dependency bubble (checksum bit-exact + spill-free; not yet benchmarked)

zlib-ng-pr1977-adler32-copy-avx2vnni-4accumulator-README.md

zlib-ng — adler32 AVX2-VNNI copy: unroll ×4, four VPDPBUSD accumulators

Follow-up to PR #1977. Enables the disabled (#if 0) adler32_copy_avx2_vnni copy variant by rebuilding it on the same structure as the shipping checksum-only path.

Why the disabled copy was slow

The author left adler32_copy_avx2_vnni under #if 0 with a note about a "data dependency bubble," faster on Gracemont E-cores but slower on Raptor Cove P-cores, and a conversation comment: "I think there may need to be more unrolling."

Root cause: the disabled body was unrolled ×2 (64 B/iter) with only 2 VPDPBUSD accumulators. VPDPBUSD has ~5-cycle latency, so two in-flight dot products can't cover it and the loop is latency-bound — that is the bubble. The shipping checksum-only path had already fixed exactly this by unrolling ×4 with 4 accumulators; the copy variant just never got the same treatment (and its vs1_0 = vs1 tail also reset the running sum instead of snapshotting it).

On P-cores the plain AVX2 copy (maddubs+madd, not dpbusd) isn't latency-bound — its vs2 advances through a 1-cycle vpaddd with the multiplies off the loop-carried path — so it beat the 2-accumulator VNNI there. This change removes that disadvantage.

Change

Replace the disabled body with the proven a/b-split, unroll-by-4 (128 B/iter), four-VPDPBUSD-accumulator structure from adler32_avx2_vnni, plus the copy stores.

Verification

• Checksum: scalar model of the accounting is bit-identical to the reference adler32 across 435 sizes, including every 32/64/96/128/NMAX boundary with random initial adler.
• Copy: correct by construction — the four stores reuse the just-loaded ymm regs and dst advances in lockstep with src; tails delegate to adler32_copy_sse42 / adler32_copy_tail.
• Codegen (clang 22, -mavxvnni): builds spill-free; hot loop is 4 loads + 4 stores + 4 vpdpbusd + 8 vpaddd, all 16 ymm live, no stack traffic.

Expectations / caveats

• Should close the P-core gap the author saw (bubble removed); on E-cores it was already ahead of the plain AVX2 copy.
• Not benchmarked — I'm on arm64, so this is asm + scalar-model verification only. Needs an A/B on real Raptor Lake P and E cores before it's re-enabled.
• Wiring still needed to activate it: uncomment the prototype in x86_functions.h, point native_adler32_fold_copy at adler32_copy_avx2_vnni, and re-enable the two commented-out copy benchmarks. This gist is just the kernel.

Apply with git apply from the repo root (onto the PR #1977 branch).

zlib-ng-pr1977-adler32-copy-avx2vnni-4accumulator.diff

diff --git a/arch/x86/adler32_avx2_vnni.c b/arch/x86/adler32_avx2_vnni.c
--- a/arch/x86/adler32_avx2_vnni.c
+++ b/arch/x86/adler32_avx2_vnni.c
@@ -139,12 +139,9 @@
     return adler;
 }
 
-/* There's a data dependency bubble to pop here to maximize throughput but I haven't been able to quite factor
- * out exactly what gets us there. I'm leaving this here as this is the closest I managed to get. For performance
- * cores on Raptor Lake, the avx2 copy function is faster. For E-cores, this function is faster. There's
- * likely some sophisticated fusion happening on Raptor Cove that is just not happening on Gracemont */
-
-#if 0
+/* Copy variant, unrolled by 4 with four VPDPBUSD accumulators. The original 2-accumulator
+ * version left a data-dependency bubble because two in-flight dot products cannot cover the
+ * instruction's ~5-cycle latency; four accumulators do, matching the checksum-only path. */
 Z_INTERNAL uint32_t adler32_copy_avx2_vnni(uint32_t adler, uint8_t *dst, const uint8_t *src, size_t len) {
     uint32_t adler0, adler1;
     adler1 = (adler >> 16) & 0xffff;
@@ -163,60 +160,84 @@
                                              14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1);
 
     const __m256i zero = _mm256_setzero_si256();
-    __m256i vs1, vs2, vs2_0, vs3_0;
+    /* Just like the checksum-only path but split over 2 registers per 64-byte block */
+    __m256i vs1a, vs2a, vs2_0a;
+    __m256i vs1b, vs2b, vs2_0b;
 
     while (len >= 32) {
-        vs1 = _mm256_zextsi128_si256(_mm_cvtsi32_si128(adler0));
-        vs2 = _mm256_zextsi128_si256(_mm_cvtsi32_si128(adler1));
-        __m256i vs1_0 = vs1;
-        __m256i vs1_1 = zero;
+        vs1a = _mm256_zextsi128_si256(_mm_cvtsi32_si128(adler0));
+        vs2a = _mm256_zextsi128_si256(_mm_cvtsi32_si128(adler1));
+
+        vs1b = zero;
+        vs2b = zero;
+
+        __m256i vs1_0a = vs1a;
+        __m256i vs1_0b = vs1b;
         __m256i vs3 = zero;
-        vs2_0 = vs3;
-        vs3_0 = vs3;
+        vs2_0a = zero;
+        vs2_0b = zero;
 
-        size_t k = MIN(len, NMAX);
-        k -= k % 32;
+        size_t k = ALIGN_DOWN(MIN(len, NMAX), 32);
         len -= k;
 
-        /* We might get a tad bit more ILP here if we sum to a second register in the loop */
-        __m256i vbuf0, vbuf1;
+        __m256i vbuf0, vbuf1, vbuf2, vbuf3;
 
-        /* Manually unrolled this loop by 2 for a decent amount of ILP */
-        while (k >= 64) {
+        /* Manually unrolled this loop by 4 for a decent amount of ILP */
+        while (k >= 128) {
             /*
                vs1 = adler + sum(c[i])
                vs2 = sum2 + 64 vs1 + sum( (64-i+1) c[i] )
             */
             vbuf0 = _mm256_loadu_si256((__m256i*)src);
             vbuf1 = _mm256_loadu_si256((__m256i*)(src + 32));
+            vbuf2 = _mm256_loadu_si256((__m256i*)(src + 64));
+            vbuf3 = _mm256_loadu_si256((__m256i*)(src + 96));
             _mm256_storeu_si256((__m256i*)dst, vbuf0);
             _mm256_storeu_si256((__m256i*)(dst + 32), vbuf1);
-            src += 64;
-            dst += 64;
-            k -= 64;
+            _mm256_storeu_si256((__m256i*)(dst + 64), vbuf2);
+            _mm256_storeu_si256((__m256i*)(dst + 96), vbuf3);
+            src += 128;
+            dst += 128;
+            k -= 128;
 
-            __m256i vs1_sad = _mm256_sad_epu8(vbuf0, zero);
-            __m256i vs1_sad2 = _mm256_sad_epu8(vbuf1, zero);
-
-            /* multiply-add, resulting in 16 ints. Fuse with sum stage from prior versions, as we now have the dp
-             * instructions to eliminate them */
-            vs2 = _mm256_dpbusd_epi32(vs2, vbuf0, dot2v);
-            vs2_0 = _mm256_dpbusd_epi32(vs2_0, vbuf1, dot2v_0);
-
-            vs1_0 = _mm256_add_epi32(vs1_0, vs1_sad);
-            vs1_1 = _mm256_add_epi32(vs1_1, vs1_sad2);
+            __m256i vs1_sada = _mm256_sad_epu8(vbuf0, zero);
+            __m256i vs1_sadb = _mm256_sad_epu8(vbuf1, zero);
 
-            vs3_0 = _mm256_add_epi32(vs3_0, vs1_1);
-            vs3 = _mm256_add_epi32(vs3, vs1_0);
-            vs1_0 = vs1;
+            vs1a = _mm256_add_epi32(vs1a, vs1_sada);
+            vs1b = _mm256_add_epi32(vs1b, vs1_sadb);
+
+            vs3 = _mm256_add_epi32(vs3, vs1_0a);
+            vs3 = _mm256_add_epi32(vs3, vs1_0b);
+
+            vs2a = _mm256_dpbusd_epi32(vs2a, vbuf0, dot2v);
+            vs2b = _mm256_dpbusd_epi32(vs2b, vbuf1, dot2v_0);
+
+            vs3 = _mm256_add_epi32(vs3, vs1a);
+            vs3 = _mm256_add_epi32(vs3, vs1b);
+
+            vs1_sada = _mm256_sad_epu8(vbuf2, zero);
+            vs1_sadb = _mm256_sad_epu8(vbuf3, zero);
+
+            vs1a = _mm256_add_epi32(vs1a, vs1_sada);
+            vs1b = _mm256_add_epi32(vs1b, vs1_sadb);
+
+            vs2_0a = _mm256_dpbusd_epi32(vs2_0a, vbuf2, dot2v);
+            vs2_0b = _mm256_dpbusd_epi32(vs2_0b, vbuf3, dot2v_0);
+
+            vs1_0a = vs1a;
+            vs1_0b = vs1b;
         }
 
-        vs2 = _mm256_add_epi32(vs2_0, vs2);
         vs3 = _mm256_slli_epi32(vs3, 6);
-        vs2 = _mm256_add_epi32(vs2, vs3);
-        vs2 = _mm256_add_epi32(vs3_0, vs2);
-        vs3 = _mm256_setzero_si256();
+        vs2_0a = _mm256_add_epi32(vs2_0a, vs2_0b);
+        vs2a = _mm256_add_epi32(vs2a, vs3);
+        vs2a = _mm256_add_epi32(vs2a, vs2_0a);
+        vs2a = _mm256_add_epi32(vs2a, vs2b);
+        vs1a = _mm256_add_epi32(vs1a, vs1b);
+        vs1_0a = _mm256_add_epi32(vs1_0a, vs1_0b);
 
+        vs3 = zero;
+
         while (k >= 32) {
             __m256i vbuf = _mm256_loadu_si256((__m256i*)src);
             _mm256_storeu_si256((__m256i*)dst, vbuf);
@@ -226,28 +247,27 @@
 
             __m256i vs1_sad = _mm256_sad_epu8(vbuf, zero); // Sum of abs diff, resulting in 2 x int32's
 
-            vs1 = _mm256_add_epi32(vs1, vs1_sad);
-            vs3 = _mm256_add_epi32(vs3, vs1_0);
-            vs2 = _mm256_dpbusd_epi32(vs2, vbuf, dot2v_0);
-            vs1_0 = vs1;
+            vs1a = _mm256_add_epi32(vs1a, vs1_sad);
+            vs3 = _mm256_add_epi32(vs3, vs1_0a);
+            vs2a = _mm256_dpbusd_epi32(vs2a, vbuf, dot2v_0);
+            vs1_0a = vs1a;
         }
 
         vs3 = _mm256_slli_epi32(vs3, 5);
-        vs2 = _mm256_add_epi32(vs2, vs3);
+        vs2a = _mm256_add_epi32(vs2a, vs3);
 
-        adler0 = partial_hsum256(vs1) % BASE;
-        adler1 = hsum256(vs2) % BASE;
+        adler0 = partial_hsum256(vs1a) % BASE;
+        adler1 = hsum256(vs2a) % BASE;
     }
 
     adler = adler0 | (adler1 << 16);
 
-    /* Process tail (len < 64). */
+    /* Process tail (len < 32). */
     if (len) {
         goto rem_peel_copy;
     }
 
     return adler;
 }
-#endif
 
 #endif