On Fri, Nov 15, 2024 at 6:30 AM 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:

You might be able to use dg-message instead. dg-message does not look
for a `note:` (dg-note), `error:` (dg-note) or `warning:`
(dg-warning).

>From gcc-dg.exp:
```
# Look for messages that don't have standard prefixes.
proc dg-message { args } {
```

Thanks,
Andrew Pinski

>
> > 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.
> +
>
>  @opindex mlow-precision-recip-sqrt
>  @opindex mno-low-precision-recip-sqrt
>
>
>
>
> --

Reply via email to