Skip to content

decompress: preserve the frame pointer in the Huffman x86-64 asm - #4811

Open
terrelln wants to merge 1 commit into
meta:devfrom
terrelln:2026-09-30-huffman-asm-frame-pointer
Open

terrelln wants to merge 1 commit into
meta:devfrom
terrelln:2026-09-30-huffman-asm-frame-pointer

Conversation

@terrelln

@terrelln terrelln commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

The x86-64 Huffman decoding loops used %rbp to hold bits[0], so while they ran %rbp didn't point to a valid frame. Profilers that unwind with frame pointers, such as perf record --call-graph=fp, couldn't unwind past these functions, so every sample inside them lost its callers.

Set up a standard frame (push %rbp; mov %rsp, %rbp) and keep %rbp intact for the whole function:

  • bits[0] moves from %rbp to %r8.
  • ip[0], which used to live in %r8, now lives on the stack. It is loaded into %r15 only around RELOAD_BITS(0). %r15 holds olimit and var[0] elsewhere, and neither is live during RELOAD_BITS(0).
  • In the 4X2 loop, DECODE stages the 2-byte symbol through %r15 instead of %r8, so %r8 no longer needs to be spilled around the decode step.

The CFI is updated to match. The CFA is defined relative to %rbp once the frame is set up, so the per-push .cfi_def_cfa_offset directives are dropped. On exit, the CFA switches back to %rsp after %rbp is popped.

Tested on x86-64 with BMI2, using gcc and clang, with and without -fcf-protection:

  • tests/fuzzer and tests/zstreamtest pass.
  • The CLI round-trips a range of inputs, levels and block sizes.
  • gdb backtraces reach main from the prologue, loop body, and epilogue of both loops.
  • With perf record --call-graph=fp, every sample inside the asm loops now unwinds to main. Before this change, none unwound past the leaf.
  • Decompression speed is unchanged, benchmarked with zstd -b1e1r silesia/.

The x86-64 Huffman decoding loops used %rbp to hold bits[0], so while
they ran %rbp didn't point to a valid frame. Profilers that unwind with
frame pointers, such as `perf record --call-graph=fp`, couldn't unwind
past these functions, so every sample inside them lost its callers.

Set up a standard frame (push %rbp; mov %rsp, %rbp) and keep %rbp
intact for the whole function:

* bits[0] moves from %rbp to %r8.
* ip[0], which used to live in %r8, now lives on the stack. It is
  loaded into %r15 only around RELOAD_BITS(0). %r15 holds olimit and
  var[0] elsewhere, and neither is live during RELOAD_BITS(0).
* In the 4X2 loop, DECODE stages the 2-byte symbol through %r15 instead
  of %r8, so %r8 no longer needs to be spilled around the decode step.

The CFI is updated to match. The CFA is defined relative to %rbp once
the frame is set up, so the per-push .cfi_def_cfa_offset directives are
dropped. On exit, the CFA switches back to %rsp after %rbp is popped.

Tested on x86-64 with BMI2, using gcc and clang, with and without
-fcf-protection:
* tests/fuzzer and tests/zstreamtest pass.
* The CLI round-trips a range of inputs, levels and block sizes.
* gdb backtraces reach main from the prologue, loop body, and
  epilogue of both loops.
* With `perf record --call-graph=fp`, every sample inside the asm loops
  now unwinds to main. Before this change, none unwound past the leaf.
* Decompression speed is unchanged.
@meta-cla meta-cla Bot added the CLA Signed label Oct 1, 2026
pps83 pushed a commit to pps83/zstd that referenced this pull request Oct 6, 2026
The windows-11-arm runner label now resolves to the
windows-11-vs2026-arm64 image (20260924), which ships Visual Studio
2026 and CMake 4.4, but no longer Visual Studio 2022. CMake fails at
configure time with "could not find any instance of Visual Studio", so
the job fails on every PR opened since 2026-10-01 (e.g. meta#4806, meta#4811,
meta#4813).

Switch that matrix entry to the "Visual Studio 18 2026" generator. The
x64 and Win32 entries run on windows-2022 and are unaffected.

Drop "VS2022" from the job name template, which would now be wrong for
the ARM64 entry: jobs become "CMake Windows (<entry>)". No branch
protection rule requires these checks by name.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant