From cc22042da0819b37cf69175bd92fe26a2975eea7 Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Fri, 24 Sep 2021 08:57:16 +0800 Subject: [PATCH 1/5] Fix a C89 error in msvc Variables (r) must be declared at the beginning of a code block. This causes msvc2012 to fail to compile 64-bit build. --- lib/compress/zstd_lazy.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index ed50550f3..fef28de8d 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -881,8 +881,11 @@ typedef U64 ZSTD_VecMask; /* Clarifies when we are interacting with a U64 repr static U32 ZSTD_VecMask_next(ZSTD_VecMask val) { assert(val != 0); # if defined(_MSC_VER) && defined(_WIN64) - unsigned long r=0; - return _BitScanForward64(&r, val) ? (U32)r : 0; /* _BitScanForward64 not defined outside of x86/64 */ + { + unsigned long r = 0; + /* _BitScanForward64 is not defined outside of x64 */ + return _BitScanForward64(&r, val) ? (U32)r : 0; + } # elif (defined(__GNUC__) && ((__GNUC__ > 3) || ((__GNUC__ == 3) && (__GNUC_MINOR__ >= 4)))) if (sizeof(size_t) == 4) { U32 mostSignificantWord = (U32)(val >> 32); From 95f492ea17834a6fb21013d2db0d03e0f12db025 Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Fri, 24 Sep 2021 19:24:29 +0800 Subject: [PATCH 2/5] Don't initialize the first parameter of _BitScanReverse* functions Like the document example, no need to initialize `r` to 0. https://docs.microsoft.com/en-us/cpp/intrinsics/bitscanreverse-bitscanreverse64 --- lib/common/bitstream.h | 2 +- lib/common/zstd_internal.h | 2 +- lib/compress/zstd_compress_internal.h | 4 ++-- lib/dictBuilder/zdict.c | 10 ++++------ lib/legacy/zstd_v01.c | 3 +-- lib/legacy/zstd_v02.c | 5 ++--- lib/legacy/zstd_v03.c | 5 ++--- lib/legacy/zstd_v04.c | 5 ++--- lib/legacy/zstd_v05.c | 5 ++--- lib/legacy/zstd_v06.c | 5 ++--- lib/legacy/zstd_v07.c | 5 ++--- 11 files changed, 21 insertions(+), 30 deletions(-) diff --git a/lib/common/bitstream.h b/lib/common/bitstream.h index 48aad7f3a..cefa1196b 100644 --- a/lib/common/bitstream.h +++ b/lib/common/bitstream.h @@ -145,7 +145,7 @@ MEM_STATIC unsigned BIT_highbit32 (U32 val) # if STATIC_BMI2 == 1 return _lzcnt_u32(val) ^ 31; # else - unsigned long r = 0; + unsigned long r; return _BitScanReverse(&r, val) ? (unsigned)r : 0; # endif # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index aaf7d4665..ba1b7bbc8 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -358,7 +358,7 @@ MEM_STATIC U32 ZSTD_highbit32(U32 val) /* compress, dictBuilder, decodeCorpus # if STATIC_BMI2 == 1 return _lzcnt_u32(val)^31; # else - unsigned long r=0; + unsigned long r; return _BitScanReverse(&r, val) ? (unsigned)r : 0; # endif # elif defined(__GNUC__) && (__GNUC__ >= 3) /* GCC Intrinsic */ diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 3dab83255..6821c9df8 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -708,7 +708,7 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # if STATIC_BMI2 return _lzcnt_u64(val) >> 3; # else - unsigned long r = 0; + unsigned long r; return _BitScanReverse64(&r, (U64)val) ? (unsigned)(r >> 3) : 0; # endif # elif defined(__GNUC__) && (__GNUC__ >= 4) @@ -723,7 +723,7 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r = 0; + unsigned long r; return _BitScanReverse( &r, (unsigned long)val ) ? (unsigned)(r >> 3) : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) return (__builtin_clz((U32)val) >> 3); diff --git a/lib/dictBuilder/zdict.c b/lib/dictBuilder/zdict.c index 9edc77fe8..2421f40f3 100644 --- a/lib/dictBuilder/zdict.c +++ b/lib/dictBuilder/zdict.c @@ -159,9 +159,8 @@ static unsigned ZDICT_NbCommonBytes (size_t val) } else { /* Big Endian CPU */ if (MEM_64bits()) { # if defined(_MSC_VER) && defined(_WIN64) - unsigned long r = 0; - _BitScanReverse64( &r, val ); - return (unsigned)(r>>3); + unsigned long r; + return _BitScanReverse64(&r, val) ? (unsigned)(r >> 3) : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_clzll(val) >> 3); # else @@ -174,9 +173,8 @@ static unsigned ZDICT_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r = 0; - _BitScanReverse( &r, (unsigned long)val ); - return (unsigned)(r>>3); + unsigned long r; + return _BitScanReverse(&r, (unsigned long)val) ? (unsigned)(r >> 3) : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_clz((U32)val) >> 3); # else diff --git a/lib/legacy/zstd_v01.c b/lib/legacy/zstd_v01.c index 7ab554797..6be9c8105 100644 --- a/lib/legacy/zstd_v01.c +++ b/lib/legacy/zstd_v01.c @@ -343,8 +343,7 @@ FORCE_INLINE unsigned FSE_highbit32 (U32 val) { # if defined(_MSC_VER) /* Visual */ unsigned long r; - _BitScanReverse ( &r, val ); - return (unsigned) r; + return _BitScanReverse(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (GCC_VERSION >= 304) /* GCC Intrinsic */ return __builtin_clz (val) ^ 31; # else /* Software version */ diff --git a/lib/legacy/zstd_v02.c b/lib/legacy/zstd_v02.c index 89fdc7169..f26b99bc8 100644 --- a/lib/legacy/zstd_v02.c +++ b/lib/legacy/zstd_v02.c @@ -353,9 +353,8 @@ MEM_STATIC size_t BIT_readBitsFast(BIT_DStream_t* bitD, unsigned nbBits); MEM_STATIC unsigned BIT_highbit32 (U32 val) { # if defined(_MSC_VER) /* Visual */ - unsigned long r=0; - _BitScanReverse ( &r, val ); - return (unsigned) r; + unsigned long r; + return _BitScanReverse(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ return __builtin_clz (val) ^ 31; # else /* Software version */ diff --git a/lib/legacy/zstd_v03.c b/lib/legacy/zstd_v03.c index 5262d515a..744bde515 100644 --- a/lib/legacy/zstd_v03.c +++ b/lib/legacy/zstd_v03.c @@ -356,9 +356,8 @@ MEM_STATIC size_t BIT_readBitsFast(BIT_DStream_t* bitD, unsigned nbBits); MEM_STATIC unsigned BIT_highbit32 (U32 val) { # if defined(_MSC_VER) /* Visual */ - unsigned long r=0; - _BitScanReverse ( &r, val ); - return (unsigned) r; + unsigned long r; + return _BitScanReverse(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ return __builtin_clz (val) ^ 31; # else /* Software version */ diff --git a/lib/legacy/zstd_v04.c b/lib/legacy/zstd_v04.c index bee1b99dd..4d336b59f 100644 --- a/lib/legacy/zstd_v04.c +++ b/lib/legacy/zstd_v04.c @@ -627,9 +627,8 @@ MEM_STATIC size_t BIT_readBitsFast(BIT_DStream_t* bitD, unsigned nbBits); MEM_STATIC unsigned BIT_highbit32 (U32 val) { # if defined(_MSC_VER) /* Visual */ - unsigned long r=0; - _BitScanReverse ( &r, val ); - return (unsigned) r; + unsigned long r; + return _BitScanReverse(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ return __builtin_clz (val) ^ 31; # else /* Software version */ diff --git a/lib/legacy/zstd_v05.c b/lib/legacy/zstd_v05.c index eb8966bb4..bc293661e 100644 --- a/lib/legacy/zstd_v05.c +++ b/lib/legacy/zstd_v05.c @@ -756,9 +756,8 @@ MEM_STATIC size_t BITv05_readBitsFast(BITv05_DStream_t* bitD, unsigned nbBits); MEM_STATIC unsigned BITv05_highbit32 (U32 val) { # if defined(_MSC_VER) /* Visual */ - unsigned long r=0; - _BitScanReverse ( &r, val ); - return (unsigned) r; + unsigned long r; + return _BitScanReverse(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ return __builtin_clz (val) ^ 31; # else /* Software version */ diff --git a/lib/legacy/zstd_v06.c b/lib/legacy/zstd_v06.c index fcb16d4d8..7d6a35922 100644 --- a/lib/legacy/zstd_v06.c +++ b/lib/legacy/zstd_v06.c @@ -860,9 +860,8 @@ MEM_STATIC size_t BITv06_readBitsFast(BITv06_DStream_t* bitD, unsigned nbBits); MEM_STATIC unsigned BITv06_highbit32 ( U32 val) { # if defined(_MSC_VER) /* Visual */ - unsigned long r=0; - _BitScanReverse ( &r, val ); - return (unsigned) r; + unsigned long r; + return _BitScanReverse(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ return __builtin_clz (val) ^ 31; # else /* Software version */ diff --git a/lib/legacy/zstd_v07.c b/lib/legacy/zstd_v07.c index 0d0e46609..1449ef3ab 100644 --- a/lib/legacy/zstd_v07.c +++ b/lib/legacy/zstd_v07.c @@ -530,9 +530,8 @@ MEM_STATIC size_t BITv07_readBitsFast(BITv07_DStream_t* bitD, unsigned nbBits); MEM_STATIC unsigned BITv07_highbit32 (U32 val) { # if defined(_MSC_VER) /* Visual */ - unsigned long r=0; - _BitScanReverse ( &r, val ); - return (unsigned) r; + unsigned long r; + return _BitScanReverse(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ return __builtin_clz (val) ^ 31; # else /* Software version */ From e5ba858270929080f03be821c53c93b8a05a42e0 Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Fri, 24 Sep 2021 19:40:24 +0800 Subject: [PATCH 3/5] Don't initialize the first parameter of _BitScanForward* functions Like the document example, no need to initialize `r` to 0. https://docs.microsoft.com/en-us/cpp/intrinsics/bitscanforward-bitscanforward64 --- lib/common/entropy_common.c | 2 +- lib/common/zstd_internal.h | 4 ++-- lib/compress/zstd_compress_internal.h | 4 ++-- lib/compress/zstd_lazy.c | 2 +- lib/dictBuilder/zdict.c | 10 ++++------ 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/common/entropy_common.c b/lib/common/entropy_common.c index 6ba1f2209..dd80b2ccc 100644 --- a/lib/common/entropy_common.c +++ b/lib/common/entropy_common.c @@ -43,7 +43,7 @@ static U32 FSE_ctz(U32 val) assert(val != 0); { # if defined(_MSC_VER) /* Visual */ - unsigned long r=0; + unsigned long r; return _BitScanForward(&r, val) ? (unsigned)r : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) /* GCC Intrinsic */ return __builtin_ctz(val); diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index ba1b7bbc8..246022c5f 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -391,7 +391,7 @@ MEM_STATIC unsigned ZSTD_countTrailingZeros(size_t val) # if STATIC_BMI2 return _tzcnt_u64(val); # else - unsigned long r = 0; + unsigned long r; return _BitScanForward64( &r, (U64)val ) ? (unsigned)(r >> 3) : 0; # endif # elif defined(__GNUC__) && (__GNUC__ >= 4) @@ -409,7 +409,7 @@ MEM_STATIC unsigned ZSTD_countTrailingZeros(size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r=0; + unsigned long r; return _BitScanForward( &r, (U32)val ) ? (unsigned)(r >> 3) : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) return (__builtin_ctz((U32)val) >> 3); diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 6821c9df8..bc30daa75 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -672,7 +672,7 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # if STATIC_BMI2 return _tzcnt_u64(val) >> 3; # else - unsigned long r = 0; + unsigned long r; return _BitScanForward64( &r, (U64)val ) ? (unsigned)(r >> 3) : 0; # endif # elif defined(__GNUC__) && (__GNUC__ >= 4) @@ -690,7 +690,7 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r=0; + unsigned long r; return _BitScanForward( &r, (U32)val ) ? (unsigned)(r >> 3) : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) return (__builtin_ctz((U32)val) >> 3); diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index fef28de8d..ac84b479f 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -882,7 +882,7 @@ static U32 ZSTD_VecMask_next(ZSTD_VecMask val) { assert(val != 0); # if defined(_MSC_VER) && defined(_WIN64) { - unsigned long r = 0; + unsigned long r; /* _BitScanForward64 is not defined outside of x64 */ return _BitScanForward64(&r, val) ? (U32)r : 0; } diff --git a/lib/dictBuilder/zdict.c b/lib/dictBuilder/zdict.c index 2421f40f3..ea00eafc7 100644 --- a/lib/dictBuilder/zdict.c +++ b/lib/dictBuilder/zdict.c @@ -135,9 +135,8 @@ static unsigned ZDICT_NbCommonBytes (size_t val) if (MEM_isLittleEndian()) { if (MEM_64bits()) { # if defined(_MSC_VER) && defined(_WIN64) - unsigned long r = 0; - _BitScanForward64( &r, (U64)val ); - return (unsigned)(r>>3); + unsigned long r; + return _BitScanForward64(&r, (U64)val) ? (unsigned)(r >> 3) : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_ctzll((U64)val) >> 3); # else @@ -146,9 +145,8 @@ static unsigned ZDICT_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r=0; - _BitScanForward( &r, (U32)val ); - return (unsigned)(r>>3); + unsigned long r; + return _BitScanForward(&r, (U32)val) ? (unsigned)(r >> 3) : 0; # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_ctz((U32)val) >> 3); # else From ae986fcdb861c44ee4267fc4b3dcf0a4cd2ee724 Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Sun, 26 Sep 2021 10:41:39 +0800 Subject: [PATCH 4/5] Use __assume(0) for unreachable code path in msvc msvc will optimize away the condition check. --- lib/common/bitstream.h | 12 ++++++-- lib/common/entropy_common.c | 10 +++++-- lib/common/zstd_internal.h | 30 ++++++++++++++++---- lib/compress/zstd_compress_internal.h | 40 +++++++++++++++++++++------ lib/compress/zstd_lazy.c | 13 +++++---- lib/dictBuilder/zdict.c | 40 +++++++++++++++++++++------ 6 files changed, 113 insertions(+), 32 deletions(-) diff --git a/lib/common/bitstream.h b/lib/common/bitstream.h index cefa1196b..696719eaf 100644 --- a/lib/common/bitstream.h +++ b/lib/common/bitstream.h @@ -143,10 +143,16 @@ MEM_STATIC unsigned BIT_highbit32 (U32 val) { # if defined(_MSC_VER) /* Visual */ # if STATIC_BMI2 == 1 - return _lzcnt_u32(val) ^ 31; + return _lzcnt_u32(val) ^ 31; # else - unsigned long r; - return _BitScanReverse(&r, val) ? (unsigned)r : 0; + if (val != 0) { + unsigned long r; + _BitScanReverse(&r, val); + return (unsigned)r; + } else { + /* Should not reach this code path */ + __assume(0); + } # endif # elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */ return __builtin_clz (val) ^ 31; diff --git a/lib/common/entropy_common.c b/lib/common/entropy_common.c index dd80b2ccc..3ac00255f 100644 --- a/lib/common/entropy_common.c +++ b/lib/common/entropy_common.c @@ -43,8 +43,14 @@ static U32 FSE_ctz(U32 val) assert(val != 0); { # if defined(_MSC_VER) /* Visual */ - unsigned long r; - return _BitScanForward(&r, val) ? (unsigned)r : 0; + if (val != 0) { + unsigned long r; + _BitScanForward(&r, val); + return (unsigned)r; + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) /* GCC Intrinsic */ return __builtin_ctz(val); # elif defined(__ICCARM__) /* IAR Intrinsic */ diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index 246022c5f..4db5b00a7 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -358,8 +358,14 @@ MEM_STATIC U32 ZSTD_highbit32(U32 val) /* compress, dictBuilder, decodeCorpus # if STATIC_BMI2 == 1 return _lzcnt_u32(val)^31; # else - unsigned long r; - return _BitScanReverse(&r, val) ? (unsigned)r : 0; + if (val != 0) { + unsigned long r; + _BitScanReverse(&r, val); + return (unsigned)r; + } else { + /* Should not reach this code path */ + __assume(0); + } # endif # elif defined(__GNUC__) && (__GNUC__ >= 3) /* GCC Intrinsic */ return __builtin_clz (val) ^ 31; @@ -391,8 +397,14 @@ MEM_STATIC unsigned ZSTD_countTrailingZeros(size_t val) # if STATIC_BMI2 return _tzcnt_u64(val); # else - unsigned long r; - return _BitScanForward64( &r, (U64)val ) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanForward64(&r, (U64)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # endif # elif defined(__GNUC__) && (__GNUC__ >= 4) return __builtin_ctzll((U64)val); @@ -409,8 +421,14 @@ MEM_STATIC unsigned ZSTD_countTrailingZeros(size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r; - return _BitScanForward( &r, (U32)val ) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanForward(&r, (U32)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) return (__builtin_ctz((U32)val) >> 3); # else diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index bc30daa75..cf0b8061a 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -672,8 +672,14 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # if STATIC_BMI2 return _tzcnt_u64(val) >> 3; # else - unsigned long r; - return _BitScanForward64( &r, (U64)val ) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanForward64(&r, (U64)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # endif # elif defined(__GNUC__) && (__GNUC__ >= 4) return (__builtin_ctzll((U64)val) >> 3); @@ -690,8 +696,14 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r; - return _BitScanForward( &r, (U32)val ) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanForward(&r, (U32)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) return (__builtin_ctz((U32)val) >> 3); # else @@ -708,8 +720,14 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # if STATIC_BMI2 return _lzcnt_u64(val) >> 3; # else - unsigned long r; - return _BitScanReverse64(&r, (U64)val) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanReverse64(&r, (U64)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # endif # elif defined(__GNUC__) && (__GNUC__ >= 4) return (__builtin_clzll(val) >> 3); @@ -723,8 +741,14 @@ static unsigned ZSTD_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r; - return _BitScanReverse( &r, (unsigned long)val ) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanReverse(&r, (unsigned long)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) return (__builtin_clz((U32)val) >> 3); # else diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index ac84b479f..cbac712a5 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -881,11 +881,14 @@ typedef U64 ZSTD_VecMask; /* Clarifies when we are interacting with a U64 repr static U32 ZSTD_VecMask_next(ZSTD_VecMask val) { assert(val != 0); # if defined(_MSC_VER) && defined(_WIN64) - { - unsigned long r; - /* _BitScanForward64 is not defined outside of x64 */ - return _BitScanForward64(&r, val) ? (U32)r : 0; - } + if (val != 0) { + unsigned long r; + _BitScanForward64(&r, val); + return (U32)(r); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif (defined(__GNUC__) && ((__GNUC__ > 3) || ((__GNUC__ == 3) && (__GNUC_MINOR__ >= 4)))) if (sizeof(size_t) == 4) { U32 mostSignificantWord = (U32)(val >> 32); diff --git a/lib/dictBuilder/zdict.c b/lib/dictBuilder/zdict.c index ea00eafc7..d93b202e7 100644 --- a/lib/dictBuilder/zdict.c +++ b/lib/dictBuilder/zdict.c @@ -135,8 +135,14 @@ static unsigned ZDICT_NbCommonBytes (size_t val) if (MEM_isLittleEndian()) { if (MEM_64bits()) { # if defined(_MSC_VER) && defined(_WIN64) - unsigned long r; - return _BitScanForward64(&r, (U64)val) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanForward64(&r, (U64)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_ctzll((U64)val) >> 3); # else @@ -145,8 +151,14 @@ static unsigned ZDICT_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r; - return _BitScanForward(&r, (U32)val) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanForward(&r, (U32)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_ctz((U32)val) >> 3); # else @@ -157,8 +169,14 @@ static unsigned ZDICT_NbCommonBytes (size_t val) } else { /* Big Endian CPU */ if (MEM_64bits()) { # if defined(_MSC_VER) && defined(_WIN64) - unsigned long r; - return _BitScanReverse64(&r, val) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanReverse64(&r, val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_clzll(val) >> 3); # else @@ -171,8 +189,14 @@ static unsigned ZDICT_NbCommonBytes (size_t val) # endif } else { /* 32 bits */ # if defined(_MSC_VER) - unsigned long r; - return _BitScanReverse(&r, (unsigned long)val) ? (unsigned)(r >> 3) : 0; + if (val != 0) { + unsigned long r; + _BitScanReverse(&r, (unsigned long)val); + return (unsigned)(r >> 3); + } else { + /* Should not reach this code path */ + __assume(0); + } # elif defined(__GNUC__) && (__GNUC__ >= 3) return (unsigned)(__builtin_clz((U32)val) >> 3); # else From 894f05e88d07dd1c791597540e91f1aa6149b67e Mon Sep 17 00:00:00 2001 From: Ma Lin Date: Tue, 28 Sep 2021 09:14:19 +0800 Subject: [PATCH 5/5] Fix ZSTD_countTrailingZeros() bug `>> 3` is wrong. --- lib/common/zstd_internal.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index 4db5b00a7..efd7360a0 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -385,10 +385,10 @@ MEM_STATIC U32 ZSTD_highbit32(U32 val) /* compress, dictBuilder, decodeCorpus } /** - * Computes CTZ on a U64. - * This will be slow on 32-bit mode, and on unsupported compilers. - * If you need this function to be fast (because it is hot) expand - * support. + * Counts the number of trailing zeros of a `size_t`. + * Most compilers should support CTZ as a builtin. A backup + * implementation is provided if the builtin isn't supported, but + * it may not be terribly efficient. */ MEM_STATIC unsigned ZSTD_countTrailingZeros(size_t val) { @@ -400,7 +400,7 @@ MEM_STATIC unsigned ZSTD_countTrailingZeros(size_t val) if (val != 0) { unsigned long r; _BitScanForward64(&r, (U64)val); - return (unsigned)(r >> 3); + return (unsigned)r; } else { /* Should not reach this code path */ __assume(0); @@ -424,13 +424,13 @@ MEM_STATIC unsigned ZSTD_countTrailingZeros(size_t val) if (val != 0) { unsigned long r; _BitScanForward(&r, (U32)val); - return (unsigned)(r >> 3); + return (unsigned)r; } else { /* Should not reach this code path */ __assume(0); } # elif defined(__GNUC__) && (__GNUC__ >= 3) - return (__builtin_ctz((U32)val) >> 3); + return __builtin_ctz((U32)val); # else static const int DeBruijnBytePos[32] = { 0, 1, 28, 2, 29, 14, 24, 3, 30, 22, 20, 15, 25, 17, 4, 8,