diff mbox series

aarch64: Fix bootstrap with old binutils [PR93053]

Message ID 20200415072737.GT2424@tucnak
State New
Headers show
Series aarch64: Fix bootstrap with old binutils [PR93053] | expand

Commit Message

Jakub Jelinek April 15, 2020, 7:27 a.m. UTC 
Hi!

As reported in the PR, GCC 10 (and also 9.3.1 but not 9.3.0) fails to build
when using older binutils which lack LSE support, because those instructions
are used in libgcc.
Thanks to Kyrylo's hint, the following patches (hopefully) allow it to build
even with older binutils by using .inst directive if LSE support isn't
available in the assembler.

There are two versions of the patch, one is using .macro to ignore the
operands when emitting .inst, the other moves the operands for the
HAVE_AS_LSE case into the macro.  My slight preference is the latter,
because for maintanance it makes it clear that if one wants to use different
operands, one needs to use a different macro and different .inst
constant(s).

I've been testing this in distro scratch builds, so I don't really have
there the old assembler, but bootstrapped/regtested 5 times, for each of the
two patches once as is and once with s/cas/caszz/ in the configure{.ac,} to
simulate missing support of LSE (and verified that as is the build logs
contain checking...supports LSE yes and with the tweaked ...supports LSE no)
and once vanilla trunk, and aarch64-linux-gnu-objdump -dr libgcc.a is
identical from all 5 builds.

Ok for trunk (which version), or do we want to instead document a newer
binutils requirement?  Yet another option is to go with the patch for a
while and later bump the requirement and revert the patch.

	Jakub
2020-04-15  Jakub Jelinek  <jakub@redhat.com>

	PR target/93053
	* configure.ac (LIBGCC_CHECK_AS_LSE): Add HAVE_AS_LSE checking.
	* config/aarch64/lse.S: Include auto-target.h, if HAVE_AS_LSE
	is not defined, use just .arch armv8-a.
	(B, M, N, OPN): Define.
	(COMMENT): New .macro.
	(CAS, CASP, SWP, LDOP): Use .inst directive if HAVE_AS_LSE is not
	defined.
	* configure: Regenerated.
	* config.in: Regenerated.
2020-04-15  Jakub Jelinek  <jakub@redhat.com>

	PR target/93053
	* configure.ac (LIBGCC_CHECK_AS_LSE): Add HAVE_AS_LSE checking.
	* config/aarch64/lse.S: Include auto-target.h, if HAVE_AS_LSE
	is not defined, use just .arch armv8-a.
	(B, M, N, OPN): Define.
	(COMMENT): New .macro.
	(CAS, CASP, SWP, LDOP): Use .inst directive if HAVE_AS_LSE is not
	defined.  Otherwise, move the operands right after the glue? and
	comment out operands where the macros are used.
	* configure: Regenerated.
	* config.in: Regenerated.

--- libgcc/configure.ac.jj	2020-01-24 22:34:36.301641823 +0100
+++ libgcc/configure.ac	2020-04-14 13:48:48.771866654 +0200
@@ -599,6 +599,25 @@ i[[34567]]86-*-* | x86_64-*-*)
 esac])
 LIBGCC_CHECK_AS_AVX
 
+dnl Check if as supports LSE instructions.
+AC_DEFUN([LIBGCC_CHECK_AS_LSE], [
+case "${target}" in
+aarch64*-*-*)
+  AC_CACHE_CHECK([if the assembler supports LSE], libgcc_cv_as_lse, [
+    AC_TRY_COMPILE([],
+changequote(,)dnl
+			asm(".arch armv8-a+lse\n\tcas w0, w1, [x2]");
+changequote([,])dnl
+		       ,
+		   [libgcc_cv_as_lse=yes], [libgcc_cv_as_lse=no])
+  ])
+  if test x$libgcc_cv_as_lse = xyes; then
+    AC_DEFINE(HAVE_AS_LSE, 1, [Define to 1 if the assembler supports LSE.])
+  fi
+  ;;
+esac])
+LIBGCC_CHECK_AS_LSE
+
 dnl Check if as supports RTM instructions.
 AC_CACHE_CHECK(for init priority support, libgcc_cv_init_priority, [
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM(,
--- libgcc/config/aarch64/lse.S.jj	2020-01-12 11:54:38.610380262 +0100
+++ libgcc/config/aarch64/lse.S	2020-04-14 19:34:19.641346299 +0200
@@ -48,8 +48,14 @@ see the files COPYING3 and COPYING.RUNTI
  * separately to minimize code size.
  */
 
+#include "auto-target.h"
+
 /* Tell the assembler to accept LSE instructions.  */
+#ifdef HAVE_AS_LSE
 	.arch armv8-a+lse
+#else
+	.arch armv8-a
+#endif
 
 /* Declare the symbol gating the LSE implementations.  */
 	.hidden	__aarch64_have_lse_atomics
@@ -58,12 +64,19 @@ see the files COPYING3 and COPYING.RUNTI
 #if SIZE == 1
 # define S     b
 # define UXT   uxtb
+# define B     0x00000000
 #elif SIZE == 2
 # define S     h
 # define UXT   uxth
+# define B     0x40000000
 #elif SIZE == 4 || SIZE == 8 || SIZE == 16
 # define S
 # define UXT   mov
+# if SIZE == 4
+#  define B    0x80000000
+# elif SIZE == 8
+#  define B    0xc0000000
+# endif
 #else
 # error
 #endif
@@ -72,18 +85,26 @@ see the files COPYING3 and COPYING.RUNTI
 # define SUFF  _relax
 # define A
 # define L
+# define M     0x000000
+# define N     0x000000
 #elif MODEL == 2
 # define SUFF  _acq
 # define A     a
 # define L
+# define M     0x400000
+# define N     0x800000
 #elif MODEL == 3
 # define SUFF  _rel
 # define A
 # define L     l
+# define M     0x008000
+# define N     0x400000
 #elif MODEL == 4
 # define SUFF  _acq_rel
 # define A     a
 # define L     l
+# define M     0x408000
+# define N     0xc00000
 #else
 # error
 #endif
@@ -144,9 +165,13 @@ STARTFN	NAME(cas)
 	JUMP_IF_NOT_LSE	8f
 
 #if SIZE < 16
-#define CAS	glue4(cas, A, L, S)
+#ifdef HAVE_AS_LSE
+# define CAS	glue4(cas, A, L, S)	s(0), s(1), [x2]
+#else
+# define CAS	.inst 0x08a07c41 + B + M
+#endif
 
-	CAS		s(0), s(1), [x2]
+	CAS		/* s(0), s(1), [x2] */
 	ret
 
 8:	UXT		s(tmp0), s(0)
@@ -160,9 +185,13 @@ STARTFN	NAME(cas)
 #else
 #define LDXP	glue3(ld, A, xp)
 #define STXP	glue3(st, L, xp)
-#define CASP	glue3(casp, A, L)
+#ifdef HAVE_AS_LSE
+# define CASP	glue3(casp, A, L)	x0, x1, x2, x3, [x4]
+#else
+# define CASP	.inst 0x48207c82 + M
+#endif
 
-	CASP		x0, x1, x2, x3, [x4]
+	CASP		/* x0, x1, x2, x3, [x4] */
 	ret
 
 8:	mov		x(tmp0), x0
@@ -181,12 +210,16 @@ ENDFN	NAME(cas)
 #endif
 
 #ifdef L_swp
-#define SWP	glue4(swp, A, L, S)
+#ifdef HAVE_AS_LSE
+# define SWP	glue4(swp, A, L, S)	s(0), s(0), [x1]
+#else
+# define SWP	.inst 0x38208020 + B + N
+#endif
 
 STARTFN	NAME(swp)
 	JUMP_IF_NOT_LSE	8f
 
-	SWP		s(0), s(0), [x1]
+	SWP		/* s(0), s(0), [x1] */
 	ret
 
 8:	mov		s(tmp0), s(0)
@@ -204,24 +237,32 @@ ENDFN	NAME(swp)
 #ifdef L_ldadd
 #define LDNM	ldadd
 #define OP	add
+#define OPN	0x0000
 #elif defined(L_ldclr)
 #define LDNM	ldclr
 #define OP	bic
+#define OPN	0x1000
 #elif defined(L_ldeor)
 #define LDNM	ldeor
 #define OP	eor
+#define OPN	0x2000
 #elif defined(L_ldset)
 #define LDNM	ldset
 #define OP	orr
+#define OPN	0x3000
 #else
 #error
 #endif
-#define LDOP	glue4(LDNM, A, L, S)
+#ifdef HAVE_AS_LSE
+# define LDOP	glue4(LDNM, A, L, S)	s(0), s(0), [x1]
+#else
+# define LDOP	.inst 0x38200020 + OPN + B + N
+#endif
 
 STARTFN	NAME(LDNM)
 	JUMP_IF_NOT_LSE	8f
 
-	LDOP		s(0), s(0), [x1]
+	LDOP		/* s(0), s(0), [x1] */
 	ret
 
 8:	mov		s(tmp0), s(0)

--- libgcc/configure.jj	2020-02-12 23:17:24.866303077 +0100
+++ libgcc/configure	2020-04-14 13:48:52.627810203 +0200
@@ -5530,6 +5530,46 @@ $as_echo "#define HAVE_AS_AVX 1" >>confd
   ;;
 esac
 
+
+
+case "${target}" in
+aarch64*-*-*)
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking if the assembler supports LSE" >&5
+$as_echo_n "checking if the assembler supports LSE... " >&6; }
+if ${libgcc_cv_as_lse+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+
+    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main ()
+{
+			asm(".arch armv8-a+lse\n\tcas w0, w1, [x2]");
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  libgcc_cv_as_lse=yes
+else
+  libgcc_cv_as_lse=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_as_lse" >&5
+$as_echo "$libgcc_cv_as_lse" >&6; }
+  if test x$libgcc_cv_as_lse = xyes; then
+
+$as_echo "#define HAVE_AS_LSE 1" >>confdefs.h
+
+  fi
+  ;;
+esac
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for init priority support" >&5
 $as_echo_n "checking for init priority support... " >&6; }
 if ${libgcc_cv_init_priority+:} false; then :
--- libgcc/config.in.jj	2020-01-12 11:54:38.610380262 +0100
+++ libgcc/config.in	2020-04-14 13:53:19.037908554 +0200
@@ -10,6 +10,9 @@
    */
 #undef HAVE_AS_CFI_SECTIONS
 
+/* Define to 1 if the assembler supports LSE. */
+#undef HAVE_AS_LSE
+
 /* Define to 1 if the target assembler supports thread-local storage. */
 #undef HAVE_CC_TLS

Comments

Kyrylo Tkachov April 15, 2020, 8:27 a.m. UTC | #1 
Hi Jakub,

> -----Original Message-----
> From: Jakub Jelinek <jakub@redhat.com>
> Sent: 15 April 2020 08:28
> To: Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
> Richard Henderson <rth@twiddle.net>
> Cc: gcc-patches@gcc.gnu.org
> Subject: [PATCH] aarch64: Fix bootstrap with old binutils [PR93053]
> 
> Hi!
> 
> As reported in the PR, GCC 10 (and also 9.3.1 but not 9.3.0) fails to build
> when using older binutils which lack LSE support, because those instructions
> are used in libgcc.
> Thanks to Kyrylo's hint, the following patches (hopefully) allow it to build
> even with older binutils by using .inst directive if LSE support isn't
> available in the assembler.
> 
> There are two versions of the patch, one is using .macro to ignore the
> operands when emitting .inst, the other moves the operands for the
> HAVE_AS_LSE case into the macro.  My slight preference is the latter,
> because for maintanance it makes it clear that if one wants to use different
> operands, one needs to use a different macro and different .inst
> constant(s).

I'm not too worried about the difference really. Let's go with the second version for the sake of making a decision.

> 
> I've been testing this in distro scratch builds, so I don't really have
> there the old assembler, but bootstrapped/regtested 5 times, for each of the
> two patches once as is and once with s/cas/caszz/ in the configure{.ac,} to
> simulate missing support of LSE (and verified that as is the build logs
> contain checking...supports LSE yes and with the tweaked ...supports LSE no)
> and once vanilla trunk, and aarch64-linux-gnu-objdump -dr libgcc.a is
> identical from all 5 builds.
> 
> Ok for trunk (which version), or do we want to instead document a newer
> binutils requirement?  Yet another option is to go with the patch for a
> while and later bump the requirement and revert the patch.

I'd definitely like the patch to go in as we've gotten requests to backport the OOL feature to GCC 8 as well...
Eventually non-LSE assemblers will be unsuitable for various reasons and I expect we'll enforce a minimum required binutils version, at which point I expect we can revert this patch. Is there an effective way of making sure we don't forget to do that? (A bugzilla entry?)

Thanks for working on this,
Kyrill

> 
> 	Jakub
Jakub Jelinek April 15, 2020, 8:40 a.m. UTC | #2 
On Wed, Apr 15, 2020 at 08:27:43AM +0000, Kyrylo Tkachov wrote:
> > I've been testing this in distro scratch builds, so I don't really have
> > there the old assembler, but bootstrapped/regtested 5 times, for each of the
> > two patches once as is and once with s/cas/caszz/ in the configure{.ac,} to
> > simulate missing support of LSE (and verified that as is the build logs
> > contain checking...supports LSE yes and with the tweaked ...supports LSE no)
> > and once vanilla trunk, and aarch64-linux-gnu-objdump -dr libgcc.a is
> > identical from all 5 builds.

FYI, I've now also tested on gcc115 in GCCFarm which has the 2013-ish gas
and without the patch it indeed fails to build and with the patch it builds
fine.

> Eventually non-LSE assemblers will be unsuitable for various reasons and I
> expect we'll enforce a minimum required binutils version, at which point I
> expect we can revert this patch.  Is there an effective way of making sure
> we don't forget to do that?  (A bugzilla entry?)

We can file a bugzilla bug, but whether anybody will remember it when bumping the
binutils requirement, no idea.

	Jakub
diff mbox series

Patch

--- libgcc/configure.ac.jj	2020-01-24 22:34:36.301641823 +0100
+++ libgcc/configure.ac	2020-04-14 13:48:48.771866654 +0200
@@ -599,6 +599,25 @@  i[[34567]]86-*-* | x86_64-*-*)
 esac])
 LIBGCC_CHECK_AS_AVX
 
+dnl Check if as supports LSE instructions.
+AC_DEFUN([LIBGCC_CHECK_AS_LSE], [
+case "${target}" in
+aarch64*-*-*)
+  AC_CACHE_CHECK([if the assembler supports LSE], libgcc_cv_as_lse, [
+    AC_TRY_COMPILE([],
+changequote(,)dnl
+			asm(".arch armv8-a+lse\n\tcas w0, w1, [x2]");
+changequote([,])dnl
+		       ,
+		   [libgcc_cv_as_lse=yes], [libgcc_cv_as_lse=no])
+  ])
+  if test x$libgcc_cv_as_lse = xyes; then
+    AC_DEFINE(HAVE_AS_LSE, 1, [Define to 1 if the assembler supports LSE.])
+  fi
+  ;;
+esac])
+LIBGCC_CHECK_AS_LSE
+
 dnl Check if as supports RTM instructions.
 AC_CACHE_CHECK(for init priority support, libgcc_cv_init_priority, [
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM(,
--- libgcc/config/aarch64/lse.S.jj	2020-01-12 11:54:38.610380262 +0100
+++ libgcc/config/aarch64/lse.S	2020-04-14 13:44:57.403253956 +0200
@@ -48,8 +48,14 @@  see the files COPYING3 and COPYING.RUNTI
  * separately to minimize code size.
  */
 
+#include "auto-target.h"
+
 /* Tell the assembler to accept LSE instructions.  */
+#ifdef HAVE_AS_LSE
 	.arch armv8-a+lse
+#else
+	.arch armv8-a
+#endif
 
 /* Declare the symbol gating the LSE implementations.  */
 	.hidden	__aarch64_have_lse_atomics
@@ -58,12 +64,19 @@  see the files COPYING3 and COPYING.RUNTI
 #if SIZE == 1
 # define S     b
 # define UXT   uxtb
+# define B     0x00000000
 #elif SIZE == 2
 # define S     h
 # define UXT   uxth
+# define B     0x40000000
 #elif SIZE == 4 || SIZE == 8 || SIZE == 16
 # define S
 # define UXT   mov
+# if SIZE == 4
+#  define B    0x80000000
+# elif SIZE == 8
+#  define B    0xc0000000
+# endif
 #else
 # error
 #endif
@@ -72,18 +85,26 @@  see the files COPYING3 and COPYING.RUNTI
 # define SUFF  _relax
 # define A
 # define L
+# define M     0x000000
+# define N     0x000000
 #elif MODEL == 2
 # define SUFF  _acq
 # define A     a
 # define L
+# define M     0x400000
+# define N     0x800000
 #elif MODEL == 3
 # define SUFF  _rel
 # define A
 # define L     l
+# define M     0x008000
+# define N     0x400000
 #elif MODEL == 4
 # define SUFF  _acq_rel
 # define A     a
 # define L     l
+# define M     0x408000
+# define N     0xc00000
 #else
 # error
 #endif
@@ -138,13 +159,23 @@  see the files COPYING3 and COPYING.RUNTI
 	cbz	w(tmp0), \label
 .endm
 
+#ifndef HAVE_AS_LSE
+/* Throw away all arguments.  */
+.macro	COMMENT args:vararg
+.endm
+#endif
+
 #ifdef L_cas
 
 STARTFN	NAME(cas)
 	JUMP_IF_NOT_LSE	8f
 
 #if SIZE < 16
-#define CAS	glue4(cas, A, L, S)
+#ifdef HAVE_AS_LSE
+# define CAS	glue4(cas, A, L, S)
+#else
+# define CAS	.inst 0x08a07c41 + B + M; COMMENT
+#endif
 
 	CAS		s(0), s(1), [x2]
 	ret
@@ -160,7 +191,11 @@  STARTFN	NAME(cas)
 #else
 #define LDXP	glue3(ld, A, xp)
 #define STXP	glue3(st, L, xp)
-#define CASP	glue3(casp, A, L)
+#ifdef HAVE_AS_LSE
+# define CASP	glue3(casp, A, L)
+#else
+# define CASP	.inst 0x48207c82 + M; COMMENT
+#endif
 
 	CASP		x0, x1, x2, x3, [x4]
 	ret
@@ -181,7 +216,11 @@  ENDFN	NAME(cas)
 #endif
 
 #ifdef L_swp
-#define SWP	glue4(swp, A, L, S)
+#ifdef HAVE_AS_LSE
+# define SWP	glue4(swp, A, L, S)
+#else
+# define SWP	.inst 0x38208020 + B + N; COMMENT
+#endif
 
 STARTFN	NAME(swp)
 	JUMP_IF_NOT_LSE	8f
@@ -204,19 +243,27 @@  ENDFN	NAME(swp)
 #ifdef L_ldadd
 #define LDNM	ldadd
 #define OP	add
+#define OPN	0x0000
 #elif defined(L_ldclr)
 #define LDNM	ldclr
 #define OP	bic
+#define OPN	0x1000
 #elif defined(L_ldeor)
 #define LDNM	ldeor
 #define OP	eor
+#define OPN	0x2000
 #elif defined(L_ldset)
 #define LDNM	ldset
 #define OP	orr
+#define OPN	0x3000
 #else
 #error
 #endif
-#define LDOP	glue4(LDNM, A, L, S)
+#ifdef HAVE_AS_LSE
+# define LDOP	glue4(LDNM, A, L, S)
+#else
+# define LDOP	.inst 0x38200020 + OPN + B + N; COMMENT
+#endif
 
 STARTFN	NAME(LDNM)
 	JUMP_IF_NOT_LSE	8f
--- libgcc/configure.jj	2020-02-12 23:17:24.866303077 +0100
+++ libgcc/configure	2020-04-14 13:48:52.627810203 +0200
@@ -5530,6 +5530,46 @@  $as_echo "#define HAVE_AS_AVX 1" >>confd
   ;;
 esac
 
+
+
+case "${target}" in
+aarch64*-*-*)
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking if the assembler supports LSE" >&5
+$as_echo_n "checking if the assembler supports LSE... " >&6; }
+if ${libgcc_cv_as_lse+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+
+    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main ()
+{
+			asm(".arch armv8-a+lse\n\tcas w0, w1, [x2]");
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  libgcc_cv_as_lse=yes
+else
+  libgcc_cv_as_lse=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_as_lse" >&5
+$as_echo "$libgcc_cv_as_lse" >&6; }
+  if test x$libgcc_cv_as_lse = xyes; then
+
+$as_echo "#define HAVE_AS_LSE 1" >>confdefs.h
+
+  fi
+  ;;
+esac
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for init priority support" >&5
 $as_echo_n "checking for init priority support... " >&6; }
 if ${libgcc_cv_init_priority+:} false; then :
--- libgcc/config.in.jj	2020-01-12 11:54:38.610380262 +0100
+++ libgcc/config.in	2020-04-14 13:53:19.037908554 +0200
@@ -10,6 +10,9 @@ 
    */
 #undef HAVE_AS_CFI_SECTIONS
 
+/* Define to 1 if the assembler supports LSE. */
+#undef HAVE_AS_LSE
+
 /* Define to 1 if the target assembler supports thread-local storage. */
 #undef HAVE_CC_TLS