> On 19 Nov 2024, at 4:18 PM, Tamar Christina <tamar.christ...@arm.com> wrote:
>
> External email: Use caution opening links or attachments
>
>
>> -----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-sqrtLGTM and +1 from me though I cannot approve ! Ramana >>> >>> >>> >>> >>> --
smime.p7s
Description: S/MIME cryptographic signature
