Hi Tamar,
> On 15 Nov 2024, at 14:24, Tamar Christina <tamar.christ...@arm.com> wrote:
>
> Hi All,
>
> This patch makes it so that when you use any of the Cortex-A53 errata
> workarounds but have specified an -march or -mcpu we know is not affected by
> it
> that we suppress the errata workaround.
>
> This is a driver only patch as the linker invocation needs to be changed as
> well. The linker and cc SPECs are different because for the linker we didn't
> seem to add an inversion flag for the option. That said, it's also not
> possible
> to configure the linker with it on by default. So not passing the flag is
> sufficient to turn it off.
>
> For the compilers however we have an inversion flag using -mno-, which is
> needed
> to disable the workarounds when the compiler has been configured with it by
> default.
>
> Note that theoretically speaking -mcpu=native on a Cortex-A53 would turn it
> off,
> but this should be ok because it's unlikely anyone is running GCC-15+ on a
> Cortex-A53 which needs it. If this is a concern I can adjust the patch to for
> targets that have HAVE_LOCAL_CPU_DETECT I can make a new custom function that
> re-queries host detection to see if it's an affected system.
>
> The workaround has the effect of suppressing certain inlining and multiply-add
> formation which leads to about ~1% SPECCPU 2017 Intrate regression on modern
> cores. This patch is needed because most distros configure GCC with the
> workaround enabled by default.
>
> I tried writing automated testcases for these, however the testsuite doesn't
> want to scan the output of -### and it makes the excess error tests always
> fail
> unless you use dg-error, which also looks for"error:". So tested manually:
>
>> gcc -mcpu=neoverse-v1 -mfix-cortex-a53-835769 -xc - -O3 -o - < /dev/null
>> -### 2>&1 | grep "\-mfix" | wc -l
> 0
>
>> gcc -mfix-cortex-a53-835769 -xc - -O3 -o - < /dev/null -### 2>&1 | grep
>> "\-mfix" | wc -l
> 5
>
>> gcc -mfix-cortex-a53-835769 -march=armv8-a -xc - -O3 -o - < /dev/null -###
>> 2>&1 | grep "\-mfix" | wc -l
> 5
>
>> gcc -mfix-cortex-a53-835769 -march=armv8.1-a -xc - -O3 -o - < /dev/null -###
>> 2>&1 | grep "\-mfix" | wc -l
> 0
>
>> gcc -mfix-cortex-a53-835769 -march=armv8.1-a -xc - -O3 -o - < /dev/null -###
>> 2>&1 | grep "\-\-fix" | wc -l
> 0
>
>> gcc -mfix-cortex-a53-835769 -march=armv8-a -xc - -O3 -o - < /dev/null -###
>> 2>&1 | grep "\-\-fix" | wc -l
> 1
>
>> -gcc -mfix-cortex-a53-835769 -xc - -O3 -o - < /dev/null -### 2>&1 | grep
>> "\-\-fix" | wc -l
> 1
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-errata.h (TARGET_SUPPRESS_OPT_SPEC,
> TARGET_TURN_OFF_OPT_SPEC, CA53_ERR_835769_COMPILE_SPEC,
> CA53_ERR_843419_COMPILE_SPEC): New.
> (CA53_ERR_835769_SPEC, CA53_ERR_843419_SPEC): Use them.
> (AARCH64_ERRATA_COMPILE_SPEC):
> * config/aarch64/aarch64-elf-raw.h (CC1_SPEC, CC1PLUS_SPEC): Add
> AARCH64_ERRATA_COMPILE_SPEC.
> * config/aarch64/aarch64-freebsd.h (CC1_SPEC, CC1PLUS_SPEC): Likewise.
> * config/aarch64/aarch64-gnu.h (CC1_SPEC, CC1PLUS_SPEC): Likewise.
> * config/aarch64/aarch64-linux.h (CC1_SPEC, CC1PLUS_SPEC): Likewise.
> * config/aarch64/aarch64-netbsd.h (CC1_SPEC, CC1PLUS_SPEC): Likewise.
> * doc/invoke.texi: Document it.
>
> ---
> diff --git a/gcc/config/aarch64/aarch64-elf-raw.h
> b/gcc/config/aarch64/aarch64-elf-raw.h
> index
> 5396da9b2d626e23e4c4d56e19cd7aa70804c475..8442a664c4fdedd9696da90e6727293c4d472a3f
> 100644
> --- a/gcc/config/aarch64/aarch64-elf-raw.h
> +++ b/gcc/config/aarch64/aarch64-elf-raw.h
> @@ -38,4 +38,12 @@
> AARCH64_ERRATA_LINK_SPEC
> #endif
>
> +#ifndef CC1_SPEC
> +# define CC1_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
> +
> +#ifndef CC1PLUS_SPEC
> +# define CC1PLUS_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
> +
> #endif /* GCC_AARCH64_ELF_RAW_H */
> diff --git a/gcc/config/aarch64/aarch64-errata.h
> b/gcc/config/aarch64/aarch64-errata.h
> index
> c323595ee49553f2b3bc106e993c14f62aee235b..ac0156848abe3e7df669a7ff54e07e72e978c5f0
> 100644
> --- a/gcc/config/aarch64/aarch64-errata.h
> +++ b/gcc/config/aarch64/aarch64-errata.h
> @@ -21,24 +21,61 @@
> #ifndef GCC_AARCH64_ERRATA_H
> #define GCC_AARCH64_ERRATA_H
>
> +/* Completely ignore the option if we've explicitly specify something other
> than
> + mcpu=cortex-a53 or march=armv8-a. */
> +#define TARGET_SUPPRESS_OPT_SPEC(OPT) \
> + "mcpu=*:%{!mcpu=cortex-a53:; " OPT \
> + "}; march=*:%{!march=armv8-a:;" OPT "}; " OPT
> +
> +/* Explicitly turn off the option if we've explicitly specify something other
> + than mcpu=cortex-a53 or march=armv8-a. This will also erase any other
> usage
> + of the flag making the order of the options not relevant. */
> +#define TARGET_TURN_OFF_OPT_SPEC(FLAG) \
> + "mcpu=*:%{!mcpu=cortex-a53:%<m" FLAG " -mno-" FLAG \
> + "}; march=*:%{!march=armv8-a:%<m" FLAG " -mno-" FLAG "}"
> +
> +/* Cortex-A53 835769 Errata. */
> +
> #if TARGET_FIX_ERR_A53_835769_DEFAULT
> -#define CA53_ERR_835769_SPEC \
> - " %{!mno-fix-cortex-a53-835769:--fix-cortex-a53-835769}"
> +#define CA53_ERR_835769_SPEC \
> + " %{" \
> + TARGET_SUPPRESS_OPT_SPEC
> ("!mno-fix-cortex-a53-835769:--fix-cortex-a53-835769") \
> + " }"
> #else
> -#define CA53_ERR_835769_SPEC \
> - " %{mfix-cortex-a53-835769:--fix-cortex-a53-835769}"
> +#define CA53_ERR_835769_SPEC \
> + " %{" \
> + TARGET_SUPPRESS_OPT_SPEC
> ("mfix-cortex-a53-835769:--fix-cortex-a53-835769") \
> + " }"
> #endif
>
> +#define CA53_ERR_835769_COMPILE_SPEC \
> + " %{" TARGET_TURN_OFF_OPT_SPEC ("fix-cortex-a53-835769") "}"
> +
> +/* Cortex-A53 843419 Errata. */
> +
> #if TARGET_FIX_ERR_A53_843419_DEFAULT
> -#define CA53_ERR_843419_SPEC \
> - " %{!mno-fix-cortex-a53-843419:--fix-cortex-a53-843419}"
> +#define CA53_ERR_843419_SPEC \
> + " %{" \
> + TARGET_SUPPRESS_OPT_SPEC
> ("!mno-fix-cortex-a53-843419:--fix-cortex-a53-843419") \
> + " }"
> #else
> -#define CA53_ERR_843419_SPEC \
> - " %{mfix-cortex-a53-843419:--fix-cortex-a53-843419}"
> +#define CA53_ERR_843419_SPEC \
> + " %{" \
> + TARGET_SUPPRESS_OPT_SPEC
> ("mfix-cortex-a53-843419:--fix-cortex-a53-843419") \
> + " }"
> #endif
>
> +#define CA53_ERR_843419_COMPILE_SPEC \
> + " %{" TARGET_TURN_OFF_OPT_SPEC ("fix-cortex-a53-843419") "}"
> +
> +/* Exports to use in SPEC files. */
> +
> #define AARCH64_ERRATA_LINK_SPEC \
> CA53_ERR_835769_SPEC \
> CA53_ERR_843419_SPEC
>
> +#define AARCH64_ERRATA_COMPILE_SPEC \
> + CA53_ERR_835769_COMPILE_SPEC \
> + CA53_ERR_843419_COMPILE_SPEC
> +
> #endif /* GCC_AARCH64_ERRATA_H */
> diff --git a/gcc/config/aarch64/aarch64-freebsd.h
> b/gcc/config/aarch64/aarch64-freebsd.h
> index
> e26d69ce46c7376402c96b846e21a6c0846ebe04..8a76017538a121b4f8ce16d7a64a080d84c72e25
> 100644
> --- a/gcc/config/aarch64/aarch64-freebsd.h
> +++ b/gcc/config/aarch64/aarch64-freebsd.h
> @@ -47,6 +47,14 @@
> -X" SUBTARGET_EXTRA_LINK_SPEC " \
> %{mbig-endian:-EB} %{mlittle-endian:-EL}"
>
> +#ifndef CC1_SPEC
> +# define CC1_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
> +
> +#ifndef CC1PLUS_SPEC
> +# define CC1PLUS_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
> +
> #undef LINK_SPEC
> #define LINK_SPEC FBSD_TARGET_LINK_SPEC AARCH64_ERRATA_LINK_SPEC
>
> diff --git a/gcc/config/aarch64/aarch64-gnu.h
> b/gcc/config/aarch64/aarch64-gnu.h
> index
> ee54940342d3000513aef28d8b29a44d1424c41b..74456263e5837ea8a53b15ebf632ef5407ae2ffb
> 100644
> --- a/gcc/config/aarch64/aarch64-gnu.h
> +++ b/gcc/config/aarch64/aarch64-gnu.h
> @@ -36,6 +36,13 @@
> %{mbig-endian:-EB} %{mlittle-endian:-EL} \
> -maarch64gnu%{mabi=ilp32:32}%{mbig-endian:b}"
>
> +#ifndef CC1_SPEC
> +# define CC1_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
> +
> +#ifndef CC1PLUS_SPEC
> +# define CC1PLUS_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
>
> #define LINK_SPEC GNU_TARGET_LINK_SPEC AARCH64_ERRATA_LINK_SPEC
>
> diff --git a/gcc/config/aarch64/aarch64-linux.h
> b/gcc/config/aarch64/aarch64-linux.h
> index
> 8e51c8202ccc87c19deb97e046ad688899680b62..7582d1734892dbbd550077b0f5a87e095f938ad8
> 100644
> --- a/gcc/config/aarch64/aarch64-linux.h
> +++ b/gcc/config/aarch64/aarch64-linux.h
> @@ -29,8 +29,12 @@
> #undef ASAN_CC1_SPEC
> #define ASAN_CC1_SPEC "%{%:sanitize(address):-funwind-tables}"
>
> -#undef CC1_SPEC
> -#define CC1_SPEC GNU_USER_TARGET_CC1_SPEC ASAN_CC1_SPEC
> +#undef CC1_SPEC
> +#define CC1_SPEC GNU_USER_TARGET_CC1_SPEC ASAN_CC1_SPEC \
> + AARCH64_ERRATA_COMPILE_SPEC
> +
> +#undef CC1PLUS_SPEC
> +#define CC1PLUS_SPEC AARCH64_ERRATA_COMPILE_SPEC
>
> #define CPP_SPEC "%{pthread:-D_REENTRANT}"
>
> diff --git a/gcc/config/aarch64/aarch64-netbsd.h
> b/gcc/config/aarch64/aarch64-netbsd.h
> index
> fb8d10ffd18f9152740199ad8c481fe2f13f3470..57fffcd448fd976b919cf874f00d3341dd6f0757
> 100644
> --- a/gcc/config/aarch64/aarch64-netbsd.h
> +++ b/gcc/config/aarch64/aarch64-netbsd.h
> @@ -39,6 +39,15 @@
> "%{mlittle-endian:-EL -m " TARGET_LINKER_LITTLE_EMULATION "} " \
> "%(netbsd_link_spec)"
>
> +
> +#ifndef CC1_SPEC
> +# define CC1_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
> +
> +#ifndef CC1PLUS_SPEC
> +# define CC1PLUS_SPEC AARCH64_ERRATA_COMPILE_SPEC
> +#endif
> +
> #undef LINK_SPEC
> #define LINK_SPEC NETBSD_LINK_SPEC_ELF \
> NETBSD_TARGET_LINK_SPEC \
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index
> 4a494f6a668cb0e51cdf56af56c094d982812458..0280126c5c28f563b1956af874cc90cd5886d09e
> 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -21361,7 +21361,9 @@ This option requires binutils 2.26 or newer.
> @itemx -mno-fix-cortex-a53-835769
> Enable or disable the workaround for the ARM Cortex-A53 erratum number 835769.
> This involves inserting a NOP instruction between memory instructions and
> -64-bit integer multiply-accumulate instructions.
> +64-bit integer multiply-accumulate instructions. This flag will be ignored
> if
> +an architecture or cpu is specified on the commandline which does not need
> the
> +workaround.
>
> @opindex mfix-cortex-a53-843419
> @opindex mno-fix-cortex-a53-843419
> @@ -21369,7 +21371,10 @@ This involves inserting a NOP instruction between
> memory instructions and
> @itemx -mno-fix-cortex-a53-843419
> Enable or disable the workaround for the ARM Cortex-A53 erratum number 843419.
> This erratum workaround is made at link time and this will only pass the
> -corresponding flag to the linker.
> +corresponding flag to the linker. This flag will be ignored if
> +an architecture or cpu is specified on the commandline which does not need
> the
> +workaround.
Let’s use “command line” with a space.
Ok.
Thanks,
Kyrill
> +
>
> @opindex mlow-precision-recip-sqrt
> @opindex mno-low-precision-recip-sqrt
>
>
>
>
> --
> <rb18968.patch>
