diff mbox series

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

Message ID oro7pssrdz.fsf@lxoliva.fsfla.org
State Accepted
Headers
Series [arm] disable aes-1742098 mitigation for a72 combine tests |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Alexandre Oliva Feb. 17, 2023, 7:06 a.m. UTC 
  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(+)

Comments

Kyrylo Tkachov Feb. 20, 2023, 3:43 p.m. UTC | #1 
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>
Alexandre Oliva Feb. 22, 2023, 5:31 p.m. UTC | #2 
Hello, Kyrylo,

On Feb 20, 2023, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote:

> 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.

Sounds good to me.

> Ok with changing the -mcpu option instead.

Thanks, here's what I've just retested and am now checking in.

[arm] avoid aes-1742098 mitigation in 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.
The mitigation was introduced after the test, and enabled by default
for the selected processor, a72.  Select a53 instead, where the
migitation is not enabled by default, and all the expected fusions can
take place.


for  gcc/testsuite/ChangeLog

	* gcc.target/arm/aes-fuse-1.c: Switch to -mcpu=cortex-a53.
	* gcc.target/arm/aes-fuse-2.c: Likewise.
---
 gcc/testsuite/gcc.target/arm/aes-fuse-1.c |    2 +-
 gcc/testsuite/gcc.target/arm/aes-fuse-2.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/aes-fuse-1.c b/gcc/testsuite/gcc.target/arm/aes-fuse-1.c
index 27b08aeef7ba7..a1bbe054e0a01 100644
--- a/gcc/testsuite/gcc.target/arm/aes-fuse-1.c
+++ b/gcc/testsuite/gcc.target/arm/aes-fuse-1.c
@@ -1,7 +1,7 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_crypto_ok } */
 /* { dg-add-options arm_crypto } */
-/* { dg-additional-options "-mcpu=cortex-a72 -O3 -dp" } */
+/* { dg-additional-options "-mcpu=cortex-a53 -O3 -dp" } */
 
 #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..ede3237ce2692 100644
--- a/gcc/testsuite/gcc.target/arm/aes-fuse-2.c
+++ b/gcc/testsuite/gcc.target/arm/aes-fuse-2.c
@@ -1,7 +1,7 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target arm_crypto_ok } */
 /* { dg-add-options arm_crypto } */
-/* { dg-additional-options "-mcpu=cortex-a72 -O3 -dp" } */
+/* { dg-additional-options "-mcpu=cortex-a53 -O3 -dp" } */
 
 #include <arm_neon.h>
diff mbox series

Patch

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" } */
 
 #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>