mirror of
https://github.com/facebook/zstd
synced 2026-08-22 22:32:07 -04:00
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.
This commit is contained in:
parent
82d322c497
commit
bcee628a22
1 changed files with 4 additions and 4 deletions
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue