[PATCH 1/2] aarch64: fix ICE in aarch64_layout_arg [PR108411]

[PATCH 1/2] aarch64: fix ICE in aarch64_layout_arg [PR108411]

[Christophe Lyon]

The previous patch added an assert which should not be applied to PST
types (Pure Scalable Types) because alignment does not matter in this
case.  This patch moves the assert after the PST case is handled to
avoid the ICE.

	PR target/108411
	gcc/
	* config/aarch64/aarch64.cc (aarch64_layout_arg): Improve
	comment. Move assert about alignment a bit later.
---
 gcc/config/aarch64/aarch64.cc | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index d36b57341b3..7175b453b3a 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -7659,7 +7659,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
        && (currently_expanding_function_start
 	   || currently_expanding_gimple_stmt));
 
-  /* There are several things to note here:
+  /* HFAs and HVAs can have an alignment greater than 16 bytes.  For example:
+
+       typedef struct foo {
+         __Int8x16_t foo[2] __attribute__((aligned(32)));
+       } foo;
+
+     is still a HVA despite its larger-than-normal alignment.
+     However, such over-aligned HFAs and HVAs are guaranteed to have
+     no padding.
+
+     If we exclude HFAs and HVAs from the discussion below, then there
+     are several things to note:
 
      - Both the C and AAPCS64 interpretations of a type's alignment should
        give a value that is no greater than the type's size.
@@ -7704,12 +7715,6 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
        would treat the alignment as though it was *equal to* 16 bytes.
 
      Both behaviors were wrong, but in different cases.  */
-  unsigned int alignment
-    = aarch64_function_arg_alignment (mode, type, &abi_break,
-				      &abi_break_packed);
-  gcc_assert (alignment <= 16 * BITS_PER_UNIT
-	      && (!alignment || abi_break < alignment)
-	      && (!abi_break_packed || alignment < abi_break_packed));
 
   pcum->aapcs_arg_processed = true;
 
@@ -7780,6 +7785,15 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
 						 &nregs);
   gcc_assert (!sve_p || !allocate_nvrn);
 
+  unsigned int alignment
+    = aarch64_function_arg_alignment (mode, type, &abi_break,
+				      &abi_break_packed);
+
+  gcc_assert (allocate_nvrn || (alignment <= 16 * BITS_PER_UNIT
+				&& (!alignment || abi_break < alignment)
+				&& (!abi_break_packed
+				    || alignment < abi_break_packed)));
+
   /* allocate_ncrn may be false-positive, but allocate_nvrn is quite reliable.
      The following code thus handles passing by SIMD/FP registers first.  */
 
-- 
2.25.1

[PATCH 2/2] aarch64: add -fno-stack-protector to some tests [PR108411]

[Christophe Lyon]

As discussed in the PR, these recently added tests fail when the
testsuite is executed with -fstack-protector-strong.  To avoid this,
this patch adds -fno-stack-protector to dg-options.

	PR target/108411
	gcc/testsuite
	* g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: Add
	-fno-stack-protector.
	* g++.target/aarch64/bitfield-abi-warning-align16-O2.C: Likewise.
	* g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: Likewise.
	* g++.target/aarch64/bitfield-abi-warning-align32-O2.C: Likewise.
	* g++.target/aarch64/bitfield-abi-warning-align8-O2.C: Likewise.
	* gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: Likewise.
	* gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: Likewise.
	* gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: Likewise.
	* gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: Likewise.
	* gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: Likewise.
---
 .../g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C  | 2 +-
 .../g++.target/aarch64/bitfield-abi-warning-align16-O2.C        | 2 +-
 .../g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C  | 2 +-
 .../g++.target/aarch64/bitfield-abi-warning-align32-O2.C        | 2 +-
 .../g++.target/aarch64/bitfield-abi-warning-align8-O2.C         | 2 +-
 .../gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c  | 2 +-
 .../gcc.target/aarch64/bitfield-abi-warning-align16-O2.c        | 2 +-
 .../gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c  | 2 +-
 .../gcc.target/aarch64/bitfield-abi-warning-align32-O2.c        | 2 +-
 .../gcc.target/aarch64/bitfield-abi-warning-align8-O2.c         | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C
index 443cd458b4c..52f9cdd1ee9 100644
--- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C
+++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
 
 #define ALIGN 16
 //#define EXTRA
diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C
index 76a7e3d0ad4..9ff4e46645b 100644
--- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C
+++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
 
 #define ALIGN 16
 #define EXTRA
diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C
index 6f8f54f41ff..55dcbfe4b7c 100644
--- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C
+++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
 
 #define ALIGN 32
 //#define EXTRA
diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C
index 6b8ad5fbea1..6bb8778ee90 100644
--- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C
+++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
 
 #define ALIGN 32
 #define EXTRA
diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C
index b1764d97ea0..41bcc894a2b 100644
--- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C
+++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
 
 #define ALIGN 8
 #define EXTRA
diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c
index f248a129509..3b2c932ac23 100644
--- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c
+++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
 
 #define ALIGN 16
 //#define EXTRA
diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c
index 22ee5ec4c92..ee5d6faa428 100644
--- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c
+++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
 
 #define ALIGN 16
 #define EXTRA
diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c
index a8a50b35e8e..6d4a883a96e 100644
--- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c
+++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
 
 #define ALIGN 32
 //#define EXTRA
diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c
index e872de3dbe0..331daba354c 100644
--- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c
+++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
 
 #define ALIGN 32
 #define EXTRA
diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c
index cb2a945a819..e6d45f5dd5c 100644
--- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c
+++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -save-temps" } */
+/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
 
 #define ALIGN 8
 #define EXTRA
-- 
2.25.1

Re: [PATCH 1/2] aarch64: fix ICE in aarch64_layout_arg [PR108411]

[Richard Sandiford]

Christophe Lyon <christophe.lyon@arm.com> writes:
> The previous patch added an assert which should not be applied to PST
> types (Pure Scalable Types) because alignment does not matter in this
> case.  This patch moves the assert after the PST case is handled to
> avoid the ICE.
>
> 	PR target/108411
> 	gcc/
> 	* config/aarch64/aarch64.cc (aarch64_layout_arg): Improve
> 	comment. Move assert about alignment a bit later.
> ---
>  gcc/config/aarch64/aarch64.cc | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index d36b57341b3..7175b453b3a 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -7659,7 +7659,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
>         && (currently_expanding_function_start
>  	   || currently_expanding_gimple_stmt));
>  
> -  /* There are several things to note here:
> +  /* HFAs and HVAs can have an alignment greater than 16 bytes.  For example:
> +
> +       typedef struct foo {
> +         __Int8x16_t foo[2] __attribute__((aligned(32)));
> +       } foo;
> +
> +     is still a HVA despite its larger-than-normal alignment.
> +     However, such over-aligned HFAs and HVAs are guaranteed to have
> +     no padding.
> +
> +     If we exclude HFAs and HVAs from the discussion below, then there
> +     are several things to note:
>  
>       - Both the C and AAPCS64 interpretations of a type's alignment should
>         give a value that is no greater than the type's size.
> @@ -7704,12 +7715,6 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
>         would treat the alignment as though it was *equal to* 16 bytes.
>  
>       Both behaviors were wrong, but in different cases.  */
> -  unsigned int alignment
> -    = aarch64_function_arg_alignment (mode, type, &abi_break,
> -				      &abi_break_packed);
> -  gcc_assert (alignment <= 16 * BITS_PER_UNIT
> -	      && (!alignment || abi_break < alignment)
> -	      && (!abi_break_packed || alignment < abi_break_packed));
>  
>    pcum->aapcs_arg_processed = true;
>  
> @@ -7780,6 +7785,15 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
>  						 &nregs);
>    gcc_assert (!sve_p || !allocate_nvrn);
>  
> +  unsigned int alignment
> +    = aarch64_function_arg_alignment (mode, type, &abi_break,
> +				      &abi_break_packed);
> +
> +  gcc_assert (allocate_nvrn || (alignment <= 16 * BITS_PER_UNIT
> +				&& (!alignment || abi_break < alignment)
> +				&& (!abi_break_packed
> +				    || alignment < abi_break_packed)));

I think allocate_nvrn should only circumvent the first part, so:

  gcc_assert ((allocate_nvrn || alignment <= 16 * BITS_PER_UNIT)
	      && (!alignment || abi_break < alignment)
	      && (!abi_break_packed || alignment < abi_break_packed));


OK with that change, and sorry for not thinking about this originally.

Richard

> +
>    /* allocate_ncrn may be false-positive, but allocate_nvrn is quite reliable.
>       The following code thus handles passing by SIMD/FP registers first.  */

Re: [PATCH 2/2] aarch64: add -fno-stack-protector to some tests [PR108411]

[Richard Sandiford]

Christophe Lyon <christophe.lyon@arm.com> writes:
> As discussed in the PR, these recently added tests fail when the
> testsuite is executed with -fstack-protector-strong.  To avoid this,
> this patch adds -fno-stack-protector to dg-options.
>
> 	PR target/108411
> 	gcc/testsuite
> 	* g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: Add
> 	-fno-stack-protector.
> 	* g++.target/aarch64/bitfield-abi-warning-align16-O2.C: Likewise.
> 	* g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: Likewise.
> 	* g++.target/aarch64/bitfield-abi-warning-align32-O2.C: Likewise.
> 	* g++.target/aarch64/bitfield-abi-warning-align8-O2.C: Likewise.
> 	* gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: Likewise.
> 	* gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: Likewise.
> 	* gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: Likewise.
> 	* gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: Likewise.
> 	* gcc.target/aarch64/bitfield-abi-warning-align8-O2.c: Likewise.

OK, thanks.

Richard

> ---
>  .../g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C  | 2 +-
>  .../g++.target/aarch64/bitfield-abi-warning-align16-O2.C        | 2 +-
>  .../g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C  | 2 +-
>  .../g++.target/aarch64/bitfield-abi-warning-align32-O2.C        | 2 +-
>  .../g++.target/aarch64/bitfield-abi-warning-align8-O2.C         | 2 +-
>  .../gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c  | 2 +-
>  .../gcc.target/aarch64/bitfield-abi-warning-align16-O2.c        | 2 +-
>  .../gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c  | 2 +-
>  .../gcc.target/aarch64/bitfield-abi-warning-align32-O2.c        | 2 +-
>  .../gcc.target/aarch64/bitfield-abi-warning-align8-O2.c         | 2 +-
>  10 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C
> index 443cd458b4c..52f9cdd1ee9 100644
> --- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C
> +++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
>  
>  #define ALIGN 16
>  //#define EXTRA
> diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C
> index 76a7e3d0ad4..9ff4e46645b 100644
> --- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C
> +++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align16-O2.C
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
>  
>  #define ALIGN 16
>  #define EXTRA
> diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C
> index 6f8f54f41ff..55dcbfe4b7c 100644
> --- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C
> +++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
>  
>  #define ALIGN 32
>  //#define EXTRA
> diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C
> index 6b8ad5fbea1..6bb8778ee90 100644
> --- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C
> +++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align32-O2.C
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
>  
>  #define ALIGN 32
>  #define EXTRA
> diff --git a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C
> index b1764d97ea0..41bcc894a2b 100644
> --- a/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C
> +++ b/gcc/testsuite/g++.target/aarch64/bitfield-abi-warning-align8-O2.C
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps -Wno-narrowing" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps -Wno-narrowing" } */
>  
>  #define ALIGN 8
>  #define EXTRA
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c
> index f248a129509..3b2c932ac23 100644
> --- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c
> +++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
>  
>  #define ALIGN 16
>  //#define EXTRA
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c
> index 22ee5ec4c92..ee5d6faa428 100644
> --- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align16-O2.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
>  
>  #define ALIGN 16
>  #define EXTRA
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c
> index a8a50b35e8e..6d4a883a96e 100644
> --- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c
> +++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
>  
>  #define ALIGN 32
>  //#define EXTRA
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c
> index e872de3dbe0..331daba354c 100644
> --- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align32-O2.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
>  
>  #define ALIGN 32
>  #define EXTRA
> diff --git a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c
> index cb2a945a819..e6d45f5dd5c 100644
> --- a/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/bitfield-abi-warning-align8-O2.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -save-temps" } */
> +/* { dg-options "-O2 -fno-stack-protector -save-temps" } */
>  
>  #define ALIGN 8
>  #define EXTRA

Re: [PATCH 1/2] aarch64: fix ICE in aarch64_layout_arg [PR108411]

[Christophe Lyon]

On 1/19/23 10:22, Richard Sandiford wrote:
> Christophe Lyon <christophe.lyon@arm.com> writes:
>> The previous patch added an assert which should not be applied to PST
>> types (Pure Scalable Types) because alignment does not matter in this
>> case.  This patch moves the assert after the PST case is handled to
>> avoid the ICE.
>>
>> 	PR target/108411
>> 	gcc/
>> 	* config/aarch64/aarch64.cc (aarch64_layout_arg): Improve
>> 	comment. Move assert about alignment a bit later.
>> ---
>>   gcc/config/aarch64/aarch64.cc | 28 +++++++++++++++++++++-------
>>   1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index d36b57341b3..7175b453b3a 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -7659,7 +7659,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
>>          && (currently_expanding_function_start
>>   	   || currently_expanding_gimple_stmt));
>>   
>> -  /* There are several things to note here:
>> +  /* HFAs and HVAs can have an alignment greater than 16 bytes.  For example:
>> +
>> +       typedef struct foo {
>> +         __Int8x16_t foo[2] __attribute__((aligned(32)));
>> +       } foo;
>> +
>> +     is still a HVA despite its larger-than-normal alignment.
>> +     However, such over-aligned HFAs and HVAs are guaranteed to have
>> +     no padding.
>> +
>> +     If we exclude HFAs and HVAs from the discussion below, then there
>> +     are several things to note:
>>   
>>        - Both the C and AAPCS64 interpretations of a type's alignment should
>>          give a value that is no greater than the type's size.
>> @@ -7704,12 +7715,6 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
>>          would treat the alignment as though it was *equal to* 16 bytes.
>>   
>>        Both behaviors were wrong, but in different cases.  */
>> -  unsigned int alignment
>> -    = aarch64_function_arg_alignment (mode, type, &abi_break,
>> -				      &abi_break_packed);
>> -  gcc_assert (alignment <= 16 * BITS_PER_UNIT
>> -	      && (!alignment || abi_break < alignment)
>> -	      && (!abi_break_packed || alignment < abi_break_packed));
>>   
>>     pcum->aapcs_arg_processed = true;
>>   
>> @@ -7780,6 +7785,15 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg)
>>   						 &nregs);
>>     gcc_assert (!sve_p || !allocate_nvrn);
>>   
>> +  unsigned int alignment
>> +    = aarch64_function_arg_alignment (mode, type, &abi_break,
>> +				      &abi_break_packed);
>> +
>> +  gcc_assert (allocate_nvrn || (alignment <= 16 * BITS_PER_UNIT
>> +				&& (!alignment || abi_break < alignment)
>> +				&& (!abi_break_packed
>> +				    || alignment < abi_break_packed)));
> 
> I think allocate_nvrn should only circumvent the first part, so:
> 
>    gcc_assert ((allocate_nvrn || alignment <= 16 * BITS_PER_UNIT)
> 	      && (!alignment || abi_break < alignment)
> 	      && (!abi_break_packed || alignment < abi_break_packed));
> 
> 
> OK with that change, and sorry for not thinking about this originally.

OK thanks, now committed with that change (and after checking the 
testsuite still passes :-) )

Christophe

> 
> Richard
> 
>> +
>>     /* allocate_ncrn may be false-positive, but allocate_nvrn is quite reliable.
>>        The following code thus handles passing by SIMD/FP registers first.  */