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
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>
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/