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.
>> && 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],
signature.asc
Description: PGP signature
