On 6/12/24 16:49, Sam James wrote:
Palmer Dabbelt <pal...@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 <pal...@rivosinc.com> # RISC-V
Reviewed-by: Palmer Dabbelt <pal...@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 <patr...@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],

Reply via email to