From b89e8c2aaf945718bf5a99e16d452728f12fc1e2 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 26 Aug 2026 22:51:57 -0700 Subject: [PATCH 1/2] perf: skip CPUID probe in CCtx when BMI2 is enabled at compile time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decompression side already compiles out its runtime BMI2 probe when DYNAMIC_BMI2 is 0. The compression side did not: cctx->bmi2 was unconditional, so both ZSTD_initCCtx() and ZSTD_initStaticCCtx() issued a CPUID sequence per context even under -mbmi2, where the result is unused. Mirror the decompression side: guard the field and both assignments, add ZSTD_CCtx_get_bmi2(). Also route ZSTD_initStaticCCtx() through ZSTD_cpuSupportsBmi2(), which checks BMI1 as well — required because BMI2_TARGET_ATTRIBUTE enables "lzcnt,bmi,bmi2". ZSTD_createCCtx + ZSTD_freeCCtx under -mbmi2: 2031 -> 100 ns; unchanged without it. Fuzzer and zstreamtest pass under -O2, -mbmi2, and -DDYNAMIC_BMI2=0. --- lib/compress/zstd_compress.c | 14 +++++++++----- lib/compress/zstd_compress_internal.h | 11 +++++++++++ lib/compress/zstd_compress_superblock.c | 2 +- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index c06f2e1bb98..50c3e6f7279 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -112,7 +112,9 @@ static void ZSTD_initCCtx(ZSTD_CCtx* cctx, ZSTD_customMem memManager) assert(cctx != NULL); ZSTD_memset(cctx, 0, sizeof(*cctx)); cctx->customMem = memManager; +#if DYNAMIC_BMI2 cctx->bmi2 = ZSTD_cpuSupportsBmi2(); +#endif { size_t const err = ZSTD_CCtx_reset(cctx, ZSTD_reset_parameters); assert(!ZSTD_isError(err)); (void)err; @@ -152,7 +154,9 @@ ZSTD_CCtx* ZSTD_initStaticCCtx(void* workspace, size_t workspaceSize) cctx->blockState.nextCBlock = (ZSTD_compressedBlockState_t*)ZSTD_cwksp_reserve_object(&cctx->workspace, sizeof(ZSTD_compressedBlockState_t)); cctx->tmpWorkspace = ZSTD_cwksp_reserve_object(&cctx->workspace, TMP_WORKSPACE_SIZE); cctx->tmpWkspSize = TMP_WORKSPACE_SIZE; - cctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid()); +#if DYNAMIC_BMI2 + cctx->bmi2 = ZSTD_cpuSupportsBmi2(); +#endif return cctx; } @@ -4152,7 +4156,7 @@ ZSTD_compressSeqStore_singleBlock(ZSTD_CCtx* zc, op + ZSTD_blockHeaderSize, dstCapacity - ZSTD_blockHeaderSize, srcSize, zc->tmpWorkspace, zc->tmpWkspSize /* statically allocated in resetCCtx */, - zc->bmi2); + ZSTD_CCtx_get_bmi2(zc)); FORWARD_IF_ERROR(cSeqsSize, "ZSTD_entropyCompressSeqStore failed!"); if (!zc->isFirstBlock && @@ -4442,7 +4446,7 @@ ZSTD_compressBlock_internal(ZSTD_CCtx* zc, dst, dstCapacity, srcSize, zc->tmpWorkspace, zc->tmpWkspSize /* statically allocated in resetCCtx */, - zc->bmi2); + ZSTD_CCtx_get_bmi2(zc)); if (frame && /* We don't want to emit our first block as a RLE even if it qualifies because @@ -7029,7 +7033,7 @@ ZSTD_compressSequences_internal(ZSTD_CCtx* cctx, op + ZSTD_blockHeaderSize /* Leave space for block header */, dstCapacity - ZSTD_blockHeaderSize, blockSize, cctx->tmpWorkspace, cctx->tmpWkspSize /* statically allocated in resetCCtx */, - cctx->bmi2); + ZSTD_CCtx_get_bmi2(cctx)); FORWARD_IF_ERROR(compressedSeqsSize, "Compressing sequences of block failed"); DEBUGLOG(5, "Compressed sequences size: %zu", compressedSeqsSize); @@ -8057,7 +8061,7 @@ ZSTD_compressSequencesAndLiterals_internal(ZSTD_CCtx* cctx, &cctx->blockState.prevCBlock->entropy, &cctx->blockState.nextCBlock->entropy, &cctx->appliedParams, cctx->tmpWorkspace, cctx->tmpWkspSize /* statically allocated in resetCCtx */, - cctx->bmi2); + ZSTD_CCtx_get_bmi2(cctx)); FORWARD_IF_ERROR(compressedSeqsSize, "Compressing sequences of block failed"); /* note: the spec forbids for any compressed block to be larger than maximum block size */ if (compressedSeqsSize > cctx->blockSizeMax) compressedSeqsSize = 0; diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 13a394b3816..a4b85be6e97 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -472,7 +472,9 @@ typedef struct { struct ZSTD_CCtx_s { ZSTD_compressionStage_e stage; int cParamsChanged; /* == 1 if cParams(except wlog) or compression level are changed in requestedParams. Triggers transmission of new params to ZSTDMT (if available) then reset to 0. */ +#if DYNAMIC_BMI2 int bmi2; /* == 1 if the CPU supports BMI2 and 0 otherwise. CPU support is determined dynamically once per context lifetime. */ +#endif ZSTD_CCtx_params requestedParams; ZSTD_CCtx_params appliedParams; ZSTD_CCtx_params simpleApiParams; /* Param storage used by the simple API - not sticky. Must only be used in top-level simple API functions for storage. */ @@ -545,6 +547,15 @@ struct ZSTD_CCtx_s { size_t extSeqBufCapacity; }; +MEM_STATIC int ZSTD_CCtx_get_bmi2(const struct ZSTD_CCtx_s* cctx) { +#if DYNAMIC_BMI2 + return cctx->bmi2; +#else + (void)cctx; + return 0; +#endif +} + typedef enum { ZSTD_dtlm_fast, ZSTD_dtlm_full } ZSTD_dictTableLoadMethod_e; typedef enum { ZSTD_tfp_forCCtx, ZSTD_tfp_forCDict } ZSTD_tableFillPurpose_e; diff --git a/lib/compress/zstd_compress_superblock.c b/lib/compress/zstd_compress_superblock.c index 6f57345be62..a1b4192cc3a 100644 --- a/lib/compress/zstd_compress_superblock.c +++ b/lib/compress/zstd_compress_superblock.c @@ -683,6 +683,6 @@ size_t ZSTD_compressSuperBlock(ZSTD_CCtx* zc, &zc->appliedParams, dst, dstCapacity, src, srcSize, - zc->bmi2, lastBlock, + ZSTD_CCtx_get_bmi2(zc), lastBlock, zc->tmpWorkspace, zc->tmpWkspSize /* statically allocated in resetCCtx */); } From 1fcad56180820e2ada36bc3d05def7352c9c2562 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 26 Aug 2026 23:28:56 -0700 Subject: [PATCH 2/2] doc: clarify that ZSTD_[CD]Ctx_get_bmi2() is not a CPU capability test Both bmi2 fields claimed "== 1 if the CPU supports BMI2 and 0 otherwise", which is false for the value consumers actually see: when BMI2 is enabled at compile time, DYNAMIC_BMI2 is 0 and the accessors return 0 even though the CPU does support BMI2. The value selects between the default and the BMI2_TARGET_ATTRIBUTE variant of a function; it is not a capability bit. Benign today, since every consumer ignores it when DYNAMIC_BMI2 is 0, but it invites future code to branch on it as a capability test and silently take the wrong path on the fastest machines. Comments only, no functional change. --- lib/compress/zstd_compress_internal.h | 13 ++++++++++++- lib/decompress/zstd_decompress_internal.h | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index a4b85be6e97..e233bc5ff1b 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -473,7 +473,9 @@ struct ZSTD_CCtx_s { ZSTD_compressionStage_e stage; int cParamsChanged; /* == 1 if cParams(except wlog) or compression level are changed in requestedParams. Triggers transmission of new params to ZSTDMT (if available) then reset to 0. */ #if DYNAMIC_BMI2 - int bmi2; /* == 1 if the CPU supports BMI2 and 0 otherwise. CPU support is determined dynamically once per context lifetime. */ + int bmi2; /* == 1 if the CPU supports BMI1 & BMI2, determined once per context lifetime. + * Never read this directly: use ZSTD_CCtx_get_bmi2(), which also handles builds + * where BMI2 is enabled at compile time and this field does not exist. */ #endif ZSTD_CCtx_params requestedParams; ZSTD_CCtx_params appliedParams; @@ -547,6 +549,15 @@ struct ZSTD_CCtx_s { size_t extSeqBufCapacity; }; +/** + * @returns 1 if calls should be dispatched to the BMI2_TARGET_ATTRIBUTE variant + * of a function, rather than to its default variant. + * + * Beware: this is *not* a "does the CPU support BMI2" test. When BMI2 is enabled + * at compile time, DYNAMIC_BMI2 is 0: the whole library is already compiled with + * BMI2, no separate variant exists, and this returns 0 even though the CPU does + * support BMI2. Only ever use it to select between the two variants of a function. + */ MEM_STATIC int ZSTD_CCtx_get_bmi2(const struct ZSTD_CCtx_s* cctx) { #if DYNAMIC_BMI2 return cctx->bmi2; diff --git a/lib/decompress/zstd_decompress_internal.h b/lib/decompress/zstd_decompress_internal.h index e4bffdbc6cf..07b9732b4d4 100644 --- a/lib/decompress/zstd_decompress_internal.h +++ b/lib/decompress/zstd_decompress_internal.h @@ -155,7 +155,9 @@ struct ZSTD_DCtx_s size_t staticSize; int isFrameDecompression; #if DYNAMIC_BMI2 - int bmi2; /* == 1 if the CPU supports BMI2 and 0 otherwise. CPU support is determined dynamically once per context lifetime. */ + int bmi2; /* == 1 if the CPU supports BMI1 & BMI2, determined once per context lifetime. + * Never read this directly: use ZSTD_DCtx_get_bmi2(), which also handles builds + * where BMI2 is enabled at compile time and this field does not exist. */ #endif /* dictionary */ @@ -210,6 +212,15 @@ struct ZSTD_DCtx_s #endif }; /* typedef'd to ZSTD_DCtx within "zstd.h" */ +/** + * @returns 1 if calls should be dispatched to the BMI2_TARGET_ATTRIBUTE variant + * of a function, rather than to its default variant. + * + * Beware: this is *not* a "does the CPU support BMI2" test. When BMI2 is enabled + * at compile time, DYNAMIC_BMI2 is 0: the whole library is already compiled with + * BMI2, no separate variant exists, and this returns 0 even though the CPU does + * support BMI2. Only ever use it to select between the two variants of a function. + */ MEM_STATIC int ZSTD_DCtx_get_bmi2(const struct ZSTD_DCtx_s *dctx) { #if DYNAMIC_BMI2 return dctx->bmi2;