diff mbox series

V2 [PATCH 3/3] Build x86 libatomic with -march=i486 or better

Message ID CAMe9rOoetfrJtcMWHXHc9DTnKVvxzbYBx6Dq=-FGsUV9i=yxNw@mail.gmail.com
State New
Headers show
Series V2 [PATCH 3/3] Build x86 libatomic with -march=i486 or better | expand

Commit Message

H.J. Lu Jan. 15, 2021, 12:08 a.m. UTC 
On Thu, Jan 14, 2021 at 3:01 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Jan 14, 2021 at 01:04:31PM -0800, H.J. Lu via Gcc-patches wrote:
> > If x86 libatomic isn't compiled with -march=i486 or better, append
> > -march=i486 XCFLAGS for x86 libatomic build.  Set try_ifunc to yes
> > if -mcx16 isn't used to compile x86-64 libatomic or -march=i686 or
> > better isn't used to compile x86 libatomic.
> >
> >       PR target/70454
> >       * configure.tgt (XCFLAGS): Append -march=i486 to compile x86
> >       libatomic if needed.
> >       (try_ifunc): Set to yes only if needed.
> > ---
> >  libatomic/configure.tgt | 73 ++++++++++++++++++++++++++++-------------
> >  1 file changed, 50 insertions(+), 23 deletions(-)
> >
> > diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
> > index 2f24817b454..1f3a3ad6c7c 100644
> > --- a/libatomic/configure.tgt
> > +++ b/libatomic/configure.tgt
> > @@ -81,32 +81,59 @@ case "${target_cpu}" in
> >       ARCH=sparc
> >       ;;
> >
> > -  i[3456]86)
> > -     case " ${CC} ${CFLAGS} " in
> > -       *" -m64 "*|*" -mx32 "*)
> > -         ;;
> > -       *)
> > -         if test -z "$with_arch"; then
> > -           XCFLAGS="${XCFLAGS} -march=i486 -mtune=${target_cpu}"
> > -           XCFLAGS="${XCFLAGS} -fomit-frame-pointer"
> > -         fi
> > -     esac
> > -     ARCH=x86
> > -     # ??? Detect when -march=i686 is already enabled.
> > -     try_ifunc=yes
> > -     ;;
> > -  x86_64)
> > -     case " ${CC} ${CFLAGS} " in
> > -       *" -m32 "*)
> > +  i[3456]86 | x86_64)
> > +     # Need i486 or better.
> > +     cat > conftestx.c <<EOF
> > +#if defined __x86_64__ || defined __i486__ || defined __pentium__ \
> > +      || defined __pentiumpro__ || defined __pentium4__ \
> > +      || defined __geode__ || defined __SSE__
> > +# error Need i486 or better
> > +#endif
>
> Rather than hoping we got all the defines right, wouldn't it be better to
> compile with -S a testcase like:
> int
> foo (int *p, int x, int y)
> {
>   return __sync_val_compare_and_swap (p, x, y);
> }
> and if there is __sync_val_compare_and_swap_4 in the assembly assume
> -march=i486 needs to be added?
> I.e. test for what exactly we need (working atomics).
>
>         Jakub
>

Here is the updated patch.  OK for master?

Thanks.

Comments

Jakub Jelinek Jan. 15, 2021, 7:59 a.m. UTC | #1 
On Thu, Jan 14, 2021 at 04:08:20PM -0800, H.J. Lu wrote:

I think best would be to revert the i386-options.c change
until this is all fixed, keeping the trunk broken too long is undesirable.

Second, I didn't mean to talk specifically about libatomic, but about all
the 3 configure.tgt changes.
And while for the i486 and cmpxchg16b cases you now use a functional test,
for the i686 test you still use macros, and I don't e.g. see how __SSE__ is
relevant, one could have in CFLAGS -march=skylake-avx512 -mno-sse and it
wouldn't be considered an i686.
Now that I look at it, I think what you should be looking at is whether
the compiler with the ${CC} ${CFLAGS} predefines:
__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 - if not, then -march=i486 needs to be
added,
__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 for 32-bit code resp.
__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 for 64-bit code, then
maybe try_ifunc=no (but it needs verification that the code will in the end
always use cmpxchg8b or cmpxchg16b rather than never).

	Jakub
diff mbox series

Patch

From f847185c4faa94b6dbb52327c13d5d5bfec9b259 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 2 May 2016 09:44:07 -0700
Subject: [PATCH] Build x86 libatomic with -march=i486 or better

If x86 libatomic isn't compiled with -march=i486 or better, append
-march=i486 XCFLAGS for x86 libatomic build.  Set try_ifunc to yes
if -mcx16 isn't used to compile x86-64 libatomic or -march=i686 or
better isn't used to compile x86 libatomic.

	PR target/70454
	* configure.tgt (XCFLAGS): Append -march=i486 to compile x86
	libatomic if needed.
	(try_ifunc): Set to yes only if needed.
---
 libatomic/configure.tgt | 74 ++++++++++++++++++++++++++++-------------
 1 file changed, 51 insertions(+), 23 deletions(-)

diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
index 2f24817b454..3530c992264 100644
--- a/libatomic/configure.tgt
+++ b/libatomic/configure.tgt
@@ -81,32 +81,60 @@  case "${target_cpu}" in
 	ARCH=sparc
 	;;
 
-  i[3456]86)
-	case " ${CC} ${CFLAGS} " in
-	  *" -m64 "*|*" -mx32 "*)
-	    ;;
-	  *)
-	    if test -z "$with_arch"; then
-	      XCFLAGS="${XCFLAGS} -march=i486 -mtune=${target_cpu}"
-	      XCFLAGS="${XCFLAGS} -fomit-frame-pointer"
-	    fi
-	esac
-	ARCH=x86
-	# ??? Detect when -march=i686 is already enabled.
-	try_ifunc=yes
-	;;
-  x86_64)
-	case " ${CC} ${CFLAGS} " in
-	  *" -m32 "*)
+  i[3456]86 | x86_64)
+	# Need i486 or better.
+	cat > conftestx.c <<EOF
+int
+foo (int *p, int x, int y)
+{
+  return __sync_val_compare_and_swap (p, x, y);
+}
+EOF
+	if ${CC} ${CFLAGS} -S conftestx.c > /dev/null 2>&1 \
+	   && grep -q __sync_val_compare_and_swap conftestx.s /dev/null; then
+	  if test "${target_cpu}" = x86_64; then
 	    XCFLAGS="${XCFLAGS} -march=i486 -mtune=generic"
 	    XCFLAGS="${XCFLAGS} -fomit-frame-pointer"
-	    ;;
-	  *)
-	    ;;
-	esac
+	  else
+	    XCFLAGS="${XCFLAGS} -march=i486 -mtune=${target_cpu}"
+	    XCFLAGS="${XCFLAGS} -fomit-frame-pointer"
+	  fi
+	fi
+	# Detect if -march=i686/-mcx16 is already enabled.
+	cat > conftestx.c <<EOF
+#if defined __x86_64__
+__int128_t v = 0;
+__int128_t expected = 0;
+__int128_t max = ~0;
+__int128_t desired = ~0;
+__int128_t zero = 0;
+
+int
+foo (void)
+{
+  return !__atomic_compare_exchange_n (&v, &expected, max, 0,
+                                       __ATOMIC_RELAXED,
+                                       __ATOMIC_RELAXED);
+}
+#else
+# if defined __pentiumpro__ || defined __pentium4__ || defined __SSE__
+asm ("# has i686");
+# endif
+#endif
+EOF
+	if ${CC} ${CFLAGS} -S -o conftestx.s conftestx.c > /dev/null 2>&1; then
+	  if grep cmpxchg16b conftestx.s >/dev/null; then
+	    # This is the 64-bit library.
+	    try_ifunc=no
+	  elif grep i686 conftestx.s >/dev/null; then
+	    # This is the 32-bit library.
+	    try_ifunc=no
+	  else
+	    try_ifunc=yes
+	  fi
+	fi
+	rm -f conftestx.c conftestx.s
 	ARCH=x86
-	# ??? Detect when -mcx16 is already enabled.
-	try_ifunc=yes
 	;;
 
   *)			ARCH="${target_cpu}" ;;
-- 
2.29.2