[Mesa-dev] [PATCH] configure.ac: rework -latomic check
The configure.ac logic added in commit
2ef7f23820a67e958c2252bd81eb0458903ebf33 ("configure: check if
-latomic is needed for __atomic_*") makes the assumption that if a
64-bit atomic intrinsic test program fails to link without -latomic,
it is because we must use -latomic.
Unfortunately, this is not completely correct: libatomic only appeared
in gcc 4.8, and therefore gcc versions before that will not have
libatomic, and therefore don't provide atomic intrinsics for all
architectures. This issue was for example encountered on PowerPC with
a gcc 4.7 toolchain, where the build fails with:
powerpc-ctng_e500v2-linux-gnuspe/bin/ld: cannot find -latomic
This commit aims at fixing that, by not assuming -latomic is
available. The commit re-organizes the atomic intrinsics detection as
follows:
(1) Test if a program using 64-bit atomic intrinsics links properly,
without -latomic. If this is the case, we have atomic intrinsics,
and we're good to go.
(2) If (1) has failed, then test to link the same program, but this
time with -latomic in LDFLAGS. If this is the case, then we have
atomic intrinsics, provided we link with -latomic.
This has been tested in three situations:
- On x86-64, where atomic instrinsics are all built-in, with no need
for libatomic. In this case, config.log contains:
GCC_ATOMIC_BUILTINS_SUPPORTED_FALSE='#'
GCC_ATOMIC_BUILTINS_SUPPORTED_TRUE=''
LIBATOMIC_LIBS=''
This means: atomic intrinsics are available, and we don't need to
link with libatomic.
- On NIOS2, where atomic intrinsics are available, but some of them
(64-bit ones) require using libatomic. In this case, config.log
contains:
GCC_ATOMIC_BUILTINS_SUPPORTED_FALSE='#'
GCC_ATOMIC_BUILTINS_SUPPORTED_TRUE=''
LIBATOMIC_LIBS='-latomic'
This means: atomic intrinsics are available, and we need to link
with libatomic.
- On PowerPC with an old gcc 4.7 toolchain, where 32-bit atomic
instrinsics are available, but not 64-bit atomic instrinsics, and
there is no libatomic. In this case, config.log contains:
GCC_ATOMIC_BUILTINS_SUPPORTED_FALSE=''
GCC_ATOMIC_BUILTINS_SUPPORTED_TRUE='#'
With means that atomic intrinsics are not usable.
Signed-off-by: Thomas Petazzoni
---
configure.ac | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/configure.ac b/configure.ac
index f1fbdcc6c7..c94e547874 100644
--- a/configure.ac
+++ b/configure.ac
@@ -433,26 +433,31 @@ fi
AM_CONDITIONAL([SSE41_SUPPORTED], [test x$SSE41_SUPPORTED = x1])
AC_SUBST([SSE41_CFLAGS], $SSE41_CFLAGS)
-dnl Check for new-style atomic builtins
-AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+dnl Check for new-style atomic builtins. We first check without linking to
+dnl -latomic.
+AC_LINK_IFELSE([AC_LANG_SOURCE([[
+#include
int main() {
-int n;
-return __atomic_load_n(&n, __ATOMIC_ACQUIRE);
+uint64_t n;
+return (int)__atomic_load_n(&n, __ATOMIC_ACQUIRE);
}]])], GCC_ATOMIC_BUILTINS_SUPPORTED=1)
+
+dnl If that didn't work, we try linking with -latomic, which is needed on some
+dnl platforms.
+if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" != x1; then
+ save_LDFLAGS=$LDFLAGS
+ LDFLAGS="$LDFLAGS -latomic"
+ AC_LINK_IFELSE([AC_LANG_SOURCE([[
+ #include
+ int main() {
+uint64_t n;
+return (int)__atomic_load_n(&n, __ATOMIC_ACQUIRE);
+ }]])], GCC_ATOMIC_BUILTINS_SUPPORTED=1 LIBATOMIC_LIBS="-latomic")
+ LDFLAGS=$save_LDFLAGS
+fi
+
if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" = x1; then
DEFINES="$DEFINES -DUSE_GCC_ATOMIC_BUILTINS"
-dnl On some platforms, new-style atomics need a helper library
-AC_MSG_CHECKING(whether -latomic is needed)
-AC_LINK_IFELSE([AC_LANG_SOURCE([[
-#include
-uint64_t v;
-int main() {
-return (int)__atomic_load_n(&v, __ATOMIC_ACQUIRE);
-}]])], GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=no,
GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=yes)
-AC_MSG_RESULT($GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC)
-if test "x$GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC" = xyes; then
-LIBATOMIC_LIBS="-latomic"
-fi
fi
AC_SUBST([LIBATOMIC_LIBS])
--
2.14.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure.ac: rework -latomic check
Hello,
On Mon, 7 May 2018 14:07:05 -0700, Matt Turner wrote:
> On Mon, May 7, 2018 at 4:34 AM, Thomas Petazzoni
> wrote:
> > The configure.ac logic added in commit
> > 2ef7f23820a67e958c2252bd81eb0458903ebf33 ("configure: check if
> > -latomic is needed for __atomic_*") makes the assumption that if a
> > 64-bit atomic intrinsic test program fails to link without -latomic,
> > it is because we must use -latomic.
>
> I've confused. What is the alternative? Does this patch make Mesa
> work when it didn't before?
Yes, it makes Mesa build when it didn't before. I didn't runtime test
it though.
> Does the "whether __sync_add_and_fetch_8 is supported" test then
> detect that on your PPC system that we don't have 64-bit atomics and
> choose to enable -DMISSING_64BIT_ATOMICS?
Yes, -DMISSING_64BIT_ATOMICS ends up being defined.
> If that's the case, them my understanding is that we incorrectly
> assume -latomic is needed when in fact there's nothing we can do to
> make 64-bit __atomic built-ins work on that platform.
With that specific old gcc version. With newer gcc versions, libatomic
is provided, and 64-bit atomics are therefore available.
> So we're currently deciding that __atomic built-ins are usable since
> we're only compiling-testing a 32-bit operation, and then wrongly
> assuming that -latomic can make 64-bit operations work. Is that right?
The current autoconf code is doing this:
- Let's *compile* (not link) a 32-bit atomic test program. If it
works, we for now assume GCC_ATOMIC_BUILTINS_SUPPORTED=1
- If GCC_ATOMIC_BUILTINS_SUPPORTED=1 thanks to the previous test, we
*link* a 64-bit atomic test program, with no special linker flags.
If it works, we're all set. If it doesn't, we assume that by adding
-latomic to LIBS, everything will be OK.
It is this last part that is wrong: it *assumes* that adding -latomic
will always guarantee that 64-bit atomics will be available, but that
is wrong. It forget to actually verify that, and my patch simply
changes that.
Hopefully this clarifies what is happening here. Don't hesitate to ask
for further details if needed.
Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure.ac: rework -latomic check
Hello Matt,
On Wed, 9 May 2018 16:30:12 -0700, Matt Turner wrote:
> Hi Thomas,
>
> I rebased this patch on
>
> commit 54ba73ef102f7b9085922686bb31719539e0dc3c
> Author: Nicolas Boichat
> Date: Thu Apr 5 09:33:09 2018 +0800
>
> configure.ac/meson.build: Fix -latomic test
>
> and readded the AC_MSG_CHECKING/AC_MSG_RESULT. If it looks good to you,
> I'll commit it.
Thanks for this new iteration! I didn't test it, but it looks good to
me. Just one minor nit below.
> -dnl Check for new-style atomic builtins
> -AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
> +dnl Check for new-style atomic builtins. We first check without linking to
> +dnl -latomic.
> +AC_MSG_CHECKING(whether __atomic_load_n is supported)
> +AC_LINK_IFELSE([AC_LANG_SOURCE([[
> +#include
> int main() {
> -int n;
> -return __atomic_load_n(&n, __ATOMIC_ACQUIRE);
> -}]])], GCC_ATOMIC_BUILTINS_SUPPORTED=1)
> -if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" = x1; then
> -DEFINES="$DEFINES -DUSE_GCC_ATOMIC_BUILTINS"
> -dnl On some platforms, new-style atomics need a helper library
> -AC_MSG_CHECKING(whether -latomic is needed)
> -AC_LINK_IFELSE([AC_LANG_SOURCE([[
> -#include
> struct {
> uint64_t *v;
> } x;
> -int main() {
> +return (int)__atomic_load_n(x.v, __ATOMIC_ACQUIRE);
> +}]])], GCC_ATOMIC_BUILTINS_SUPPORTED=yes, GCC_ATOMIC_BUILTINS_SUPPORTED=no)
> +
> +dnl If that didn't work, we try linking with -latomic, which is needed on
> some
> +dnl platforms.
> +if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" != xyes; then
> + save_LDFLAGS=$LDFLAGS
> + LDFLAGS="$LDFLAGS -latomic"
> + AC_LINK_IFELSE([AC_LANG_SOURCE([[
> + #include
> + int main() {
> +struct {
> +uint64_t *v;
> +} x;
> return (int)__atomic_load_n(x.v, __ATOMIC_ACQUIRE);
> -}]])], GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=no,
> GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=yes)
> -AC_MSG_RESULT($GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC)
> -if test "x$GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC" = xyes; then
> -LIBATOMIC_LIBS="-latomic"
> -fi
> + }]])], GCC_ATOMIC_BUILTINS_SUPPORTED=yes LIBATOMIC_LIBS="-latomic",
> + GCC_ATOMIC_BUILTINS_SUPPORTED=no)
> + LDFLAGS=$save_LDFLAGS
> +
> +
Those two empty lines don't seem to be very useful.
> +fi
> +AC_MSG_RESULT($GCC_ATOMIC_BUILTINS_SUPPORTED)
> +
> +if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" = xyes; then
> +DEFINES="$DEFINES -DUSE_GCC_ATOMIC_BUILTINS"
> fi
> AC_SUBST([LIBATOMIC_LIBS])
>
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
