From 9efb09749b85acfbc2299fb6dec6146b942c6b2e Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 18 Jan 2025 20:19:55 -0800 Subject: [PATCH 1/6] added a CI test for x86 32-bit + avx2 combination which is expected to be quite rare, but nonetheless possible. This test is initially expected to fail, before integration of #4248 fix --- .github/workflows/dev-short-tests.yml | 13 ++++++++++++- lib/common/bitstream.h | 2 +- lib/common/compiler.h | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dev-short-tests.yml b/.github/workflows/dev-short-tests.yml index 18c66f4f6..fa28283f7 100644 --- a/.github/workflows/dev-short-tests.yml +++ b/.github/workflows/dev-short-tests.yml @@ -164,7 +164,7 @@ jobs: make clean bash tests/libzstd_builds.sh - gcc-make-tests-32bit: + gcc-make-all-32bit: runs-on: ubuntu-latest steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # tag=v4.1.1 @@ -174,6 +174,17 @@ jobs: make libc6install CFLAGS="-Werror -m32" make -j all32 + gcc-make-all-32bit-avx2: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # tag=v4.1.1 + - name: Make all, 32bit mode + run: | + sudo apt-get -qqq update + make libc6install + CFLAGS="-Werror -m32 -mavx2" make -j all32 + + gcc-8-make: runs-on: ubuntu-latest steps: diff --git a/lib/common/bitstream.h b/lib/common/bitstream.h index bc7118b37..88f448aa6 100644 --- a/lib/common/bitstream.h +++ b/lib/common/bitstream.h @@ -238,7 +238,7 @@ MEM_STATIC size_t BIT_closeCStream(BIT_CStream_t* bitC) BIT_addBitsFast(bitC, 1, 1); /* endMark */ BIT_flushBits(bitC); if (bitC->ptr >= bitC->endPtr) return 0; /* overflow detected */ - return (bitC->ptr - bitC->startPtr) + (bitC->bitPos > 0); + return (size_t)(bitC->ptr - bitC->startPtr) + (bitC->bitPos > 0); } diff --git a/lib/common/compiler.h b/lib/common/compiler.h index 2a8002288..84ce62349 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -210,7 +210,7 @@ /*Like DYNAMIC_BMI2 but for compile time determination of BMI2 support*/ #ifndef STATIC_BMI2 # if defined(_MSC_VER) && (defined(_M_X64) || defined(_M_I86)) -# ifdef __AVX2__ //MSVC does not have a BMI2 specific flag, but every CPU that supports AVX2 also supports BMI2 +# ifdef __AVX2__ /* MSVC does not have a BMI2 specific flag, but every CPU that supports AVX2 also supports BMI2 */ # define STATIC_BMI2 1 # endif # elif defined(__BMI2__) && defined(__x86_64__) && defined(__GNUC__) From 35edbc20dc31fcc64f7dfe0c256d5005879b1b59 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 18 Jan 2025 20:38:48 -0800 Subject: [PATCH 2/6] added avx2 (x64) compilation test --- .github/workflows/dev-short-tests.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dev-short-tests.yml b/.github/workflows/dev-short-tests.yml index fa28283f7..6bd311f09 100644 --- a/.github/workflows/dev-short-tests.yml +++ b/.github/workflows/dev-short-tests.yml @@ -164,6 +164,16 @@ jobs: make clean bash tests/libzstd_builds.sh + gcc-make-all-avx2: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # tag=v4.1.1 + - name: Make all, with AVX2 + run: | + sudo apt-get -qqq update + make libc6install + CFLAGS="-Werror -mavx2" make -j all + gcc-make-all-32bit: runs-on: ubuntu-latest steps: @@ -178,7 +188,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # tag=v4.1.1 - - name: Make all, 32bit mode + - name: Make all, 32bit + AVX2 mode run: | sudo apt-get -qqq update make libc6install From f0b5f65bca6587f6d9b642e3a95d58ae36b7cdea Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 18 Jan 2025 20:43:30 -0800 Subject: [PATCH 3/6] fixed minor static function declaration issue in AVX2 mode only --- lib/compress/zstd_compress.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 5d5ada0ae..abc75fb2d 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -7125,7 +7125,7 @@ size_t ZSTD_compressSequences(ZSTD_CCtx* cctx, * @returns > 0 if there is one long length (> 65535), * indicating the position, and type. */ -size_t convertSequences_noRepcodes( +static size_t convertSequences_noRepcodes( SeqDef* dstSeqs, const ZSTD_Sequence* inSeqs, size_t nbSequences) @@ -7277,9 +7277,10 @@ size_t convertSequences_noRepcodes( #else /* no AVX2 */ -static size_t -convertSequences_noRepcodes(SeqDef* dstSeqs, - const ZSTD_Sequence* const inSeqs, size_t nbSequences) +static size_t convertSequences_noRepcodes( + SeqDef* dstSeqs, + const ZSTD_Sequence* inSeqs, + size_t nbSequences) { size_t longLen = 0; size_t n; From 050109589800be49d3840927b9e821d24622e1ea Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 18 Jan 2025 22:40:18 -0800 Subject: [PATCH 4/6] added -DSTATIC_BMI2=1 for the -mavx2 test --- .github/workflows/dev-short-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/dev-short-tests.yml b/.github/workflows/dev-short-tests.yml index 6bd311f09..71afc0991 100644 --- a/.github/workflows/dev-short-tests.yml +++ b/.github/workflows/dev-short-tests.yml @@ -192,7 +192,7 @@ jobs: run: | sudo apt-get -qqq update make libc6install - CFLAGS="-Werror -m32 -mavx2" make -j all32 + CPPFLAGS="-DSTATIC_BMI2=1" CFLAGS="-Werror -m32 -mavx2" make -j all32 gcc-8-make: From 27d794063162096e3a4be7d2d3f80ce117a5592d Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 18 Jan 2025 22:48:49 -0800 Subject: [PATCH 5/6] minor: cosmetic, indentation --- lib/common/compiler.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index 84ce62349..7927c4fb6 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -207,7 +207,7 @@ # pragma warning(disable : 4324) /* disable: C4324: padded structure */ #endif -/*Like DYNAMIC_BMI2 but for compile time determination of BMI2 support*/ +/* Like DYNAMIC_BMI2 but for compile time determination of BMI2 support */ #ifndef STATIC_BMI2 # if defined(_MSC_VER) && (defined(_M_X64) || defined(_M_I86)) # ifdef __AVX2__ /* MSVC does not have a BMI2 specific flag, but every CPU that supports AVX2 also supports BMI2 */ @@ -219,7 +219,7 @@ #endif #ifndef STATIC_BMI2 - #define STATIC_BMI2 0 +# define STATIC_BMI2 0 #endif /* compile time determination of SIMD support */ From d2d74616c0bfaf9d76186398a23fffafbb591c52 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 18 Jan 2025 22:58:03 -0800 Subject: [PATCH 6/6] also add `-mbmi2` to the compilation test --- .github/workflows/dev-short-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/dev-short-tests.yml b/.github/workflows/dev-short-tests.yml index 71afc0991..a84510686 100644 --- a/.github/workflows/dev-short-tests.yml +++ b/.github/workflows/dev-short-tests.yml @@ -192,7 +192,7 @@ jobs: run: | sudo apt-get -qqq update make libc6install - CPPFLAGS="-DSTATIC_BMI2=1" CFLAGS="-Werror -m32 -mavx2" make -j all32 + CPPFLAGS="-DSTATIC_BMI2=1" CFLAGS="-Werror -m32 -mavx2 -mbmi2" make -j all32 gcc-8-make: