Re: svn commit: r326506 - in head/sys/contrib/zstd/lib: common compress
On Mon, 4 Dec 2017, Allan Jude wrote:
Log:
Use __has_builtin() to ensure clz and clzll builtins are available
The existing check of the GCC version number is not sufficient
It also checked a wrong version number, and still doesn't.
This fixes the build on sparc64 in preparation for integrating ZSTD into
the kernel for ZFS and Crash Dumps.
This doesn't really work. __has_builtin() is a dummy (always 0) in
at least gcc-4.2.1 and earlier versions of gcc, and is broken as
designed in compilers that implement it. It just tells you if the
compiler supports the builtin, but you don't want to know about the
builtin unless the builtin will generate better code for the target
arch than your alternative. Using __has_builtin() is especially wrong
in the kernel and other pure freestanding cases. Then the builtin
generates a libcall to nonexistent function unless the target arch
supports a better inline alternative. Nonexistent functions are never
better alternatives than yours.
Modified: head/sys/contrib/zstd/lib/common/bitstream.h
==
--- head/sys/contrib/zstd/lib/common/bitstream.hMon Dec 4 01:14:17
2017(r326505)
+++ head/sys/contrib/zstd/lib/common/bitstream.hMon Dec 4 01:16:26
2017(r326506)
@@ -175,7 +175,7 @@ MEM_STATIC unsigned BIT_highbit32 (register U32 val)
unsigned long r=0;
_BitScanReverse ( &r, val );
return (unsigned) r;
-# elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */
This version check is very broken. gcc-3.3.3 doesn't have __builtin_clz(),
but passes the check.
+# elif defined(__GNUC__) && (__GNUC__ >= 3) && __has_builtin(__builtin_clz)
/* Use GCC Intrinsic */
gcc-4.2.1 doesn't have __builtin_clz() on x86, but not __has_builtin().
sys/cdefs.h defines __has_builtin() as 0 unless __has_builtin is defined.
The first part of the test is now usually just an obfuscation. It
still uses a wrong version number and a redumdant test that __GNUC__
is defined, but when (__GNUC__ < 3), __has_builtin() is usually 0
and gives the same result of 0. It is only in the unusual (unsupported)
case where the compiler doesn't pretend to be gcc but defines __has_builtin()
that the first part of the check has an effect, and then its effect is to
break the second part.
sys/cdefs.h gives an example of a similar but presumably-correct version
check. For using __builtin_unreachable(), it doesn't trust __has_builtin(),
but checks that the version is >= 4.6. 4.6 is fine-grained enough to have
a chance of being correct.
return 31 - __builtin_clz (val);
I think this is only for userland, so it is not broken when the builtin
reduce a libcall. The libcall is presumably to __clzdi2(). The kernel
doesn't have this, but libgcc.a does.
# else /* Software version */
static const unsigned DeBruijnClz[32] = { 0, 9, 1, 10, 13, 21, 2, 29,
This is unreachable when the libcall is used.
If optimizing this is actually important, then so not is pessimizing it on
arches with the builtin implemented in hardware but __has_builtin() not
implemented.
Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r326506 - in head/sys/contrib/zstd/lib: common compress
Author: allanjude
Date: Mon Dec 4 01:16:26 2017
New Revision: 326506
URL: https://svnweb.freebsd.org/changeset/base/326506
Log:
Use __has_builtin() to ensure clz and clzll builtins are available
The existing check of the GCC version number is not sufficient
This fixes the build on sparc64 in preparation for integrating ZSTD into
the kernel for ZFS and Crash Dumps.
Modified:
head/sys/contrib/zstd/lib/common/bitstream.h
head/sys/contrib/zstd/lib/common/zstd_internal.h
head/sys/contrib/zstd/lib/compress/zstd_compress.h
Modified: head/sys/contrib/zstd/lib/common/bitstream.h
==
--- head/sys/contrib/zstd/lib/common/bitstream.hMon Dec 4 01:14:17
2017(r326505)
+++ head/sys/contrib/zstd/lib/common/bitstream.hMon Dec 4 01:16:26
2017(r326506)
@@ -175,7 +175,7 @@ MEM_STATIC unsigned BIT_highbit32 (register U32 val)
unsigned long r=0;
_BitScanReverse ( &r, val );
return (unsigned) r;
-# elif defined(__GNUC__) && (__GNUC__ >= 3) /* Use GCC Intrinsic */
+# elif defined(__GNUC__) && (__GNUC__ >= 3) && __has_builtin(__builtin_clz)
/* Use GCC Intrinsic */
return 31 - __builtin_clz (val);
# else /* Software version */
static const unsigned DeBruijnClz[32] = { 0, 9, 1, 10, 13, 21, 2,
29,
Modified: head/sys/contrib/zstd/lib/common/zstd_internal.h
==
--- head/sys/contrib/zstd/lib/common/zstd_internal.hMon Dec 4 01:14:17
2017(r326505)
+++ head/sys/contrib/zstd/lib/common/zstd_internal.hMon Dec 4 01:16:26
2017(r326506)
@@ -327,7 +327,7 @@ MEM_STATIC U32 ZSTD_highbit32(U32 val)
unsigned long r=0;
_BitScanReverse(&r, val);
return (unsigned)r;
-# elif defined(__GNUC__) && (__GNUC__ >= 3) /* GCC Intrinsic */
+# elif defined(__GNUC__) && (__GNUC__ >= 3) && __has_builtin(__builtin_clz)
/* GCC Intrinsic */
return 31 - __builtin_clz(val);
# else /* Software version */
static const int DeBruijnClz[32] = { 0, 9, 1, 10, 13, 21, 2, 29, 11,
14, 16, 18, 22, 25, 3, 30, 8, 12, 20, 28, 15, 17, 24, 7, 19, 27, 23, 6, 26, 5,
4, 31 };
Modified: head/sys/contrib/zstd/lib/compress/zstd_compress.h
==
--- head/sys/contrib/zstd/lib/compress/zstd_compress.h Mon Dec 4 01:14:17
2017(r326505)
+++ head/sys/contrib/zstd/lib/compress/zstd_compress.h Mon Dec 4 01:16:26
2017(r326506)
@@ -203,7 +203,7 @@ static unsigned ZSTD_NbCommonBytes (register size_t va
unsigned long r = 0;
_BitScanReverse64( &r, val );
return (unsigned)(r>>3);
-# elif defined(__GNUC__) && (__GNUC__ >= 4)
+# elif defined(__GNUC__) && (__GNUC__ >= 4) &&
__has_builtin(__builtin_clzll)
return (__builtin_clzll(val) >> 3);
# else
unsigned r;
@@ -218,7 +218,7 @@ static unsigned ZSTD_NbCommonBytes (register size_t va
unsigned long r = 0;
_BitScanReverse( &r, (unsigned long)val );
return (unsigned)(r>>3);
-# elif defined(__GNUC__) && (__GNUC__ >= 3)
+# elif defined(__GNUC__) && (__GNUC__ >= 3) &&
__has_builtin(__builtin_clz)
return (__builtin_clz((U32)val) >> 3);
# else
unsigned r;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
