From bcee628a228dc3b3eceb55309665308f4c624f28 Mon Sep 17 00:00:00 2001 From: Matevz Kovacic <117923752+matevz-kovacic@users.noreply.github.com> Date: Tue, 11 Aug 2026 11:38:44 +0200 Subject: [PATCH] decompress: keep the repeat-offset triple in registers on x86-64 ZSTD_decodeSequence() keeps the three recent offsets in seqState->prevOffset[3] and updates them through variable subscripts (prevOffset[ll0], prevOffset[!ll0], prevOffset[offset]). A variable index is what stops the compiler promoting the array to registers, so the triple stays in memory; and because it is carried from one sequence to the next, the sequence-decoding loop ends up with a store-to-load dependency chain running through memory across iterations. The format does not require the array. RFC 8878 3.1.1.5 defines the repeat-offset update as a total function on the triple: seven cases, each a constant rearrangement plus at most one new value. Every index used is statically confined to {0, 1, 2}. ll0 is a C comparison result, so ll0 and !ll0 are both in {0,1}. The prevOffset[offset] subscript is reached only on the ofBits == 1 path; OF_bits[] is the identity map, so that means offset code 1, and OF_base[1] == 1. Hence offset = ofBase + ll0 + readBits(1) = 1 + ll0 + bit, with both addends in {0,1}, so offset is in {1,2,3} and can never be 0. offset == 3 is then peeled off into prevOffset[0] - 1 by the preceding conditional, leaving offset in {1,2} at the subscript. The shift-down updates use the literal indices 0, 1 and 2. The scalar form is already in the tree, inside #if defined(__aarch64__). That arm holds two independent things: the hoisting of the triple into locals, which carries no comment, and a separate ZSTD_seqSymbol ZSTD_memcpy hack whose comment describes an aarch64 ldr/ldrb/ldrh codegen problem. Nothing in the tree states why the hoisting itself is aarch64-only. Case analysis over the seven cases shows the two arms compute the same function on any 64-bit target; every remaining difference sits under MEM_32bits(), a compile-time 0 there. This widens the fence for the hoisting and leaves the ZSTD_seqSymbol hack fenced to aarch64 where it is. No new logic is introduced, and aarch64 code paths are unchanged. !defined(__ILP32__) matters: the scalar arm omits every MEM_32bits() path, so it is equivalent only on LP64/LLP64, not on x32. Hardware counters, per decode pass over Silesia at level 9, AMD Zen 5, gcc (change relative to baseline): store dispatches 2.5428e8 -> 1.9198e8 -24.5% store-to-load forwards 2.2236e8 -> 1.7509e8 -21.3% L1d loads 8.6149e8 -> 7.5978e8 -11.8% instructions 2.1640e9 -> 2.0145e9 -6.9% cycles 5.1631e8 -> 4.9480e8 -4.2% Both compilers lose stores; they differ in what else moves. gcc drops store-to-load forwards ~21% and loads ~8% as well. clang drops stores harder (-42%) but issues ~9% more load dispatches, and still ends up with the larger cycle win. L1d load misses are unchanged, so this removes accesses rather than improving locality. Decode throughput on Silesia, single-threaded, -O3, no -march, median of 22 reps on a pinned core with baseline and patched runs alternated and counterbalanced. AMD Ryzen 9700X (Zen 5), bare metal, turbo off, SMT sibling offline; measurement noise floor on that machine 0.44%: gcc clang level 1 +2.74% +5.48% level 3 +3.42% +9.31% level 9 +3.19% +8.58% level 19 +3.18% +6.58% All eight cells have disjoint interquartile ranges. A cycle-accurate cross-check agrees with wall-clock throughout, ruling out a frequency artefact. On Intel Raptor Lake with the same compiler versions the effect has the same sign and the same compiler ordering, at smaller magnitude under gcc (+1.4% to +2.3%). Compressed output is unchanged: this touches only lib/decompress, and total compressed size over Silesia was byte-identical to baseline at every level under both compilers on both machines. Round-trip, forward and backward format compatibility against unmodified upstream binaries, and tests/fuzzer plus tests/zstreamtest all pass at levels 1/3/9/19 under both toolchains. One caveat worth weighing: encode throughput moves by under +/-1% in both directions, despite this touching no encoder source. Three of the eight cells are statistically distinguishable from baseline and they disagree on sign -- clang L1 -0.52%, clang L3 +0.90%, gcc L19 +0.66%. Movement of that size that does not agree on sign across cells is a code-layout effect. This file already acknowledges layout sensitivity of comparable magnitude: the .p2align block ~140 lines below exists because decompression-loop alignment moved throughput by about 10% via DSB/MITE residency, and its comment reports that as reproduced on Kabylake and Coffeelake but not on Haswell, Broadwell or Skylake. A gate forbidding any detectable regression anywhere rejects on the single -0.52% cell. --- lib/decompress/zstd_decompress_block.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/decompress/zstd_decompress_block.c b/lib/decompress/zstd_decompress_block.c index 56e1f9eda..f8676a7b9 100644 --- a/lib/decompress/zstd_decompress_block.c +++ b/lib/decompress/zstd_decompress_block.c @@ -1236,7 +1236,7 @@ FORCE_INLINE_TEMPLATE seq_t ZSTD_decodeSequence(seqState_t* seqState, const ZSTD_longOffset_e longOffsets, const int isLastSeq) { seq_t seq; -#if defined(__aarch64__) +#if defined(__aarch64__) || (defined(__x86_64__) && !defined(__ILP32__)) size_t prevOffset0 = seqState->prevOffset[0]; size_t prevOffset1 = seqState->prevOffset[1]; size_t prevOffset2 = seqState->prevOffset[2]; @@ -1248,7 +1248,7 @@ ZSTD_decodeSequence(seqState_t* seqState, const ZSTD_longOffset_e longOffsets, c * operations that cause performance drop. This can be avoided by using this * ZSTD_memcpy hack. */ -# if defined(__GNUC__) && !defined(__clang__) +# if defined(__aarch64__) && defined(__GNUC__) && !defined(__clang__) ZSTD_seqSymbol llDInfoS, mlDInfoS, ofDInfoS; ZSTD_seqSymbol* const llDInfo = &llDInfoS; ZSTD_seqSymbol* const mlDInfo = &mlDInfoS; @@ -1348,7 +1348,7 @@ ZSTD_decodeSequence(seqState_t* seqState, const ZSTD_longOffset_e longOffsets, c seqState->prevOffset[0] = prevOffset0; seqState->prevOffset[1] = prevOffset1; seqState->prevOffset[2] = prevOffset2; -#else /* !defined(__aarch64__) */ +#else /* scalar-repoffset arm not selected */ const ZSTD_seqSymbol* const llDInfo = seqState->stateLL.table + seqState->stateLL.state; const ZSTD_seqSymbol* const mlDInfo = seqState->stateML.table + seqState->stateML.state; const ZSTD_seqSymbol* const ofDInfo = seqState->stateOffb.table + seqState->stateOffb.state; @@ -1441,7 +1441,7 @@ ZSTD_decodeSequence(seqState_t* seqState, const ZSTD_longOffset_e longOffsets, c BIT_reloadDStream(&seqState->DStream); } } -#endif /* defined(__aarch64__) */ +#endif /* scalar-repoffset arm */ return seq; }