Repository navigation
decompress: sink nextState/nbBits loads in ZSTD_decodeSequence - #4771
Merged
Merged
Conversation
The generic path loads all twelve fields of the three ZSTD_seqSymbol
entries up front, but the six nextState/nbBits values are not consumed
until the final FSE state update. Keeping them live across the whole
offset, matchLength and litLength decode costs six registers, and gcc
11.5.0 -O3 loses that fight: two of the three nextState values are
spilled and reloaded per sequence in the hot loop of
ZSTD_decompressSequences_bmi2().
mov %bx,0x20(%rsp)
...
mov %bx,0x28(%rsp)
...
movzwl 0x20(%rsp),%ecx
...
movzwl 0x28(%rsp),%ecx
clang 22.1.3 is worse, not better: it materialises all six at the top
of the block and spills four of them immediately, so each makes a
memory -> register -> stack -> register round trip before it is used.
d3af: movzwl (%rsi,%rdi,8),%r13d
d3b4: mov %r13,0x178(%rsp)
Read them at the point of use instead, turning each spill/reload pair
back into an L1 hit on a line already touched for nbAdditionalBits and
baseValue. That removes all of the nextState spill/reload pairs, in
every ZSTD_decompressSequences* instantiation; for clang it holds at
-O2, -O3, ThinLTO and -march=native alike.
Measured on an AMD EPYC 9D85 over three 32 MiB corpora -- lorem ipsum
text, JSON log lines, C/Python source -- compressed at level 3 and
decompressed in memory with a reused DCtx. Throughput is the mean over
16 code layouts per revision, produced by linking a padding object
ahead of the library: layout alone moves this loop by up to 4%, so a
single build pair cannot resolve the effect.
128 KiB payload before after
gcc text 1230.1 1309.0 +6.4%
source 1435.1 1476.8 +2.9%
JSON 1777.3 1786.3 +0.5%
clang text 958.0 990.4 +3.4%
source 1205.1 1229.2 +2.0%
JSON 1537.4 1533.1 -0.3%
16 KiB payloads track the same pattern, +1.1% to +4.3% for gcc and
-0.8% to +2.4% for clang. 4 KiB payloads are flat, as expected: there
the per-block header decode dominates over the sequence loop.
Signed-off-by: Usama Arif <usama.arif@linux.dev>
uarif1
force-pushed
the
zstd_register_skip
branch
from
September 14, 2026 13:52
9a1592d to
cc10195
Compare
terrelln
approved these changes
Sep 14, 2026
Contributor
|
With Apple Clang 21.0.0 on M4 Mac (ARM) this compiles to exactly the same binary before & after. |
Author
|
@terrelln that is the expected result, the patch only edits the #else /* !defined(aarch64) */ arm, so on any target where aarch64 is defined - which includes Apple Silicon - the compiler sees identical input. This should help x86 and other archs. |
Contributor
|
Good point, I forgot there was a second copy of the loop for aarch64. I'd love to get rid of that... |
Cyan4973
approved these changes
Sep 18, 2026
Cyan4973
left a comment
Contributor
There was a problem hiding this comment.
I'm impressed this could have such a large impact.
I would have expected the compiler to allocate registers less mechanically.
Yet another proof that one should check the output to be certain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The generic path loads all twelve fields of the three ZSTD_seqSymbol
entries up front, but the six nextState/nbBits values are not consumed
until the final FSE state update. Keeping them live across the whole
offset, matchLength and litLength decode costs six registers, and gcc
11.5.0 -O3 loses that fight: two of the three nextState values are
spilled and reloaded per sequence in the hot loop of
ZSTD_decompressSequences_bmi2().
clang 22.1.3 is worse, not better: it materialises all six at the top
of the block and spills four of them immediately, so each makes a
memory -> register -> stack -> register round trip before it is used.
Read them at the point of use instead, turning each spill/reload pair
back into an L1 hit on a line already touched for nbAdditionalBits and
baseValue. That removes all of the nextState spill/reload pairs, in
every ZSTD_decompressSequences* instantiation; for clang it holds at
-O2, -O3, ThinLTO and -march=native alike.
Measured on an AMD EPYC 9D85 over three 32 MiB corpora -- lorem ipsum
text, JSON log lines, C/Python source -- compressed at level 3 and
decompressed in memory with a reused DCtx. Throughput is the mean over
16 code layouts per revision, produced by linking a padding object
ahead of the library: layout alone moves this loop by up to 4%, so a
single build pair cannot resolve the effect.
16 KiB payloads track the same pattern, +1.1% to +4.3% for gcc and
-0.8% to +2.4% for clang. 4 KiB payloads are flat, as expected: there
the per-block header decode dominates over the sequence loop.