Repository navigation
zstd: use x86 feature infrastructure for BMI2 dispatch - #4767
Conversation
When dynamic BMI2 dispatch is disabled, the BMI2-specific functions are not compiled and each selector is conditionally compiled to avoid naming them. The selector-level preprocessor guards will be replaced with a predicate that becomes constant false when dynamic BMI2 dispatch is disabled. Although the compiler eliminates an if (0) branch, it must still parse and resolve the BMI2 function referenced by it. Add aliases from the unavailable BMI2 function names to their default implementations. These aliases make the names valid without emitting BMI2-specific code. The selectors remain unchanged in this patch, so the aliases are not used yet and there is no code-generation change. Signed-off-by: Usama Arif <usama.arif@linux.dev>
terrelln
left a comment
terrelln
left a comment
There was a problem hiding this comment.
A more targeted change would only update the places where this patch calls ZSTD_SET_BMI2, so that the kernel can use cpu_feature_enabled(X86_FEATURE_BMI2) there rather than ZSTD_cpuSupportsBmi2().
This would significantly shrink the patch size & make it easier to reason about. What is the advantage of making the larger change?
If we decide we want to keep the larger change, can we split it up into two parts:
- Only update the places where
ZSTD_SET_BMI2()is called - Everything else.
I know that @Cyan4973 is also working on some refactors in this space, so the more targeted change would be at less risk of conflict. We'll need to see what he says about these patches.
|
@terrelln A minimal patch would be something like https://gist.github.com/uarif1/721e033e3e498b810238560063774f4c. The reason the patch is in the current state is because of feedback from Linus: https://lore.kernel.org/all/CAHk-=whZf9=X+qKkO9_pQzbLdOnbMw5yO8gu0TkahegVW3cC-g@mail.gmail.com/ If the preference is for the minimal patch like the above gist, I can change it to that. |
Dynamic BMI2 dispatch probes CPUID when a compression or decompression
context is initialized. It caches the result in the context and tests the
caller-provided state at each final dispatch site. This is suitable for
standalone userspace, but prevents integrations such as the Linux kernel
from applying their own CPU feature policy at those sites.
Add ZSTD_USE_BMI2() for final selectors and ZSTD_SET_BMI2() for context
initialization. Their default definitions preserve existing userspace
behavior, while allowing an integration to replace both the selection
policy and cached state.
For the Linux kernel import, normal x86 objects now select BMI2 through
cpu_feature_enabled(X86_FEATURE_BMI2), allowing x86 alternatives to resolve
the check. Preboot objects, identified by __DISABLE_EXPORTS, retain the
existing CPUID-backed path because the normal kernel CPU feature
infrastructure is unavailable there.
Use the fallback aliases added by the preceding change when target-
attributed variants are absent, and verify that the import process consumes
ZSTD_LINUX_KERNEL.
A 4 KiB zstd-generic crypto_acomp benchmark in a one-vCPU KVM guest gave
these median results:
Before After Change
Compression 16,634 ns/op 13,394 ns/op -19.5%
Decompression 3,480 ns/op 963 ns/op -72.3%
Signed-off-by: Usama Arif <usama.arif@linux.dev>
4c20d56 to
86cb063
Compare
| # define ZSTD_USE_BMI2(bmi2) 0 | ||
| # define ZSTD_SET_BMI2(state, value) do { } while (0) | ||
| #elif defined(ZSTD_USE_KERNEL_CPU_FEATURES) | ||
| # define ZSTD_USE_BMI2(bmi2) cpu_feature_enabled(X86_FEATURE_BMI2) |
There was a problem hiding this comment.
This guard selects code compiled with BMI2_TARGET_ATTRIBUTE, which also permits BMI1 instructions. The existing runtime probe therefore checks both features.
Unless the kernel guarantees that X86_FEATURE_BMI2 implies X86_FEATURE_BMI1 after feature policy is applied, we should probably continue to check both: cpu_feature_enabled(X86_FEATURE_BMI1) && cpu_feature_enabled(X86_FEATURE_BMI2)
There was a problem hiding this comment.
So this came about because Linus Torvalds raised it on the mailing list in https://lore.kernel.org/all/CAHk-=whZf9=X+qKkO9_pQzbLdOnbMw5yO8gu0TkahegVW3cC-g@mail.gmail.com/
Mainly there is no CPU that has BMI2 but does not have BMI1. If there is a preference to add it, I can do it, but I believe just checking for BMI2 is sufficient.
| #include "portability_macros.h" | ||
|
|
||
| #if defined(ZSTD_USE_KERNEL_CPU_FEATURES) | ||
| # include <asm/cpufeature.h> |
There was a problem hiding this comment.
Is this kernel-only path compiled by any CI job?
There was a problem hiding this comment.
I can check, where are the jobs defined? I think if there is VM infrastructure, it would probably be quite complicated to add a test?
There was a problem hiding this comment.
@uarif1 We can test it, at least that all the code compiles as expected
- Add a fake header in
contrib/linux-kernel/test/include/asm/cpufeature.hthat defines the required symbols & macros so thatcpu_feature_enabled(X86_FEATURE_BMI2)has a mock implementation (e.g. just always return0). - Add another version of
testincontrib/linux-kernel/test/Makefilethat definesCONFIG_X86so thatZSTD_USE_KERNEL_CPU_FEATURESis enabled.
This will then run in our CI.
|
There is indeed a substantial overlap with the ongoing The proposed final-dispatch approach makes sense, it lets kernel alternatives optimize the dispatch sites directly. |
| #include "portability_macros.h" | ||
|
|
||
| #if defined(ZSTD_USE_KERNEL_CPU_FEATURES) | ||
| # include <asm/cpufeature.h> |
There was a problem hiding this comment.
@uarif1 We can test it, at least that all the code compiles as expected
- Add a fake header in
contrib/linux-kernel/test/include/asm/cpufeature.hthat defines the required symbols & macros so thatcpu_feature_enabled(X86_FEATURE_BMI2)has a mock implementation (e.g. just always return0). - Add another version of
testincontrib/linux-kernel/test/Makefilethat definesCONFIG_X86so thatZSTD_USE_KERNEL_CPU_FEATURESis enabled.
This will then run in our CI.
| */ | ||
| #if defined(ZSTD_LINUX_KERNEL) | ||
| # if defined(__KERNEL__) && defined(CONFIG_X86) && \ | ||
| !defined(__DISABLE_EXPORTS) |
There was a problem hiding this comment.
This means that we will continue to do BMI2 detection via cpuid in the pre-boot environment. Is this desired, or should we instead just disable the BMI2 detection entirely in preboot?
This might be a better question for on-list discussion.
The linux-kernel test harness compiles imported sources without __KERNEL__ or CONFIG_X86, so it does not exercise the ZSTD_USE_KERNEL_CPU_FEATURES path. Add a mock asm/cpufeature.h and a second test variant that compiles imported objects with __KERNEL__ and CONFIG_X86. Keep the objects in a separate tree so parallel builds do not reuse objects built with different flags. Signed-off-by: Usama Arif <usama.arif@linux.dev>
### Problem - On x64, each zstd context init runs `cpuid` twice. Under a hypervisor a `cpuid` is a VM exit, 2.05 us on an AWS Ice Lake guest. `Bun.zstdDecompressSync` of 1 KB went from 1.25 us (1.3.14) to 5.1 us (1.4.0, main). `node:zlib` zstd calls pay the same 4 us for each context. - The cause is `ZSTD_cpuSupportsBmi2()` (`lib/common/zstd_internal.h:318`), called from `ZSTD_initCCtx` and `ZSTD_initDCtx_internal`. zstd compiles it in when `__BMI2__` is not defined. That is so since #34782 made every x64 build target nehalem. ### Fix - `patches/zstd/bmi2-probe-once.patch` keeps the answer in a function-local static, read and written with seq_cst atomic builtins. CPU features do not change while the process runs, and zstd still selects its BMI2 kernels at run time. - Verified: `test/js/bun/util/zstd.test.ts` ("asks CPUID once"). The child turns on CPUID faulting (`arch_prctl(ARCH_SET_CPUID, 0)`) after one round trip, so a later `cpuid` raises SIGSEGV. It fails without the patch and passes with it (release and debug ASAN). It needs Linux x64 with the `cpuid_fault` CPU flag, which the CI x64 lane has. - Release build, 1 KB, median of 7 interleaved runs: `Bun.zstdDecompressSync` 5.11 to 1.38 us, `zlib.zstdDecompressSync` 15.24 to 10.42 us. - Self-reviewed: 1 concern raised, 1 addressed. It is the libjpeg-turbo note below: that library also probes for each object, and it is not part of this PR. ### Background - zstd has a BMI2 build and a plain build of its hot loops. With a compile target that has no BMI2, each context records which one to call. - In a VM `cpuid` always traps to the hypervisor. On bare metal it costs about 100 cycles. - meta/zstd#4767 (merged 2026-09-14) removes the probe for Linux kernel builds only. <details><summary>Notes</summary> - Found from a perf ledger entry: `zlib.zstdDecompressSync` of 1 KB went from 4.34 us (1.3.14) to about 15 us (1.4.0 to main). The zstd part of that is this probe. The rest is GC frequency, see #42958. - Profile: 139 of 200 main-thread samples of a `new NativeZstd` + `init` + `close` loop were at the instruction after `cpuid` in `ZSTD_createDCtx`. No page faults (`minflt` 0 per iteration), so it is not the 96 KB allocation. - `cpuid` cost on the test machine (Xeon Platinum 8375C guest, `hypervisor` flag set): 2,040 ns for each instruction, measured with a `noinline` loop. - Binary check: the bytes of `movq $0x8000001, off(%reg)` (the default `maxWindowSize`) have a `cpuid` in the 160 bytes before them in the 1.4.0, 1.4.2, and canary x64 Linux binaries. 1.3.14 has none, it was built with `-march=haswell`. - Standalone zstd at `-O3 -march=nehalem`, without and with the patch: `ZSTD_createDCtx` + `ZSTD_freeDCtx` 3,600 to 30 ns, `ZSTD_createCCtx` + `ZSTD_freeCCtx` 3,400 to 66 ns, `ZSTD_decompress` of 1 KB 3,900 to 390 ns, `ZSTD_compress` of 1 KB at level 3 4,700 to 1,320 ns. - The Bun A/B is one commit (09bb546) built without and with the patch, runs interleaved in alternating order, on a loaded machine. 1.3.14 for reference: `Bun.zstdDecompressSync` 1.25 us, `zlib.zstdCompressSync` 10.50 us, `zlib.zstdDecompressSync` 5.31 us. - The static is one for each translation unit that calls the function (`zstd_compress.c`, `zstd_decompress.c`), so a process runs at most 4 `cpuid` for zstd. - The test child sets `BUN_JSC_useBaselineJIT=0`. JSC runs `cpuid` as a serializing instruction each time it installs JIT code (`BaselineJITPlan::finalize`), which would fault too. `bun:ffi` needs `useJIT`, so the JIT stays on and only the baseline tier and above are off. - `arch_prctl(ARCH_SET_CPUID)` is per thread. The test covers the sync APIs, which init both context kinds on the JS thread. The async APIs and the streams call the same two init functions. - The patch does not use `__builtin_cpu_supports`, which would link the `__cpu_indicator_init` constructor that #41233 removed. - clang-cl 23.1.1 compiles the `__atomic_load_n` / `__atomic_store_n` builtins in C for x64 and arm64 Windows targets. - `ZSTD_initStaticCCtx` has its own `ZSTD_cpuid()` call. Bun does not use static contexts, so the patch leaves it. - Audit of every function with a `cpuid` in the x64 release binary (`objdump -d`): std_detect, compiler_builtins, mimalloc, highway, BoringSSL, simdutf, libdeflate, zlib-ng, libwebp, and JSC's `collectCPUFeatures` probe once for the process. JSC's JIT finalizers and thunk generators use `cpuid` as a serializing instruction when they install code. The crash handler probes at crash time. Two libraries probe for each object: zstd (the inlined `ZSTD_cpuSupportsBmi2` in `ZSTD_compress`, `ZSTD_decompress`, `ZSTDMT_createCCtxPool`, `bun_zstd::decompress_alloc`, the fetch decompressor, `CompressionStreamCoder__create`, and `NativeZstd` `Context::init`, all fixed by this one helper) and libjpeg-turbo. - Not part of this PR: libjpeg-turbo's `init_simd` (`simd/jsimd.c`) caches `jpeg_simd_cpu_support()` in each compress or decompress object (`cinfo->master->simd_support`), so each `Bun.Image` JPEG operation runs 3 `cpuid` and one `xgetbv`. It is a different dependency with a different patch and test, and it has its own follow-up. - Other suites run on the debug ASAN build: all of `test/js/bun/util/zstd.test.ts` (96 pass), `test/js/node/zlib/` (all but `leak.test.ts`, whose `beforeAll` exceeds the hook timeout on a debug build with and without this change and passes on the release build), and the 61 `test/js/node/test/parallel/test-zlib-*.js` files. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/util/zstd.test.ts <!-- robobun:evidence:end -->
Zstd currently probes CPUID whenever a compression or decompression
context is initialized, stores the result in the context, and tests that
value at each BMI2 dispatch site. For normal x86 kernel builds this
duplicates the kernel's CPU feature infrastructure, bypasses its feature
policy, and leaves an ordinary runtime test in the dispatch path.
Use cpu_feature_enabled(X86_FEATURE_BMI2) directly at the dispatch sites
for normal x86 kernel objects. This uses the x86 alternatives-backed
static CPU feature mechanism, allowing the feature test to be resolved at
boot instead of loading and testing a value stored in each context.
ZSTD_USE_BMI2() keeps the other build modes working as before. It expands
to the caller-provided flag for standalone and preboot builds and to false
when DYNAMIC_BMI2 is disabled. ZSTD_SET_BMI2() similarly stores the
caller-provided state only when it will be used, avoiding preprocessor
conditionals at the context initialization sites.
Patch 1 adds aliases from BMI2 function names to their default
implementations when the BMI2 variants are not compiled. This is a
no-functional-change preparation: after patch 2 removes the affected
selector-level preprocessor guards, the compiler must still resolve the
function named in an if (0) branch before eliminating it.
Patch 2 adds ZSTD_USE_BMI2() and ZSTD_SET_BMI2(), converts the runtime
selectors, and avoids Zstd's private CPUID probes in normal x86 kernel
objects. The kernel-specific policy lives in zstd_deps.h. Preboot builds
are excluded because the normal alternatives infrastructure is not
available there, so they retain the existing raw-CPUID dispatch.
A 4 KiB zstd-generic crypto_acomp benchmark [1] in a one-vCPU KVM guest
gave these median results:
Compression 16,634 ns 13,394 ns -19.5%
Decompression 3,480 ns 963 ns -72.3%
The improvement is especially large in a guest because raw CPUID causes
a VM exit.
This was originally posted on the kernel mailing list [2].
[1] https://gist.github.com/uarif1/5cf02f0e22c23f0d1b3d84348f12914c
[2] https://lore.kernel.org/all/20260901110850.1805747-1-usama.arif@linux.dev/