Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions contrib/linux-kernel/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,51 @@ LINUX_ZSTD_FILES := $(LINUX_ZSTD_MODULE) $(LINUX_ZSTD_COMMON) $(LINUX_ZSTD_
LINUX_ZSTD_OBJECTS0 := $(LINUX_ZSTD_FILES:.c=.o)
LINUX_ZSTD_OBJECTS := $(LINUX_ZSTD_OBJECTS0:.S=.o)

X86_BUILD_DIR := x86
X86_LINUX_ZSTD_FILES := $(patsubst $(LINUX_ZSTDLIB)/%,%,$(LINUX_ZSTD_FILES))
X86_LINUX_ZSTD_OBJECTS0 := $(patsubst %.c,$(X86_BUILD_DIR)/%.o,$(X86_LINUX_ZSTD_FILES))
X86_LINUX_ZSTD_OBJECTS := $(patsubst %.S,$(X86_BUILD_DIR)/%.o,$(X86_LINUX_ZSTD_OBJECTS0))
X86_LINUX_ZSTD_CPPFLAGS := -D__KERNEL__ -DCONFIG_X86

%.o: %.S
$(COMPILE.S) $(OUTPUT_OPTION) $<

$(X86_LINUX_ZSTD_OBJECTS): CPPFLAGS += $(X86_LINUX_ZSTD_CPPFLAGS)

$(X86_BUILD_DIR)/%.o: $(LINUX_ZSTDLIB)/%.c
@mkdir -p $(@D)
$(COMPILE.c) $(OUTPUT_OPTION) $<

$(X86_BUILD_DIR)/%.o: $(LINUX_ZSTDLIB)/%.S
@mkdir -p $(@D)
$(COMPILE.S) $(OUTPUT_OPTION) $<

liblinuxzstd.a: $(LINUX_ZSTD_OBJECTS)
$(AR) $(ARFLAGS) $@ $^

liblinuxzstd_x86.a: $(X86_LINUX_ZSTD_OBJECTS)
$(AR) $(ARFLAGS) $@ $^

test: test.c liblinuxzstd.a
$(CC) $(LDFLAGS) $(CPPFLAGS) $(CFLAGS) $^ -o $@

test_x86: test.c liblinuxzstd_x86.a
$(CC) $(LDFLAGS) $(CPPFLAGS) $(CFLAGS) $^ -o $@

static_test: static_test.c
$(CC) $(LDFLAGS) $(CPPFLAGS) $(CFLAGS) $^ -o $@

run-test: test static_test
run-test: test test_x86 static_test
./macro-test.sh
./test
./test_x86
./static_test

.PHONY:
clean:
$(RM) -f $(LINUX_ZSTDLIB)/*.o
$(RM) -f $(LINUX_ZSTDLIB)/**/*.o
$(RM) -rf $(X86_BUILD_DIR)
$(RM) -f *.o *.a
$(RM) -f static_test
$(RM) -f test
$(RM) -f test test_x86
16 changes: 16 additions & 0 deletions contrib/linux-kernel/test/include/asm/cpufeature.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
* All rights reserved.
*
* This source code is licensed under both the BSD-style license (found in the
* LICENSE file in the root directory of this source tree) and the GPLv2 (found
* in the COPYING file in the root directory of this source tree).
* You may select, at your option, one of the above-listed licenses.
*/
#ifndef ASM_CPUFEATURE_H
#define ASM_CPUFEATURE_H

#define X86_FEATURE_BMI2 0
#define cpu_feature_enabled(feature) ((void)(feature), 0)

#endif
1 change: 1 addition & 0 deletions contrib/linux-kernel/test/macro-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ println ""
test_not_present "ZSTD_NO_INTRINSICS"
test_not_present "ZSTD_NO_UNUSED_FUNCTIONS"
test_not_present "ZSTD_LEGACY_SUPPORT"
test_not_present "ZSTD_LINUX_KERNEL"
test_not_present "STATIC_BMI2"
test_not_present "ZSTD_DLL_EXPORT"
test_not_present "ZSTD_DLL_IMPORT"
Expand Down
1 change: 1 addition & 0 deletions lib/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ cxx_library(
header_namespace='',
visibility=['PUBLIC'],
exported_headers=subdir_glob([
('common', 'bmi2.h'),
('common', 'compiler.h'),
]),
)
Expand Down
40 changes: 40 additions & 0 deletions lib/common/bmi2.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
* All rights reserved.
*
* This source code is licensed under both the BSD-style license (found in the
* LICENSE file in the root directory of this source tree) and the GPLv2 (found
* in the COPYING file in the root directory of this source tree).
* You may select, at your option, one of the above-listed licenses.
*/

#ifndef ZSTD_BMI2_H
#define ZSTD_BMI2_H

#include "portability_macros.h"

#if defined(ZSTD_USE_KERNEL_CPU_FEATURES)
# include <asm/cpufeature.h>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this kernel-only path compiled by any CI job?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uarif1 We can test it, at least that all the code compiles as expected

  1. Add a fake header in contrib/linux-kernel/test/include/asm/cpufeature.h that defines the required symbols & macros so that cpu_feature_enabled(X86_FEATURE_BMI2) has a mock implementation (e.g. just always return 0).
  2. Add another version of test in contrib/linux-kernel/test/Makefile that defines CONFIG_X86 so that ZSTD_USE_KERNEL_CPU_FEATURES is enabled.

This will then run in our CI.

#endif

/*
* Select between the BMI2 and default implementations at the final dispatch
* site. Integrations may ignore the caller-provided hint and consult their own
* CPU-feature policy instead.
*
* ZSTD_USE_BMI2() may not evaluate its argument. ZSTD_SET_BMI2() may not
* evaluate its value argument, which avoids executing the userspace CPUID
* probe when the integration supplies the dispatch decision.
*/
#if !DYNAMIC_BMI2
# 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

# define ZSTD_SET_BMI2(state, value) do { (state) = 0; } while (0)
#else
# define ZSTD_USE_BMI2(bmi2) (bmi2)
# define ZSTD_SET_BMI2(state, value) do { (state) = (value); } while (0)
#endif

#endif /* ZSTD_BMI2_H */
13 changes: 7 additions & 6 deletions lib/common/entropy_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
/* *************************************
* Dependencies
***************************************/
#include "bmi2.h"
#include "mem.h"
#include "error_private.h" /* ERR_*, ERROR */
#define FSE_STATIC_LINKING_ONLY /* FSE_MIN_TABLELOG */
Expand Down Expand Up @@ -201,17 +202,17 @@ BMI2_TARGET_ATTRIBUTE static size_t FSE_readNCount_body_bmi2(
{
return FSE_readNCount_body(normalizedCounter, maxSVPtr, tableLogPtr, headerBuffer, hbSize);
}
#else
#define FSE_readNCount_body_bmi2 FSE_readNCount_body_default
#endif

size_t FSE_readNCount_bmi2(
short* normalizedCounter, unsigned* maxSVPtr, unsigned* tableLogPtr,
const void* headerBuffer, size_t hbSize, int bmi2)
{
#if DYNAMIC_BMI2
if (bmi2) {
if (ZSTD_USE_BMI2(bmi2)) {
return FSE_readNCount_body_bmi2(normalizedCounter, maxSVPtr, tableLogPtr, headerBuffer, hbSize);
}
#endif
(void)bmi2;
return FSE_readNCount_body_default(normalizedCounter, maxSVPtr, tableLogPtr, headerBuffer, hbSize);
}
Expand Down Expand Up @@ -322,6 +323,8 @@ static BMI2_TARGET_ATTRIBUTE size_t HUF_readStats_body_bmi2(BYTE* huffWeight, si
{
return HUF_readStats_body(huffWeight, hwSize, rankStats, nbSymbolsPtr, tableLogPtr, src, srcSize, workSpace, wkspSize, 1);
}
#else
#define HUF_readStats_body_bmi2 HUF_readStats_body_default
#endif

size_t HUF_readStats_wksp(BYTE* huffWeight, size_t hwSize, U32* rankStats,
Expand All @@ -330,11 +333,9 @@ size_t HUF_readStats_wksp(BYTE* huffWeight, size_t hwSize, U32* rankStats,
void* workSpace, size_t wkspSize,
int flags)
{
#if DYNAMIC_BMI2
if (flags & HUF_flags_bmi2) {
if (ZSTD_USE_BMI2(flags & HUF_flags_bmi2)) {
return HUF_readStats_body_bmi2(huffWeight, hwSize, rankStats, nbSymbolsPtr, tableLogPtr, src, srcSize, workSpace, wkspSize);
}
#endif
(void)flags;
return HUF_readStats_body_default(huffWeight, hwSize, rankStats, nbSymbolsPtr, tableLogPtr, src, srcSize, workSpace, wkspSize);
}
7 changes: 4 additions & 3 deletions lib/common/fse_decompress.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
****************************************************************/
#include "debug.h" /* assert */
#include "bitstream.h"
#include "bmi2.h"
#include "compiler.h"
#define FSE_STATIC_LINKING_ONLY
#include "fse.h"
Expand Down Expand Up @@ -299,15 +300,15 @@ BMI2_TARGET_ATTRIBUTE static size_t FSE_decompress_wksp_body_bmi2(void* dst, siz
{
return FSE_decompress_wksp_body(dst, dstCapacity, cSrc, cSrcSize, maxLog, workSpace, wkspSize, 1);
}
#else
#define FSE_decompress_wksp_body_bmi2 FSE_decompress_wksp_body_default
#endif

size_t FSE_decompress_wksp_bmi2(void* dst, size_t dstCapacity, const void* cSrc, size_t cSrcSize, unsigned maxLog, void* workSpace, size_t wkspSize, int bmi2)
{
#if DYNAMIC_BMI2
if (bmi2) {
if (ZSTD_USE_BMI2(bmi2)) {
return FSE_decompress_wksp_body_bmi2(dst, dstCapacity, cSrc, cSrcSize, maxLog, workSpace, wkspSize);
}
#endif
(void)bmi2;
return FSE_decompress_wksp_body_default(dst, dstCapacity, cSrc, cSrcSize, maxLog, workSpace, wkspSize);
}
Expand Down
12 changes: 12 additions & 0 deletions lib/common/portability_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,18 @@
# endif
#endif

/*
* The Linux kernel importer consumes the outer guard while preserving the
* distinction below. Normal x86 kernel objects can use the kernel CPU-feature
* policy, while preboot objects cannot use that infrastructure.
*/
#if defined(ZSTD_LINUX_KERNEL)
# if defined(__KERNEL__) && defined(CONFIG_X86) && \
!defined(__DISABLE_EXPORTS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

# define ZSTD_USE_KERNEL_CPU_FEATURES
# endif
#endif

/**
* Only enable assembly for GNU C compatible compilers,
* because other platforms may not support GAS assembly syntax.
Expand Down
4 changes: 3 additions & 1 deletion lib/compress/huf_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* Includes
****************************************************************/
#include "../common/zstd_deps.h" /* ZSTD_memcpy, ZSTD_memset */
#include "../common/bmi2.h"
#include "../common/compiler.h"
#include "../common/bitstream.h"
#include "hist.h"
Expand Down Expand Up @@ -1141,9 +1142,10 @@ HUF_compress1X_usingCTable_internal(void* dst, size_t dstSize,
const void* src, size_t srcSize,
const HUF_CElt* CTable, const int flags)
{
if (flags & HUF_flags_bmi2) {
if (ZSTD_USE_BMI2(flags & HUF_flags_bmi2)) {
return HUF_compress1X_usingCTable_internal_bmi2(dst, dstSize, src, srcSize, CTable);
}
(void)flags;
return HUF_compress1X_usingCTable_internal_default(dst, dstSize, src, srcSize, CTable);
}

Expand Down
5 changes: 3 additions & 2 deletions lib/compress/zstd_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
***************************************/
#include "../common/allocations.h" /* ZSTD_customMalloc, ZSTD_customCalloc, ZSTD_customFree */
#include "../common/zstd_deps.h" /* INT_MAX, ZSTD_memset, ZSTD_memcpy */
#include "../common/bmi2.h"
#include "../common/mem.h"
#include "../common/error_private.h"
#include "hist.h" /* HIST_countFast_wksp */
Expand Down Expand Up @@ -113,7 +114,7 @@ static void ZSTD_initCCtx(ZSTD_CCtx* cctx, ZSTD_customMem memManager)
ZSTD_memset(cctx, 0, sizeof(*cctx));
cctx->customMem = memManager;
#if DYNAMIC_BMI2
cctx->bmi2 = ZSTD_cpuSupportsBmi2();
ZSTD_SET_BMI2(cctx->bmi2, ZSTD_cpuSupportsBmi2());
#endif
{ size_t const err = ZSTD_CCtx_reset(cctx, ZSTD_reset_parameters);
assert(!ZSTD_isError(err));
Expand Down Expand Up @@ -155,7 +156,7 @@ ZSTD_CCtx* ZSTD_initStaticCCtx(void* workspace, size_t workspaceSize)
cctx->tmpWorkspace = ZSTD_cwksp_reserve_object(&cctx->workspace, TMP_WORKSPACE_SIZE);
cctx->tmpWkspSize = TMP_WORKSPACE_SIZE;
#if DYNAMIC_BMI2
cctx->bmi2 = ZSTD_cpuSupportsBmi2();
ZSTD_SET_BMI2(cctx->bmi2, ZSTD_cpuSupportsBmi2());
#endif
return cctx;
}
Expand Down
15 changes: 9 additions & 6 deletions lib/compress/zstd_compress_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,10 @@ 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 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. */
int bmi2; /* == 1 if the CPU supports BMI2 and 0 otherwise when CPU state
* is cached per context. Never read this directly: use ZSTD_CCtx_get_bmi2(),
* which also handles builds
* where cached CPU state is not used. */
#endif
ZSTD_CCtx_params requestedParams;
ZSTD_CCtx_params appliedParams;
Expand Down Expand Up @@ -550,13 +551,15 @@ struct ZSTD_CCtx_s {
};

/**
* @returns 1 if calls should be dispatched to the BMI2_TARGET_ATTRIBUTE variant
* of a function, rather than to its default variant.
* @returns the cached per-context BMI2 dispatch hint, or 0 when the build does
* not use cached CPU state.
*
* 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.
* support BMI2. Only use the result as a dispatch hint passed to a lower-level
* BMI2/default selector. Integrations may ignore the cached hint and consult
* their own CPU-feature policy at that selector.
*/
MEM_STATIC int ZSTD_CCtx_get_bmi2(const struct ZSTD_CCtx_s* cctx) {
#if DYNAMIC_BMI2
Expand Down
9 changes: 6 additions & 3 deletions lib/compress/zstd_compress_sequences.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
/*-*************************************
* Dependencies
***************************************/
#include "../common/bmi2.h"
#include "zstd_compress_sequences.h"

/**
Expand Down Expand Up @@ -414,6 +415,10 @@ ZSTD_encodeSequences_bmi2(
sequences, nbSeq, longOffsets);
}

#else

#define ZSTD_encodeSequences_bmi2 ZSTD_encodeSequences_default

#endif

size_t ZSTD_encodeSequences(
Expand All @@ -424,15 +429,13 @@ size_t ZSTD_encodeSequences(
SeqDef const* sequences, size_t nbSeq, int longOffsets, int bmi2)
{
DEBUGLOG(5, "ZSTD_encodeSequences: dstCapacity = %u", (unsigned)dstCapacity);
#if DYNAMIC_BMI2
if (bmi2) {
if (ZSTD_USE_BMI2(bmi2)) {
return ZSTD_encodeSequences_bmi2(dst, dstCapacity,
CTable_MatchLength, mlCodeTable,
CTable_OffsetBits, ofCodeTable,
CTable_LitLength, llCodeTable,
sequences, nbSeq, longOffsets);
}
#endif
(void)bmi2;
return ZSTD_encodeSequences_default(dst, dstCapacity,
CTable_MatchLength, mlCodeTable,
Expand Down
Loading
Loading