> -----Original Message----- > From: Andrew Pinski <pins...@gmail.com> > Sent: Friday, November 15, 2024 7:16 PM > To: Tamar Christina <tamar.christ...@arm.com> > Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; Richard Earnshaw > <richard.earns...@arm.com>; ktkac...@gcc.gnu.org; Richard Sandiford > <richard.sandif...@arm.com> > Subject: Re: [PATCH]AArch64 Suppress default options when march or mcpu used > is not affected by it. > > 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 for the suggestion, I did try it before but both dg-output and dg-message fail test for excess errors since the -### output goes to stderr. But I realized I misunderstood the dg-message syntax and found a way to silence the excess error. So respinning the patch. Thanks, Tamar > 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..8442a664c4fdedd9696da90 > e6727293c4d472a3f 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..ac0156848abe3e7df669a7ff5 > 4e07e72e978c5f0 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..8a76017538a121b4f8ce16d > 7a64a080d84c72e25 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..74456263e5837ea8a53b15 > ebf632ef5407ae2ffb 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..7582d1734892dbbd550077 > b0f5a87e095f938ad8 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..57fffcd448fd976b919cf874f00 > d3341dd6f0757 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..0280126c5c28f563b1956af8 > 74cc90cd5886d09e 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 > > > > > > > > > > --