RE: [PATCH] [arm] disable aes-1742098 mitigation for a72 combine tests

[Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Oliva <oliva@adacore.com>
> Sent: Friday, February 17, 2023 7:06 AM
> To: gcc-patches@gcc.gnu.org
> Cc: nickc@redhat.com; Richard Earnshaw <Richard.Earnshaw@arm.com>;
> ramana.gcc@gmail.com; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: [PATCH] [arm] disable aes-1742098 mitigation for a72 combine tests
> 
> 
> The expected asm output for aes-fuse-[12].c does not correspond to
> that which is generated when -mfix-cortex-a57-aes-1742098 is enabled.
> It was introduced after the test, and enabled by default for the
> selected processor.  Disabling the option restores the circumstance
> that was tested for.
> 
> Regstrapped on x86_64-linux-gnu.
> Tested on arm-vxworks7 (gcc-12) and arm-eabi (trunk).  Ok to install?
> 
> for  gcc/testsuite/ChangeLog
> 
> 	* gcc.target/arm/aes-fuse-1.c: Add
> 	-mno-fix-cortex-a57-aes-1742098.
> 	* gcc.target/arm/aes-fuse-2.c: Likewise.
> ---
>  gcc/testsuite/gcc.target/arm/aes-fuse-1.c |    4 ++++
>  gcc/testsuite/gcc.target/arm/aes-fuse-2.c |    4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/gcc/testsuite/gcc.target/arm/aes-fuse-1.c
> b/gcc/testsuite/gcc.target/arm/aes-fuse-1.c
> index 27b08aeef7ba7..6ffb4991cca69 100644
> --- a/gcc/testsuite/gcc.target/arm/aes-fuse-1.c
> +++ b/gcc/testsuite/gcc.target/arm/aes-fuse-1.c
> @@ -2,6 +2,10 @@
>  /* { dg-require-effective-target arm_crypto_ok } */
>  /* { dg-add-options arm_crypto } */
>  /* { dg-additional-options "-mcpu=cortex-a72 -O3 -dp" } */
> +/* The mitigation applies to a72 by default, and protects the CRYPTO_AES
> +   inputs, such as the explicit xor ops, from being combined like test used to
> +   expect.  */
> +/* { dg-additional-options "-mno-fix-cortex-a57-aes-1742098" } */

Actually the -mcpu=cortex-a72 here is significant only in that it's one of the CPUs that enables AES/AESMC fusion.
So rather than overriding this awkward part with -mno-fix-cortex-a57-aes-1742098 I'd rather just select a different
CPU that enables that fusion and isn't afflicted by this workaround, such as -mcpu=cortex-a53.
More broadly, I think we should be enabling tune_params::FUSE_AES_AESMC for the generic target in A profile, but that would be a non-testsuite change.

Ok with changing the -mcpu option instead.
Thanks,
Kyrill

> 
>  #include <arm_neon.h>
> 
> diff --git a/gcc/testsuite/gcc.target/arm/aes-fuse-2.c
> b/gcc/testsuite/gcc.target/arm/aes-fuse-2.c
> index 1266a28753169..b72479c0e5726 100644
> --- a/gcc/testsuite/gcc.target/arm/aes-fuse-2.c
> +++ b/gcc/testsuite/gcc.target/arm/aes-fuse-2.c
> @@ -2,6 +2,10 @@
>  /* { dg-require-effective-target arm_crypto_ok } */
>  /* { dg-add-options arm_crypto } */
>  /* { dg-additional-options "-mcpu=cortex-a72 -O3 -dp" } */
> +/* The mitigation applies to a72 by default, and protects the CRYPTO_AES
> +   inputs, such as the explicit xor ops, from being combined like test used to
> +   expect.  */
> +/* { dg-additional-options "-mno-fix-cortex-a57-aes-1742098" } */
> 
>  #include <arm_neon.h>
> 
> 
> --
> Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
>    Free Software Activist                       GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>