public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
@ 2024-06-12 23:20 Patrick O'Neill
  2024-06-12 23:39 ` Palmer Dabbelt
  2024-06-13 20:02 ` Jeff Law
  0 siblings, 2 replies; 8+ messages in thread
From: Patrick O'Neill @ 2024-06-12 23:20 UTC (permalink / raw)
  To: gcc-patches
  Cc: kito.cheng, jeffreyalaw, palmer, gnu-toolchain, pan2.li, schwab,
	andrea, Patrick O'Neill

Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
check to prevent emitting Zaamo/Zalrsc in the arch string when the
assember does not support it.

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc
	  (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
	  supported by the assembler.
	* config.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Add zaamo/zalrsc assmeber check.

Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
---
Tested using newlib rv64gc with binutils tip-of-tree and 2.42.

This results in calls being emitted when compiling for _zaamo_zalrsc
when the assember does not support these extensions.

> cat amo.c
void foo (int* bar, int* baz)
{
  __atomic_add_fetch(bar, baz, __ATOMIC_RELAXED);
}
> gcc -march=rv64id_zaamo_zalrsc -O3 amo.c
results in:
foo:
        sext.w  a1,a1
        li      a2,0
        tail    __atomic_fetch_add_4

As a result there are some testsuite failures on zalrsc specific
testcases and when using an old version of binutils on non-a targets.
Not a cause for concern imo but worth calling out.
Also testcases that check for the default isa string will fail with
the old binutils since zaamo/zalrsc aren't emitted anymore.
---
 gcc/common/config/riscv/riscv-common.cc | 11 +++++++
 gcc/config.in                           |  6 ++++
 gcc/configure                           | 41 ++++++++++++++++++++++---
 gcc/configure.ac                        |  5 +++
 4 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 78dfd6b1470..1dc1d9904c7 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -916,6 +916,7 @@ riscv_subset_list::to_string (bool version_p) const
   riscv_subset_t *subset;

   bool skip_zifencei = false;
+  bool skip_zaamo_zalrsc = false;
   bool skip_zicsr = false;
   bool i2p0 = false;

@@ -943,6 +944,10 @@ riscv_subset_list::to_string (bool version_p) const
      a mistake in that binutils 2.35 supports zicsr but not zifencei.  */
   skip_zifencei = true;
 #endif
+#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
+  /* Skip since binutils 2.42 and earlier don't recognize zaamo/zalrsc.  */
+  skip_zaamo_zalrsc = true;
+#endif

   for (subset = m_head; subset != NULL; subset = subset->next)
     {
@@ -954,6 +959,12 @@ riscv_subset_list::to_string (bool version_p) const
 	  subset->name == "zicsr")
 	continue;

+      if (skip_zaamo_zalrsc && subset->name == "zaamo")
+	continue;
+
+      if (skip_zaamo_zalrsc && subset->name == "zalrsc")
+	continue;
+
       /* For !version_p, we only separate extension with underline for
 	 multi-letter extension.  */
       if (!first &&
diff --git a/gcc/config.in b/gcc/config.in
index e41b6dc97cd..acab3c0f126 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -629,6 +629,12 @@
 #endif


+/* Define if the assembler understands -march=rv*_zaamo_zalrsc. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_AS_MARCH_ZAAMO_ZALRSC
+#endif
+
+
 /* Define if the assembler understands -march=rv*_zifencei. */
 #ifndef USED_FOR_TARGET
 #undef HAVE_AS_MARCH_ZIFENCEI
diff --git a/gcc/configure b/gcc/configure
index aaf5899cc03..09b794c1225 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -6228,7 +6228,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -6274,7 +6274,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -6298,7 +6298,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -6343,7 +6343,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -6367,7 +6367,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -30820,6 +30820,37 @@ if test $gcc_cv_as_riscv_march_zifencei = yes; then

 $as_echo "#define HAVE_AS_MARCH_ZIFENCEI 1" >>confdefs.h

+fi
+
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for -march=rv32i_zaamo_zalrsc support" >&5
+$as_echo_n "checking assembler for -march=rv32i_zaamo_zalrsc support... " >&6; }
+if ${gcc_cv_as_riscv_march_zaamo_zalrsc+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  gcc_cv_as_riscv_march_zaamo_zalrsc=no
+  if test x$gcc_cv_as != x; then
+    $as_echo '' > conftest.s
+    if { ac_try='$gcc_cv_as $gcc_cv_as_flags -march=rv32i_zaamo_zalrsc -o conftest.o conftest.s >&5'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; }
+    then
+	gcc_cv_as_riscv_march_zaamo_zalrsc=yes
+    else
+      echo "configure: failed program was" >&5
+      cat conftest.s >&5
+    fi
+    rm -f conftest.o conftest.s
+  fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_riscv_march_zaamo_zalrsc" >&5
+$as_echo "$gcc_cv_as_riscv_march_zaamo_zalrsc" >&6; }
+if test $gcc_cv_as_riscv_march_zaamo_zalrsc = yes; then
+
+$as_echo "#define HAVE_AS_MARCH_ZAAMO_ZALRSC 1" >>confdefs.h
+
 fi

     ;;
diff --git a/gcc/configure.ac b/gcc/configure.ac
index f8d67efeb98..c54748cd9aa 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -5452,6 +5452,11 @@ configured with --enable-newlib-nano-formatted-io.])
       [-march=rv32i_zifencei2p0],,,
       [AC_DEFINE(HAVE_AS_MARCH_ZIFENCEI, 1,
 		 [Define if the assembler understands -march=rv*_zifencei.])])
+    gcc_GAS_CHECK_FEATURE([-march=rv32i_zaamo_zalrsc support],
+      gcc_cv_as_riscv_march_zaamo_zalrsc,
+      [-march=rv32i_zaamo_zalrsc],,,
+      [AC_DEFINE(HAVE_AS_MARCH_ZAAMO_ZALRSC, 1,
+		 [Define if the assembler understands -march=rv*_zaamo_zalrsc.])])
     ;;
     loongarch*-*-*)
     gcc_GAS_CHECK_FEATURE([.dtprelword support],
--
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
  2024-06-12 23:20 [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support Patrick O'Neill
@ 2024-06-12 23:39 ` Palmer Dabbelt
  2024-06-12 23:49   ` Sam James
  2024-06-13 20:02 ` Jeff Law
  1 sibling, 1 reply; 8+ messages in thread
From: Palmer Dabbelt @ 2024-06-12 23:39 UTC (permalink / raw)
  To: Patrick O'Neill
  Cc: gcc-patches, Kito Cheng, jeffreyalaw, gnu-toolchain, pan2.li,
	schwab, Andrea Parri, Patrick O'Neill

On Wed, 12 Jun 2024 16:20:26 PDT (-0700), Patrick O'Neill wrote:
> Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
> check to prevent emitting Zaamo/Zalrsc in the arch string when the
> assember does not support it.

Should we just rewrite these to A when binutils doesn't support the 
subsets?  That'd avoid a forced binutils bump, but really user should be 
upgrading anyway...  Either way

Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V

though I'm not suer if the configure churn is sane, it looks like a 
version mismatch of some sort.  Hopefully someone who knows those bits 
better can chime in?

> gcc/ChangeLog:
>
> 	* common/config/riscv/riscv-common.cc
> 	  (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
> 	  supported by the assembler.
> 	* config.in: Regenerate.
> 	* configure: Regenerate.
> 	* configure.ac: Add zaamo/zalrsc assmeber check.
>
> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
> ---
> Tested using newlib rv64gc with binutils tip-of-tree and 2.42.
>
> This results in calls being emitted when compiling for _zaamo_zalrsc
> when the assember does not support these extensions.
>
>> cat amo.c
> void foo (int* bar, int* baz)
> {
>   __atomic_add_fetch(bar, baz, __ATOMIC_RELAXED);
> }
>> gcc -march=rv64id_zaamo_zalrsc -O3 amo.c
> results in:
> foo:
>         sext.w  a1,a1
>         li      a2,0
>         tail    __atomic_fetch_add_4
>
> As a result there are some testsuite failures on zalrsc specific
> testcases and when using an old version of binutils on non-a targets.
> Not a cause for concern imo but worth calling out.
> Also testcases that check for the default isa string will fail with
> the old binutils since zaamo/zalrsc aren't emitted anymore.
> ---
>  gcc/common/config/riscv/riscv-common.cc | 11 +++++++
>  gcc/config.in                           |  6 ++++
>  gcc/configure                           | 41 ++++++++++++++++++++++---
>  gcc/configure.ac                        |  5 +++
>  4 files changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index 78dfd6b1470..1dc1d9904c7 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -916,6 +916,7 @@ riscv_subset_list::to_string (bool version_p) const
>    riscv_subset_t *subset;
>
>    bool skip_zifencei = false;
> +  bool skip_zaamo_zalrsc = false;
>    bool skip_zicsr = false;
>    bool i2p0 = false;
>
> @@ -943,6 +944,10 @@ riscv_subset_list::to_string (bool version_p) const
>       a mistake in that binutils 2.35 supports zicsr but not zifencei.  */
>    skip_zifencei = true;
>  #endif
> +#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
> +  /* Skip since binutils 2.42 and earlier don't recognize zaamo/zalrsc.  */
> +  skip_zaamo_zalrsc = true;
> +#endif
>
>    for (subset = m_head; subset != NULL; subset = subset->next)
>      {
> @@ -954,6 +959,12 @@ riscv_subset_list::to_string (bool version_p) const
>  	  subset->name == "zicsr")
>  	continue;
>
> +      if (skip_zaamo_zalrsc && subset->name == "zaamo")
> +	continue;
> +
> +      if (skip_zaamo_zalrsc && subset->name == "zalrsc")
> +	continue;
> +
>        /* For !version_p, we only separate extension with underline for
>  	 multi-letter extension.  */
>        if (!first &&
> diff --git a/gcc/config.in b/gcc/config.in
> index e41b6dc97cd..acab3c0f126 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -629,6 +629,12 @@
>  #endif
>
>
> +/* Define if the assembler understands -march=rv*_zaamo_zalrsc. */
> +#ifndef USED_FOR_TARGET
> +#undef HAVE_AS_MARCH_ZAAMO_ZALRSC
> +#endif
> +
> +
>  /* Define if the assembler understands -march=rv*_zifencei. */
>  #ifndef USED_FOR_TARGET
>  #undef HAVE_AS_MARCH_ZIFENCEI
> diff --git a/gcc/configure b/gcc/configure
> index aaf5899cc03..09b794c1225 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -6228,7 +6228,7 @@ else
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -6274,7 +6274,7 @@ else
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -6298,7 +6298,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -6343,7 +6343,7 @@ else
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -6367,7 +6367,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -30820,6 +30820,37 @@ if test $gcc_cv_as_riscv_march_zifencei = yes; then
>
>  $as_echo "#define HAVE_AS_MARCH_ZIFENCEI 1" >>confdefs.h
>
> +fi
> +
> +    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for -march=rv32i_zaamo_zalrsc support" >&5
> +$as_echo_n "checking assembler for -march=rv32i_zaamo_zalrsc support... " >&6; }
> +if ${gcc_cv_as_riscv_march_zaamo_zalrsc+:} false; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +  gcc_cv_as_riscv_march_zaamo_zalrsc=no
> +  if test x$gcc_cv_as != x; then
> +    $as_echo '' > conftest.s
> +    if { ac_try='$gcc_cv_as $gcc_cv_as_flags -march=rv32i_zaamo_zalrsc -o conftest.o conftest.s >&5'
> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> +  (eval $ac_try) 2>&5
> +  ac_status=$?
> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> +  test $ac_status = 0; }; }
> +    then
> +	gcc_cv_as_riscv_march_zaamo_zalrsc=yes
> +    else
> +      echo "configure: failed program was" >&5
> +      cat conftest.s >&5
> +    fi
> +    rm -f conftest.o conftest.s
> +  fi
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_riscv_march_zaamo_zalrsc" >&5
> +$as_echo "$gcc_cv_as_riscv_march_zaamo_zalrsc" >&6; }
> +if test $gcc_cv_as_riscv_march_zaamo_zalrsc = yes; then
> +
> +$as_echo "#define HAVE_AS_MARCH_ZAAMO_ZALRSC 1" >>confdefs.h
> +
>  fi
>
>      ;;
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index f8d67efeb98..c54748cd9aa 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -5452,6 +5452,11 @@ configured with --enable-newlib-nano-formatted-io.])
>        [-march=rv32i_zifencei2p0],,,
>        [AC_DEFINE(HAVE_AS_MARCH_ZIFENCEI, 1,
>  		 [Define if the assembler understands -march=rv*_zifencei.])])
> +    gcc_GAS_CHECK_FEATURE([-march=rv32i_zaamo_zalrsc support],
> +      gcc_cv_as_riscv_march_zaamo_zalrsc,
> +      [-march=rv32i_zaamo_zalrsc],,,
> +      [AC_DEFINE(HAVE_AS_MARCH_ZAAMO_ZALRSC, 1,
> +		 [Define if the assembler understands -march=rv*_zaamo_zalrsc.])])
>      ;;
>      loongarch*-*-*)
>      gcc_GAS_CHECK_FEATURE([.dtprelword support],

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
  2024-06-12 23:39 ` Palmer Dabbelt
@ 2024-06-12 23:49   ` Sam James
  2024-06-12 23:56     ` Patrick O'Neill
  0 siblings, 1 reply; 8+ messages in thread
From: Sam James @ 2024-06-12 23:49 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Patrick O'Neill, gcc-patches, Kito Cheng, jeffreyalaw,
	gnu-toolchain, pan2.li, schwab, Andrea Parri

[-- Attachment #1: Type: text/plain, Size: 9138 bytes --]

Palmer Dabbelt <palmer@dabbelt.com> writes:

> On Wed, 12 Jun 2024 16:20:26 PDT (-0700), Patrick O'Neill wrote:
>> Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
>> check to prevent emitting Zaamo/Zalrsc in the arch string when the
>> assember does not support it.
>
> Should we just rewrite these to A when binutils doesn't support the
> subsets?  That'd avoid a forced binutils bump, but really user should
> be upgrading anyway...  Either way
>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>
> though I'm not suer if the configure churn is sane, it looks like a
> version mismatch of some sort.  Hopefully someone who knows those bits
> better can chime in?

Your instinct is right!

>
>> gcc/ChangeLog:
>>
>> 	* common/config/riscv/riscv-common.cc
>> 	  (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
>> 	  supported by the assembler.
>> 	* config.in: Regenerate.
>> 	* configure: Regenerate.
>> 	* configure.ac: Add zaamo/zalrsc assmeber check.
>>
>> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
>> ---
>> Tested using newlib rv64gc with binutils tip-of-tree and 2.42.
>>
>> This results in calls being emitted when compiling for _zaamo_zalrsc
>> when the assember does not support these extensions.
>>
>>> cat amo.c
>> void foo (int* bar, int* baz)
>> {
>>   __atomic_add_fetch(bar, baz, __ATOMIC_RELAXED);
>> }
>>> gcc -march=rv64id_zaamo_zalrsc -O3 amo.c
>> results in:
>> foo:
>>         sext.w  a1,a1
>>         li      a2,0
>>         tail    __atomic_fetch_add_4
>>
>> As a result there are some testsuite failures on zalrsc specific
>> testcases and when using an old version of binutils on non-a targets.
>> Not a cause for concern imo but worth calling out.
>> Also testcases that check for the default isa string will fail with
>> the old binutils since zaamo/zalrsc aren't emitted anymore.
>> ---
>>  gcc/common/config/riscv/riscv-common.cc | 11 +++++++
>>  gcc/config.in                           |  6 ++++
>>  gcc/configure                           | 41 ++++++++++++++++++++++---
>>  gcc/configure.ac                        |  5 +++
>>  4 files changed, 58 insertions(+), 5 deletions(-)
>>
>> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
>> index 78dfd6b1470..1dc1d9904c7 100644
>> --- a/gcc/common/config/riscv/riscv-common.cc
>> +++ b/gcc/common/config/riscv/riscv-common.cc
>> @@ -916,6 +916,7 @@ riscv_subset_list::to_string (bool version_p) const
>>    riscv_subset_t *subset;
>>
>>    bool skip_zifencei = false;
>> +  bool skip_zaamo_zalrsc = false;
>>    bool skip_zicsr = false;
>>    bool i2p0 = false;
>>
>> @@ -943,6 +944,10 @@ riscv_subset_list::to_string (bool version_p) const
>>       a mistake in that binutils 2.35 supports zicsr but not zifencei.  */
>>    skip_zifencei = true;
>>  #endif
>> +#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
>> +  /* Skip since binutils 2.42 and earlier don't recognize zaamo/zalrsc.  */
>> +  skip_zaamo_zalrsc = true;
>> +#endif
>>
>>    for (subset = m_head; subset != NULL; subset = subset->next)
>>      {
>> @@ -954,6 +959,12 @@ riscv_subset_list::to_string (bool version_p) const
>>  	  subset->name == "zicsr")
>>  	continue;
>>
>> +      if (skip_zaamo_zalrsc && subset->name == "zaamo")
>> +	continue;
>> +
>> +      if (skip_zaamo_zalrsc && subset->name == "zalrsc")
>> +	continue;
>> +
>>        /* For !version_p, we only separate extension with underline for
>>  	 multi-letter extension.  */
>>        if (!first &&
>> diff --git a/gcc/config.in b/gcc/config.in
>> index e41b6dc97cd..acab3c0f126 100644
>> --- a/gcc/config.in
>> +++ b/gcc/config.in
>> @@ -629,6 +629,12 @@
>>  #endif
>>
>>
>> +/* Define if the assembler understands -march=rv*_zaamo_zalrsc. */
>> +#ifndef USED_FOR_TARGET
>> +#undef HAVE_AS_MARCH_ZAAMO_ZALRSC
>> +#endif
>> +
>> +
>>  /* Define if the assembler understands -march=rv*_zifencei. */
>>  #ifndef USED_FOR_TARGET
>>  #undef HAVE_AS_MARCH_ZIFENCEI
>> diff --git a/gcc/configure b/gcc/configure
>> index aaf5899cc03..09b794c1225 100755
>> --- a/gcc/configure
>> +++ b/gcc/configure
>> @@ -6228,7 +6228,7 @@ else
>>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>      since some C++ compilers masquerading as C compilers
>>      incorrectly reject 9223372036854775807.  */
>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721

I think you may be using patched autoconf which fixes
http://bugs.debian.org/742780.

The fix landed in 2.70: https://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commit;h=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e.

Please drop those hunks.


>>  		       && LARGE_OFF_T % 2147483647 == 1)
>>  		      ? 1 : -1];
>> @@ -6274,7 +6274,7 @@ else
>>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>      since some C++ compilers masquerading as C compilers
>>      incorrectly reject 9223372036854775807.  */
>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>  		       && LARGE_OFF_T % 2147483647 == 1)
>>  		      ? 1 : -1];
>> @@ -6298,7 +6298,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>      since some C++ compilers masquerading as C compilers
>>      incorrectly reject 9223372036854775807.  */
>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>  		       && LARGE_OFF_T % 2147483647 == 1)
>>  		      ? 1 : -1];
>> @@ -6343,7 +6343,7 @@ else
>>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>      since some C++ compilers masquerading as C compilers
>>      incorrectly reject 9223372036854775807.  */
>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>  		       && LARGE_OFF_T % 2147483647 == 1)
>>  		      ? 1 : -1];
>> @@ -6367,7 +6367,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>      since some C++ compilers masquerading as C compilers
>>      incorrectly reject 9223372036854775807.  */
>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>  		       && LARGE_OFF_T % 2147483647 == 1)
>>  		      ? 1 : -1];
>> @@ -30820,6 +30820,37 @@ if test $gcc_cv_as_riscv_march_zifencei = yes; then
>>
>>  $as_echo "#define HAVE_AS_MARCH_ZIFENCEI 1" >>confdefs.h
>>
>> +fi
>> +
>> +    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for -march=rv32i_zaamo_zalrsc support" >&5
>> +$as_echo_n "checking assembler for -march=rv32i_zaamo_zalrsc support... " >&6; }
>> +if ${gcc_cv_as_riscv_march_zaamo_zalrsc+:} false; then :
>> +  $as_echo_n "(cached) " >&6
>> +else
>> +  gcc_cv_as_riscv_march_zaamo_zalrsc=no
>> +  if test x$gcc_cv_as != x; then
>> +    $as_echo '' > conftest.s
>> +    if { ac_try='$gcc_cv_as $gcc_cv_as_flags -march=rv32i_zaamo_zalrsc -o conftest.o conftest.s >&5'
>> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>> +  (eval $ac_try) 2>&5
>> +  ac_status=$?
>> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>> +  test $ac_status = 0; }; }
>> +    then
>> +	gcc_cv_as_riscv_march_zaamo_zalrsc=yes
>> +    else
>> +      echo "configure: failed program was" >&5
>> +      cat conftest.s >&5
>> +    fi
>> +    rm -f conftest.o conftest.s
>> +  fi
>> +fi
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_riscv_march_zaamo_zalrsc" >&5
>> +$as_echo "$gcc_cv_as_riscv_march_zaamo_zalrsc" >&6; }
>> +if test $gcc_cv_as_riscv_march_zaamo_zalrsc = yes; then
>> +
>> +$as_echo "#define HAVE_AS_MARCH_ZAAMO_ZALRSC 1" >>confdefs.h
>> +
>>  fi
>>
>>      ;;
>> diff --git a/gcc/configure.ac b/gcc/configure.ac
>> index f8d67efeb98..c54748cd9aa 100644
>> --- a/gcc/configure.ac
>> +++ b/gcc/configure.ac
>> @@ -5452,6 +5452,11 @@ configured with --enable-newlib-nano-formatted-io.])
>>        [-march=rv32i_zifencei2p0],,,
>>        [AC_DEFINE(HAVE_AS_MARCH_ZIFENCEI, 1,
>>  		 [Define if the assembler understands -march=rv*_zifencei.])])
>> +    gcc_GAS_CHECK_FEATURE([-march=rv32i_zaamo_zalrsc support],
>> +      gcc_cv_as_riscv_march_zaamo_zalrsc,
>> +      [-march=rv32i_zaamo_zalrsc],,,
>> +      [AC_DEFINE(HAVE_AS_MARCH_ZAAMO_ZALRSC, 1,
>> +		 [Define if the assembler understands -march=rv*_zaamo_zalrsc.])])
>>      ;;
>>      loongarch*-*-*)
>>      gcc_GAS_CHECK_FEATURE([.dtprelword support],

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
  2024-06-12 23:49   ` Sam James
@ 2024-06-12 23:56     ` Patrick O'Neill
  2024-06-13  0:01       ` Palmer Dabbelt
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick O'Neill @ 2024-06-12 23:56 UTC (permalink / raw)
  To: Sam James, Palmer Dabbelt
  Cc: gcc-patches, Kito Cheng, jeffreyalaw, gnu-toolchain, pan2.li,
	schwab, Andrea Parri


On 6/12/24 16:49, Sam James wrote:
> Palmer Dabbelt <palmer@dabbelt.com> writes:
>
>> On Wed, 12 Jun 2024 16:20:26 PDT (-0700), Patrick O'Neill wrote:
>>> Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
>>> check to prevent emitting Zaamo/Zalrsc in the arch string when the
>>> assember does not support it.
>> Should we just rewrite these to A when binutils doesn't support the
>> subsets?  That'd avoid a forced binutils bump, but really user should
>> be upgrading anyway...  Either way
>>
>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>>
>> though I'm not suer if the configure churn is sane, it looks like a
>> version mismatch of some sort.  Hopefully someone who knows those bits
>> better can chime in?
> Your instinct is right!
>
>>> gcc/ChangeLog:
>>>
>>> 	* common/config/riscv/riscv-common.cc
>>> 	  (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
>>> 	  supported by the assembler.
>>> 	* config.in: Regenerate.
>>> 	* configure: Regenerate.
>>> 	* configure.ac: Add zaamo/zalrsc assmeber check.
>>>
>>> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
>>> ---
>>> Tested using newlib rv64gc with binutils tip-of-tree and 2.42.
>>>
>>> This results in calls being emitted when compiling for _zaamo_zalrsc
>>> when the assember does not support these extensions.
>>>
>>>> cat amo.c
>>> void foo (int* bar, int* baz)
>>> {
>>>    __atomic_add_fetch(bar, baz, __ATOMIC_RELAXED);
>>> }
>>>> gcc -march=rv64id_zaamo_zalrsc -O3 amo.c
>>> results in:
>>> foo:
>>>          sext.w  a1,a1
>>>          li      a2,0
>>>          tail    __atomic_fetch_add_4
>>>
>>> As a result there are some testsuite failures on zalrsc specific
>>> testcases and when using an old version of binutils on non-a targets.
>>> Not a cause for concern imo but worth calling out.
>>> Also testcases that check for the default isa string will fail with
>>> the old binutils since zaamo/zalrsc aren't emitted anymore.
>>> ---
>>>   gcc/common/config/riscv/riscv-common.cc | 11 +++++++
>>>   gcc/config.in                           |  6 ++++
>>>   gcc/configure                           | 41 ++++++++++++++++++++++---
>>>   gcc/configure.ac                        |  5 +++
>>>   4 files changed, 58 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
>>> index 78dfd6b1470..1dc1d9904c7 100644
>>> --- a/gcc/common/config/riscv/riscv-common.cc
>>> +++ b/gcc/common/config/riscv/riscv-common.cc
>>> @@ -916,6 +916,7 @@ riscv_subset_list::to_string (bool version_p) const
>>>     riscv_subset_t *subset;
>>>
>>>     bool skip_zifencei = false;
>>> +  bool skip_zaamo_zalrsc = false;
>>>     bool skip_zicsr = false;
>>>     bool i2p0 = false;
>>>
>>> @@ -943,6 +944,10 @@ riscv_subset_list::to_string (bool version_p) const
>>>        a mistake in that binutils 2.35 supports zicsr but not zifencei.  */
>>>     skip_zifencei = true;
>>>   #endif
>>> +#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
>>> +  /* Skip since binutils 2.42 and earlier don't recognize zaamo/zalrsc.  */
>>> +  skip_zaamo_zalrsc = true;
>>> +#endif
>>>
>>>     for (subset = m_head; subset != NULL; subset = subset->next)
>>>       {
>>> @@ -954,6 +959,12 @@ riscv_subset_list::to_string (bool version_p) const
>>>   	  subset->name == "zicsr")
>>>   	continue;
>>>
>>> +      if (skip_zaamo_zalrsc && subset->name == "zaamo")
>>> +	continue;
>>> +
>>> +      if (skip_zaamo_zalrsc && subset->name == "zalrsc")
>>> +	continue;
>>> +
>>>         /* For !version_p, we only separate extension with underline for
>>>   	 multi-letter extension.  */
>>>         if (!first &&
>>> diff --git a/gcc/config.in b/gcc/config.in
>>> index e41b6dc97cd..acab3c0f126 100644
>>> --- a/gcc/config.in
>>> +++ b/gcc/config.in
>>> @@ -629,6 +629,12 @@
>>>   #endif
>>>
>>>
>>> +/* Define if the assembler understands -march=rv*_zaamo_zalrsc. */
>>> +#ifndef USED_FOR_TARGET
>>> +#undef HAVE_AS_MARCH_ZAAMO_ZALRSC
>>> +#endif
>>> +
>>> +
>>>   /* Define if the assembler understands -march=rv*_zifencei. */
>>>   #ifndef USED_FOR_TARGET
>>>   #undef HAVE_AS_MARCH_ZIFENCEI
>>> diff --git a/gcc/configure b/gcc/configure
>>> index aaf5899cc03..09b794c1225 100755
>>> --- a/gcc/configure
>>> +++ b/gcc/configure
>>> @@ -6228,7 +6228,7 @@ else
>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>       since some C++ compilers masquerading as C compilers
>>>       incorrectly reject 9223372036854775807.  */
>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
> I think you may be using patched autoconf which fixes
> http://bugs.debian.org/742780.
>
> The fix landed in 2.70: https://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commit;h=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e.
>
> Please drop those hunks.

I thought I could get away with using the apt autoconf2.69 package 
directly ;)

Thanks. I'll regenerate without those hunks for v2.

Patrick

>
>
>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>   		      ? 1 : -1];
>>> @@ -6274,7 +6274,7 @@ else
>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>       since some C++ compilers masquerading as C compilers
>>>       incorrectly reject 9223372036854775807.  */
>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>   		      ? 1 : -1];
>>> @@ -6298,7 +6298,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>       since some C++ compilers masquerading as C compilers
>>>       incorrectly reject 9223372036854775807.  */
>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>   		      ? 1 : -1];
>>> @@ -6343,7 +6343,7 @@ else
>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>       since some C++ compilers masquerading as C compilers
>>>       incorrectly reject 9223372036854775807.  */
>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>   		      ? 1 : -1];
>>> @@ -6367,7 +6367,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>       since some C++ compilers masquerading as C compilers
>>>       incorrectly reject 9223372036854775807.  */
>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>   		      ? 1 : -1];
>>> @@ -30820,6 +30820,37 @@ if test $gcc_cv_as_riscv_march_zifencei = yes; then
>>>
>>>   $as_echo "#define HAVE_AS_MARCH_ZIFENCEI 1" >>confdefs.h
>>>
>>> +fi
>>> +
>>> +    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for -march=rv32i_zaamo_zalrsc support" >&5
>>> +$as_echo_n "checking assembler for -march=rv32i_zaamo_zalrsc support... " >&6; }
>>> +if ${gcc_cv_as_riscv_march_zaamo_zalrsc+:} false; then :
>>> +  $as_echo_n "(cached) " >&6
>>> +else
>>> +  gcc_cv_as_riscv_march_zaamo_zalrsc=no
>>> +  if test x$gcc_cv_as != x; then
>>> +    $as_echo '' > conftest.s
>>> +    if { ac_try='$gcc_cv_as $gcc_cv_as_flags -march=rv32i_zaamo_zalrsc -o conftest.o conftest.s >&5'
>>> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>>> +  (eval $ac_try) 2>&5
>>> +  ac_status=$?
>>> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>>> +  test $ac_status = 0; }; }
>>> +    then
>>> +	gcc_cv_as_riscv_march_zaamo_zalrsc=yes
>>> +    else
>>> +      echo "configure: failed program was" >&5
>>> +      cat conftest.s >&5
>>> +    fi
>>> +    rm -f conftest.o conftest.s
>>> +  fi
>>> +fi
>>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_riscv_march_zaamo_zalrsc" >&5
>>> +$as_echo "$gcc_cv_as_riscv_march_zaamo_zalrsc" >&6; }
>>> +if test $gcc_cv_as_riscv_march_zaamo_zalrsc = yes; then
>>> +
>>> +$as_echo "#define HAVE_AS_MARCH_ZAAMO_ZALRSC 1" >>confdefs.h
>>> +
>>>   fi
>>>
>>>       ;;
>>> diff --git a/gcc/configure.ac b/gcc/configure.ac
>>> index f8d67efeb98..c54748cd9aa 100644
>>> --- a/gcc/configure.ac
>>> +++ b/gcc/configure.ac
>>> @@ -5452,6 +5452,11 @@ configured with --enable-newlib-nano-formatted-io.])
>>>         [-march=rv32i_zifencei2p0],,,
>>>         [AC_DEFINE(HAVE_AS_MARCH_ZIFENCEI, 1,
>>>   		 [Define if the assembler understands -march=rv*_zifencei.])])
>>> +    gcc_GAS_CHECK_FEATURE([-march=rv32i_zaamo_zalrsc support],
>>> +      gcc_cv_as_riscv_march_zaamo_zalrsc,
>>> +      [-march=rv32i_zaamo_zalrsc],,,
>>> +      [AC_DEFINE(HAVE_AS_MARCH_ZAAMO_ZALRSC, 1,
>>> +		 [Define if the assembler understands -march=rv*_zaamo_zalrsc.])])
>>>       ;;
>>>       loongarch*-*-*)
>>>       gcc_GAS_CHECK_FEATURE([.dtprelword support],

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
  2024-06-12 23:56     ` Patrick O'Neill
@ 2024-06-13  0:01       ` Palmer Dabbelt
  2024-06-13  0:10         ` Sam James
  0 siblings, 1 reply; 8+ messages in thread
From: Palmer Dabbelt @ 2024-06-13  0:01 UTC (permalink / raw)
  To: Patrick O'Neill
  Cc: sam, gcc-patches, Kito Cheng, jeffreyalaw, gnu-toolchain,
	pan2.li, schwab, Andrea Parri

On Wed, 12 Jun 2024 16:56:09 PDT (-0700), Patrick O'Neill wrote:
>
> On 6/12/24 16:49, Sam James wrote:
>> Palmer Dabbelt <palmer@dabbelt.com> writes:
>>
>>> On Wed, 12 Jun 2024 16:20:26 PDT (-0700), Patrick O'Neill wrote:
>>>> Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
>>>> check to prevent emitting Zaamo/Zalrsc in the arch string when the
>>>> assember does not support it.
>>> Should we just rewrite these to A when binutils doesn't support the
>>> subsets?  That'd avoid a forced binutils bump, but really user should
>>> be upgrading anyway...  Either way
>>>
>>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>>> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>>>
>>> though I'm not suer if the configure churn is sane, it looks like a
>>> version mismatch of some sort.  Hopefully someone who knows those bits
>>> better can chime in?
>> Your instinct is right!
>>
>>>> gcc/ChangeLog:
>>>>
>>>> 	* common/config/riscv/riscv-common.cc
>>>> 	  (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
>>>> 	  supported by the assembler.
>>>> 	* config.in: Regenerate.
>>>> 	* configure: Regenerate.
>>>> 	* configure.ac: Add zaamo/zalrsc assmeber check.
>>>>
>>>> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
>>>> ---
>>>> Tested using newlib rv64gc with binutils tip-of-tree and 2.42.
>>>>
>>>> This results in calls being emitted when compiling for _zaamo_zalrsc
>>>> when the assember does not support these extensions.
>>>>
>>>>> cat amo.c
>>>> void foo (int* bar, int* baz)
>>>> {
>>>>    __atomic_add_fetch(bar, baz, __ATOMIC_RELAXED);
>>>> }
>>>>> gcc -march=rv64id_zaamo_zalrsc -O3 amo.c
>>>> results in:
>>>> foo:
>>>>          sext.w  a1,a1
>>>>          li      a2,0
>>>>          tail    __atomic_fetch_add_4
>>>>
>>>> As a result there are some testsuite failures on zalrsc specific
>>>> testcases and when using an old version of binutils on non-a targets.
>>>> Not a cause for concern imo but worth calling out.
>>>> Also testcases that check for the default isa string will fail with
>>>> the old binutils since zaamo/zalrsc aren't emitted anymore.
>>>> ---
>>>>   gcc/common/config/riscv/riscv-common.cc | 11 +++++++
>>>>   gcc/config.in                           |  6 ++++
>>>>   gcc/configure                           | 41 ++++++++++++++++++++++---
>>>>   gcc/configure.ac                        |  5 +++
>>>>   4 files changed, 58 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
>>>> index 78dfd6b1470..1dc1d9904c7 100644
>>>> --- a/gcc/common/config/riscv/riscv-common.cc
>>>> +++ b/gcc/common/config/riscv/riscv-common.cc
>>>> @@ -916,6 +916,7 @@ riscv_subset_list::to_string (bool version_p) const
>>>>     riscv_subset_t *subset;
>>>>
>>>>     bool skip_zifencei = false;
>>>> +  bool skip_zaamo_zalrsc = false;
>>>>     bool skip_zicsr = false;
>>>>     bool i2p0 = false;
>>>>
>>>> @@ -943,6 +944,10 @@ riscv_subset_list::to_string (bool version_p) const
>>>>        a mistake in that binutils 2.35 supports zicsr but not zifencei.  */
>>>>     skip_zifencei = true;
>>>>   #endif
>>>> +#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
>>>> +  /* Skip since binutils 2.42 and earlier don't recognize zaamo/zalrsc.  */
>>>> +  skip_zaamo_zalrsc = true;
>>>> +#endif
>>>>
>>>>     for (subset = m_head; subset != NULL; subset = subset->next)
>>>>       {
>>>> @@ -954,6 +959,12 @@ riscv_subset_list::to_string (bool version_p) const
>>>>   	  subset->name == "zicsr")
>>>>   	continue;
>>>>
>>>> +      if (skip_zaamo_zalrsc && subset->name == "zaamo")
>>>> +	continue;
>>>> +
>>>> +      if (skip_zaamo_zalrsc && subset->name == "zalrsc")
>>>> +	continue;
>>>> +
>>>>         /* For !version_p, we only separate extension with underline for
>>>>   	 multi-letter extension.  */
>>>>         if (!first &&
>>>> diff --git a/gcc/config.in b/gcc/config.in
>>>> index e41b6dc97cd..acab3c0f126 100644
>>>> --- a/gcc/config.in
>>>> +++ b/gcc/config.in
>>>> @@ -629,6 +629,12 @@
>>>>   #endif
>>>>
>>>>
>>>> +/* Define if the assembler understands -march=rv*_zaamo_zalrsc. */
>>>> +#ifndef USED_FOR_TARGET
>>>> +#undef HAVE_AS_MARCH_ZAAMO_ZALRSC
>>>> +#endif
>>>> +
>>>> +
>>>>   /* Define if the assembler understands -march=rv*_zifencei. */
>>>>   #ifndef USED_FOR_TARGET
>>>>   #undef HAVE_AS_MARCH_ZIFENCEI
>>>> diff --git a/gcc/configure b/gcc/configure
>>>> index aaf5899cc03..09b794c1225 100755
>>>> --- a/gcc/configure
>>>> +++ b/gcc/configure
>>>> @@ -6228,7 +6228,7 @@ else
>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>       since some C++ compilers masquerading as C compilers
>>>>       incorrectly reject 9223372036854775807.  */
>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>> I think you may be using patched autoconf which fixes
>> http://bugs.debian.org/742780.
>>
>> The fix landed in 2.70: https://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commit;h=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e.
>>
>> Please drop those hunks.
>
> I thought I could get away with using the apt autoconf2.69 package
> directly ;)
>
> Thanks. I'll regenerate without those hunks for v2.

FWIW, I just use the distro packages, toss the hunks I don't like, and 
thes re-build things to make sure it doesn't fall over and die.  Just 
don't tell the autoconf people that, I'm sure they'd be horrified ;)

>
> Patrick
>
>>
>>
>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>   		      ? 1 : -1];
>>>> @@ -6274,7 +6274,7 @@ else
>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>       since some C++ compilers masquerading as C compilers
>>>>       incorrectly reject 9223372036854775807.  */
>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>   		      ? 1 : -1];
>>>> @@ -6298,7 +6298,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>       since some C++ compilers masquerading as C compilers
>>>>       incorrectly reject 9223372036854775807.  */
>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>   		      ? 1 : -1];
>>>> @@ -6343,7 +6343,7 @@ else
>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>       since some C++ compilers masquerading as C compilers
>>>>       incorrectly reject 9223372036854775807.  */
>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>   		      ? 1 : -1];
>>>> @@ -6367,7 +6367,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>       since some C++ compilers masquerading as C compilers
>>>>       incorrectly reject 9223372036854775807.  */
>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>   		      ? 1 : -1];
>>>> @@ -30820,6 +30820,37 @@ if test $gcc_cv_as_riscv_march_zifencei = yes; then
>>>>
>>>>   $as_echo "#define HAVE_AS_MARCH_ZIFENCEI 1" >>confdefs.h
>>>>
>>>> +fi
>>>> +
>>>> +    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for -march=rv32i_zaamo_zalrsc support" >&5
>>>> +$as_echo_n "checking assembler for -march=rv32i_zaamo_zalrsc support... " >&6; }
>>>> +if ${gcc_cv_as_riscv_march_zaamo_zalrsc+:} false; then :
>>>> +  $as_echo_n "(cached) " >&6
>>>> +else
>>>> +  gcc_cv_as_riscv_march_zaamo_zalrsc=no
>>>> +  if test x$gcc_cv_as != x; then
>>>> +    $as_echo '' > conftest.s
>>>> +    if { ac_try='$gcc_cv_as $gcc_cv_as_flags -march=rv32i_zaamo_zalrsc -o conftest.o conftest.s >&5'
>>>> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>>>> +  (eval $ac_try) 2>&5
>>>> +  ac_status=$?
>>>> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>>>> +  test $ac_status = 0; }; }
>>>> +    then
>>>> +	gcc_cv_as_riscv_march_zaamo_zalrsc=yes
>>>> +    else
>>>> +      echo "configure: failed program was" >&5
>>>> +      cat conftest.s >&5
>>>> +    fi
>>>> +    rm -f conftest.o conftest.s
>>>> +  fi
>>>> +fi
>>>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_riscv_march_zaamo_zalrsc" >&5
>>>> +$as_echo "$gcc_cv_as_riscv_march_zaamo_zalrsc" >&6; }
>>>> +if test $gcc_cv_as_riscv_march_zaamo_zalrsc = yes; then
>>>> +
>>>> +$as_echo "#define HAVE_AS_MARCH_ZAAMO_ZALRSC 1" >>confdefs.h
>>>> +
>>>>   fi
>>>>
>>>>       ;;
>>>> diff --git a/gcc/configure.ac b/gcc/configure.ac
>>>> index f8d67efeb98..c54748cd9aa 100644
>>>> --- a/gcc/configure.ac
>>>> +++ b/gcc/configure.ac
>>>> @@ -5452,6 +5452,11 @@ configured with --enable-newlib-nano-formatted-io.])
>>>>         [-march=rv32i_zifencei2p0],,,
>>>>         [AC_DEFINE(HAVE_AS_MARCH_ZIFENCEI, 1,
>>>>   		 [Define if the assembler understands -march=rv*_zifencei.])])
>>>> +    gcc_GAS_CHECK_FEATURE([-march=rv32i_zaamo_zalrsc support],
>>>> +      gcc_cv_as_riscv_march_zaamo_zalrsc,
>>>> +      [-march=rv32i_zaamo_zalrsc],,,
>>>> +      [AC_DEFINE(HAVE_AS_MARCH_ZAAMO_ZALRSC, 1,
>>>> +		 [Define if the assembler understands -march=rv*_zaamo_zalrsc.])])
>>>>       ;;
>>>>       loongarch*-*-*)
>>>>       gcc_GAS_CHECK_FEATURE([.dtprelword support],

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
  2024-06-13  0:01       ` Palmer Dabbelt
@ 2024-06-13  0:10         ` Sam James
  0 siblings, 0 replies; 8+ messages in thread
From: Sam James @ 2024-06-13  0:10 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Patrick O'Neill, gcc-patches, Kito Cheng, jeffreyalaw,
	gnu-toolchain, pan2.li, schwab, Andrea Parri

[-- Attachment #1: Type: text/plain, Size: 10635 bytes --]

Palmer Dabbelt <palmer@dabbelt.com> writes:

> On Wed, 12 Jun 2024 16:56:09 PDT (-0700), Patrick O'Neill wrote:
>>
>> On 6/12/24 16:49, Sam James wrote:
>>> Palmer Dabbelt <palmer@dabbelt.com> writes:
>>>
>>>> On Wed, 12 Jun 2024 16:20:26 PDT (-0700), Patrick O'Neill wrote:
>>>>> Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
>>>>> check to prevent emitting Zaamo/Zalrsc in the arch string when the
>>>>> assember does not support it.
>>>> Should we just rewrite these to A when binutils doesn't support the
>>>> subsets?  That'd avoid a forced binutils bump, but really user should
>>>> be upgrading anyway...  Either way
>>>>
>>>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>>>> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
>>>>
>>>> though I'm not suer if the configure churn is sane, it looks like a
>>>> version mismatch of some sort.  Hopefully someone who knows those bits
>>>> better can chime in?
>>> Your instinct is right!
>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 	* common/config/riscv/riscv-common.cc
>>>>> 	  (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
>>>>> 	  supported by the assembler.
>>>>> 	* config.in: Regenerate.
>>>>> 	* configure: Regenerate.
>>>>> 	* configure.ac: Add zaamo/zalrsc assmeber check.
>>>>>
>>>>> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
>>>>> ---
>>>>> Tested using newlib rv64gc with binutils tip-of-tree and 2.42.
>>>>>
>>>>> This results in calls being emitted when compiling for _zaamo_zalrsc
>>>>> when the assember does not support these extensions.
>>>>>
>>>>>> cat amo.c
>>>>> void foo (int* bar, int* baz)
>>>>> {
>>>>>    __atomic_add_fetch(bar, baz, __ATOMIC_RELAXED);
>>>>> }
>>>>>> gcc -march=rv64id_zaamo_zalrsc -O3 amo.c
>>>>> results in:
>>>>> foo:
>>>>>          sext.w  a1,a1
>>>>>          li      a2,0
>>>>>          tail    __atomic_fetch_add_4
>>>>>
>>>>> As a result there are some testsuite failures on zalrsc specific
>>>>> testcases and when using an old version of binutils on non-a targets.
>>>>> Not a cause for concern imo but worth calling out.
>>>>> Also testcases that check for the default isa string will fail with
>>>>> the old binutils since zaamo/zalrsc aren't emitted anymore.
>>>>> ---
>>>>>   gcc/common/config/riscv/riscv-common.cc | 11 +++++++
>>>>>   gcc/config.in                           |  6 ++++
>>>>>   gcc/configure                           | 41 ++++++++++++++++++++++---
>>>>>   gcc/configure.ac                        |  5 +++
>>>>>   4 files changed, 58 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
>>>>> index 78dfd6b1470..1dc1d9904c7 100644
>>>>> --- a/gcc/common/config/riscv/riscv-common.cc
>>>>> +++ b/gcc/common/config/riscv/riscv-common.cc
>>>>> @@ -916,6 +916,7 @@ riscv_subset_list::to_string (bool version_p) const
>>>>>     riscv_subset_t *subset;
>>>>>
>>>>>     bool skip_zifencei = false;
>>>>> +  bool skip_zaamo_zalrsc = false;
>>>>>     bool skip_zicsr = false;
>>>>>     bool i2p0 = false;
>>>>>
>>>>> @@ -943,6 +944,10 @@ riscv_subset_list::to_string (bool version_p) const
>>>>>        a mistake in that binutils 2.35 supports zicsr but not zifencei.  */
>>>>>     skip_zifencei = true;
>>>>>   #endif
>>>>> +#ifndef HAVE_AS_MARCH_ZAAMO_ZALRSC
>>>>> +  /* Skip since binutils 2.42 and earlier don't recognize zaamo/zalrsc.  */
>>>>> +  skip_zaamo_zalrsc = true;
>>>>> +#endif
>>>>>
>>>>>     for (subset = m_head; subset != NULL; subset = subset->next)
>>>>>       {
>>>>> @@ -954,6 +959,12 @@ riscv_subset_list::to_string (bool version_p) const
>>>>>   	  subset->name == "zicsr")
>>>>>   	continue;
>>>>>
>>>>> +      if (skip_zaamo_zalrsc && subset->name == "zaamo")
>>>>> +	continue;
>>>>> +
>>>>> +      if (skip_zaamo_zalrsc && subset->name == "zalrsc")
>>>>> +	continue;
>>>>> +
>>>>>         /* For !version_p, we only separate extension with underline for
>>>>>   	 multi-letter extension.  */
>>>>>         if (!first &&
>>>>> diff --git a/gcc/config.in b/gcc/config.in
>>>>> index e41b6dc97cd..acab3c0f126 100644
>>>>> --- a/gcc/config.in
>>>>> +++ b/gcc/config.in
>>>>> @@ -629,6 +629,12 @@
>>>>>   #endif
>>>>>
>>>>>
>>>>> +/* Define if the assembler understands -march=rv*_zaamo_zalrsc. */
>>>>> +#ifndef USED_FOR_TARGET
>>>>> +#undef HAVE_AS_MARCH_ZAAMO_ZALRSC
>>>>> +#endif
>>>>> +
>>>>> +
>>>>>   /* Define if the assembler understands -march=rv*_zifencei. */
>>>>>   #ifndef USED_FOR_TARGET
>>>>>   #undef HAVE_AS_MARCH_ZIFENCEI
>>>>> diff --git a/gcc/configure b/gcc/configure
>>>>> index aaf5899cc03..09b794c1225 100755
>>>>> --- a/gcc/configure
>>>>> +++ b/gcc/configure
>>>>> @@ -6228,7 +6228,7 @@ else
>>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>>       since some C++ compilers masquerading as C compilers
>>>>>       incorrectly reject 9223372036854775807.  */
>>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>> I think you may be using patched autoconf which fixes
>>> http://bugs.debian.org/742780.
>>>
>>> The fix landed in 2.70: https://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commit;h=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e.
>>>
>>> Please drop those hunks.
>>
>> I thought I could get away with using the apt autoconf2.69 package
>> directly ;)
>>
>> Thanks. I'll regenerate without those hunks for v2.
>
> FWIW, I just use the distro packages, toss the hunks I don't like, and
> thes re-build things to make sure it doesn't fall over and die.  Just
> don't tell the autoconf people that, I'm sure they'd be horrified ;)

On good Linux distributions, you can do:
$ emerge dev-build/autoconf-vanilla dev-build/automake-vanilla
$ export WANT_AUTOCONF=vanilla-2.69 WANT_AUTOMAKE=vanilla-1.15
$ autoconf --version
autoconf (GNU Autoconf) 2.69
[...]

;)

>
>>
>> Patrick
>>
>>>
>>>
>>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>>   		      ? 1 : -1];
>>>>> @@ -6274,7 +6274,7 @@ else
>>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>>       since some C++ compilers masquerading as C compilers
>>>>>       incorrectly reject 9223372036854775807.  */
>>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>>   		      ? 1 : -1];
>>>>> @@ -6298,7 +6298,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>>       since some C++ compilers masquerading as C compilers
>>>>>       incorrectly reject 9223372036854775807.  */
>>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>>   		      ? 1 : -1];
>>>>> @@ -6343,7 +6343,7 @@ else
>>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>>       since some C++ compilers masquerading as C compilers
>>>>>       incorrectly reject 9223372036854775807.  */
>>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>>   		      ? 1 : -1];
>>>>> @@ -6367,7 +6367,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>>>>>       We can't simply define LARGE_OFF_T to be 9223372036854775807,
>>>>>       since some C++ compilers masquerading as C compilers
>>>>>       incorrectly reject 9223372036854775807.  */
>>>>> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
>>>>> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>>>>>     int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>>>>>   		       && LARGE_OFF_T % 2147483647 == 1)
>>>>>   		      ? 1 : -1];
>>>>> @@ -30820,6 +30820,37 @@ if test $gcc_cv_as_riscv_march_zifencei = yes; then
>>>>>
>>>>>   $as_echo "#define HAVE_AS_MARCH_ZIFENCEI 1" >>confdefs.h
>>>>>
>>>>> +fi
>>>>> +
>>>>> +    { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for -march=rv32i_zaamo_zalrsc support" >&5
>>>>> +$as_echo_n "checking assembler for -march=rv32i_zaamo_zalrsc support... " >&6; }
>>>>> +if ${gcc_cv_as_riscv_march_zaamo_zalrsc+:} false; then :
>>>>> +  $as_echo_n "(cached) " >&6
>>>>> +else
>>>>> +  gcc_cv_as_riscv_march_zaamo_zalrsc=no
>>>>> +  if test x$gcc_cv_as != x; then
>>>>> +    $as_echo '' > conftest.s
>>>>> +    if { ac_try='$gcc_cv_as $gcc_cv_as_flags -march=rv32i_zaamo_zalrsc -o conftest.o conftest.s >&5'
>>>>> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>>>>> +  (eval $ac_try) 2>&5
>>>>> +  ac_status=$?
>>>>> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>>>>> +  test $ac_status = 0; }; }
>>>>> +    then
>>>>> +	gcc_cv_as_riscv_march_zaamo_zalrsc=yes
>>>>> +    else
>>>>> +      echo "configure: failed program was" >&5
>>>>> +      cat conftest.s >&5
>>>>> +    fi
>>>>> +    rm -f conftest.o conftest.s
>>>>> +  fi
>>>>> +fi
>>>>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_riscv_march_zaamo_zalrsc" >&5
>>>>> +$as_echo "$gcc_cv_as_riscv_march_zaamo_zalrsc" >&6; }
>>>>> +if test $gcc_cv_as_riscv_march_zaamo_zalrsc = yes; then
>>>>> +
>>>>> +$as_echo "#define HAVE_AS_MARCH_ZAAMO_ZALRSC 1" >>confdefs.h
>>>>> +
>>>>>   fi
>>>>>
>>>>>       ;;
>>>>> diff --git a/gcc/configure.ac b/gcc/configure.ac
>>>>> index f8d67efeb98..c54748cd9aa 100644
>>>>> --- a/gcc/configure.ac
>>>>> +++ b/gcc/configure.ac
>>>>> @@ -5452,6 +5452,11 @@ configured with --enable-newlib-nano-formatted-io.])
>>>>>         [-march=rv32i_zifencei2p0],,,
>>>>>         [AC_DEFINE(HAVE_AS_MARCH_ZIFENCEI, 1,
>>>>>   		 [Define if the assembler understands -march=rv*_zifencei.])])
>>>>> +    gcc_GAS_CHECK_FEATURE([-march=rv32i_zaamo_zalrsc support],
>>>>> +      gcc_cv_as_riscv_march_zaamo_zalrsc,
>>>>> +      [-march=rv32i_zaamo_zalrsc],,,
>>>>> +      [AC_DEFINE(HAVE_AS_MARCH_ZAAMO_ZALRSC, 1,
>>>>> +		 [Define if the assembler understands -march=rv*_zaamo_zalrsc.])])
>>>>>       ;;
>>>>>       loongarch*-*-*)
>>>>>       gcc_GAS_CHECK_FEATURE([.dtprelword support],

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
  2024-06-12 23:20 [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support Patrick O'Neill
  2024-06-12 23:39 ` Palmer Dabbelt
@ 2024-06-13 20:02 ` Jeff Law
  2024-06-17 16:54   ` Patrick O'Neill
  1 sibling, 1 reply; 8+ messages in thread
From: Jeff Law @ 2024-06-13 20:02 UTC (permalink / raw)
  To: Patrick O'Neill, gcc-patches
  Cc: kito.cheng, palmer, gnu-toolchain, pan2.li, schwab, andrea



On 6/12/24 5:20 PM, Patrick O'Neill wrote:
> Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
> check to prevent emitting Zaamo/Zalrsc in the arch string when the
> assember does not support it.
> 
> gcc/ChangeLog:
> 
> 	* common/config/riscv/riscv-common.cc
> 	  (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
> 	  supported by the assembler.
> 	* config.in: Regenerate.
> 	* configure: Regenerate.
> 	* configure.ac: Add zaamo/zalrsc assmeber check.
OK.

It looks like you've got some unexpected diff fragmets in configure -- 
all the LARGE_OFF_T stuff.  They look OK to me, but something like that 
is usually a sign of different autoconf versions.   I wouldn't lose any 
sleep if you left them as-is or removed those hunks before committing.

jeff


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support
  2024-06-13 20:02 ` Jeff Law
@ 2024-06-17 16:54   ` Patrick O'Neill
  0 siblings, 0 replies; 8+ messages in thread
From: Patrick O'Neill @ 2024-06-17 16:54 UTC (permalink / raw)
  To: Jeff Law, gcc-patches
  Cc: kito.cheng, palmer, gnu-toolchain, pan2.li, schwab, andrea


On 6/13/24 13:02, Jeff Law wrote:
>
>
> On 6/12/24 5:20 PM, Patrick O'Neill wrote:
>> Binutils 2.42 and before don't support Zaamo/Zalrsc. Add a configure
>> check to prevent emitting Zaamo/Zalrsc in the arch string when the
>> assember does not support it.
>>
>> gcc/ChangeLog:
>>
>>     * common/config/riscv/riscv-common.cc
>>       (riscv_subset_list::to_string): Skip zaamo/zalrsc when not
>>       supported by the assembler.
>>     * config.in: Regenerate.
>>     * configure: Regenerate.
>>     * configure.ac: Add zaamo/zalrsc assmeber check.
> OK.
>
> It looks like you've got some unexpected diff fragmets in configure -- 
> all the LARGE_OFF_T stuff.  They look OK to me, but something like 
> that is usually a sign of different autoconf versions.   I wouldn't 
> lose any sleep if you left them as-is or removed those hunks before 
> committing.
>
> jeff
>
Removed the hunks and committed.
Sent the committed version to the list for the archiver.

I'll rebase the promotion RFC [1] on top and resolve the warning that 
Andreas Schwab noticed.

Patrick

[1]: 
https://patchwork.sourceware.org/project/gcc/patch/20240613233059.1451117-1-patrick@rivosinc.com/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-06-17 16:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-12 23:20 [PATCH] RISC-V: Add configure check for Zaamo/Zalrsc assembler support Patrick O'Neill
2024-06-12 23:39 ` Palmer Dabbelt
2024-06-12 23:49   ` Sam James
2024-06-12 23:56     ` Patrick O'Neill
2024-06-13  0:01       ` Palmer Dabbelt
2024-06-13  0:10         ` Sam James
2024-06-13 20:02 ` Jeff Law
2024-06-17 16:54   ` Patrick O'Neill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).