[1/2,ARM] PR/65956 AAPCS update for alignment attribute

These include tests of structs, scalars, and vectors - only general-purpose 
registers are affected by the ABI rules for alignment, but we can restrict the 
vector test to use the base AAPCS.

Prior to this patch, align2.c, align3.c and align_rec1.c were failing (the 
latter showing an internal inconsistency, the first two merely that GCC did not 
obey the new ABI).

With this patch, the align_rec2.c fails, and also 
gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a latent bug 
where we can emit strd/ldrd on an odd-numbered register in ARM state, fixed by 
the second patch.

gcc/ChangeLog:

	* config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
	alignment attribute, exploring one level down for aggregates.

gcc/testsuite/ChangeLog:

	* gcc.target/arm/aapcs/align1.c: New.
	* gcc.target/arm/aapcs/align_rec1.c: New.
	* gcc.target/arm/aapcs/align2.c: New.
	* gcc.target/arm/aapcs/align_rec2.c: New.
	* gcc.target/arm/aapcs/align3.c: New.
	* gcc.target/arm/aapcs/align_rec3.c: New.
	* gcc.target/arm/aapcs/align4.c: New.
	* gcc.target/arm/aapcs/align_rec4.c: New.
	* gcc.target/arm/aapcs/align_vararg1.c: New.
	* gcc.target/arm/aapcs/align_vararg2.c: New.

On 03/07/15 16:26, Alan Lawrence wrote:
> These include tests of structs, scalars, and vectors - only
> general-purpose registers are affected by the ABI rules for alignment,
> but we can restrict the vector test to use the base AAPCS.
> 
> Prior to this patch, align2.c, align3.c and align_rec1.c were failing
> (the latter showing an internal inconsistency, the first two merely that
> GCC did not obey the new ABI).
> 
> With this patch, the align_rec2.c fails, and also
> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a latent
> bug where we can emit strd/ldrd on an odd-numbered register in ARM
> state, fixed by the second patch.
> 
> gcc/ChangeLog:
> 
>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>     alignment attribute, exploring one level down for aggregates.
> 
> gcc/testsuite/ChangeLog:
> 
>     * gcc.target/arm/aapcs/align1.c: New.
>     * gcc.target/arm/aapcs/align_rec1.c: New.
>     * gcc.target/arm/aapcs/align2.c: New.
>     * gcc.target/arm/aapcs/align_rec2.c: New.
>     * gcc.target/arm/aapcs/align3.c: New.
>     * gcc.target/arm/aapcs/align_rec3.c: New.
>     * gcc.target/arm/aapcs/align4.c: New.
>     * gcc.target/arm/aapcs/align_rec4.c: New.
>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>     * gcc.target/arm/aapcs/align_vararg2.c: New.
> 
> arm_overalign_1.patch
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS *pcum, tree fntype,
>  static bool
>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>  {
> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
> +  if (!type)
> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
> +
> +  if (!AGGREGATE_TYPE_P (type))
> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
> +
> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
> +      return true;
> +

Technically this is incorrect since AGGREGATE_TYPE_P includes ARRAY_TYPE
and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
case though (unless there's a language that allows passing arrays by value).

For array types I think you need to check TYPE_ALIGN (TREE_TYPE (type)).

R.

> +  return false;
>  }
>  
>  
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
> @@ -0,0 +1,29 @@
> +/* Test AAPCS layout (alignment).  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O" } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align1.c"
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +alignedint a = 11;
> +alignedint b = 13;
> +alignedint c = 17;
> +alignedint d = 19;
> +alignedint e = 23;
> +alignedint f = 29;
> +
> +#include "abitest.h"
> +#else
> +  ARG (alignedint, a, R0)
> +  /* Attribute suggests R2, but we should use only natural alignment:  */
> +  ARG (alignedint, b, R1)
> +  ARG (alignedint, c, R2)
> +  ARG (alignedint, d, R3)
> +  ARG (alignedint, e, STACK)
> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
> +  LAST_ARG (alignedint, f, STACK + 4)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
> @@ -0,0 +1,30 @@
> +/* Test AAPCS layout (alignment).  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O" } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align2.c"
> +
> +/* The underlying struct here has alignment 4.  */
> +typedef struct __attribute__((aligned (8)))
> +  {
> +    int x;
> +    int y;
> +  } overaligned;
> +
> +/* A couple of instances, at 8-byte-aligned memory locations.  */
> +overaligned a = { 2, 3 };
> +overaligned b = { 5, 8 };
> +
> +#include "abitest.h"
> +#else
> +  ARG (int, 7, R0)
> +  /* Alignment should be 4.  */
> +  ARG (overaligned, a, R1)
> +  ARG (int, 9, R3)
> +  ARG (int, 10, STACK)
> +  /* Alignment should be 4.  */
> +  LAST_ARG (overaligned, b, STACK + 4)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
> @@ -0,0 +1,42 @@
> +/* Test AAPCS layout (alignment).  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O3" } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align3.c"
> +
> +/* Struct will be aligned to 8.  */
> +struct s
> +  {
> +    int x;
> +    /* 4 bytes padding here.  */
> +    __attribute__((aligned (8))) int y;
> +    /* 4 bytes padding here.  */
> +  };
> +
> +typedef struct s __attribute__((aligned (4))) underaligned;
> +
> +#define EXPECTED_STRUCT_SIZE 16
> +extern void link_failure (void);
> +int
> +foo ()
> +{
> +  /* Optimization gets rid of this before linking.  */
> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
> +    link_failure ();
> +}
> +
> +underaligned a = { 1, 4 };
> +underaligned b = { 9, 16 };
> +
> +#include "abitest.h"
> +#else
> +  ARG (int, 3, R0)
> +  /* Object alignment is 8, so split between 2 regs and 8 on stack.  */
> +  ARG (underaligned, a, R2)
> +  ARG (int, 6, STACK + 8)
> +  /* Object alignment is 8, so skip over STACK + 12.  */
> +  LAST_ARG (underaligned, b, STACK + 16)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
> @@ -0,0 +1,29 @@
> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-require-effective-target arm_neon_ok  } */
> +/* { dg-options "-O" } */
> +/* { dg-add-options arm_neon } */
> +
> +#ifndef IN_FRAMEWORK
> +#define TESTFILE "align4.c"
> +
> +#define PCSATTR __attribute__((pcs("aapcs")))
> +
> +#include <arm_neon.h>
> +
> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
> +
> +unalignedvec a = {11, 13};
> +unalignedvec b = {17, 19};
> +
> +#include "abitest.h"
> +#else
> +  ARG (int, 2, R0)
> +  /* Attribute suggests R1, but we should use natural alignment:  */
> +  ARG (unalignedvec, a, R2)
> +  ARG (int, 6, STACK)
> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
> +  LAST_ARG (unalignedvec, b, STACK + 8)
> +#endif
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
> @@ -0,0 +1,36 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +alignedint a = 11;
> +alignedint b = 13;
> +alignedint c = 17;
> +alignedint d = 19;
> +alignedint e = 23;
> +alignedint f = 29;
> +
> +void
> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
> +     alignedint stack, alignedint stack4)
> +{
> +  if (r0 != a
> +      || r1 != b
> +      || r2 != c
> +      || r3 != d
> +      || stack != e
> +      || stack4 !=f)
> +    abort ();
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  foo (a, b, c, d, e, f);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
> @@ -0,0 +1,41 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
> +extern void abort (void);
> +
> +typedef struct __attribute__((aligned (8)))
> +  {
> +    int x;
> +    int y;
> +  } overaligned;
> +
> +overaligned a = { 2, 3 };
> +overaligned b = { 5, 8 };
> +
> +void
> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
> +{
> +  if (r0 != 7 || r3 != 9 || stack != 10)
> +    abort ();
> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
> +    abort ();
> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
> +    abort ();
> +  int addr = ((int) &stack4) & 7;
> +  if (addr != 0)
> +    {
> +      __builtin_printf ("Alignment was %d\n", addr);
> +      abort ();
> +    }
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  f (7, a, 9, 10, b);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
> @@ -0,0 +1,43 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
> +extern void abort (void);
> +
> +
> +/* Struct will be aligned to 8.  */
> +struct s
> +  {
> +    int x;
> +    /* 4 bytes padding here.  */
> +    __attribute__((aligned (8))) int y;
> +    /* 4 bytes padding here.  */
> +  };
> +
> +typedef struct s __attribute__((aligned (4))) underaligned;
> +
> +underaligned a = { 1, 4 };
> +underaligned b = { 9, 16 };
> +
> +void
> +f (int r0, underaligned r2, int stack8, underaligned stack16)
> +{
> +  if (r0 != 3 || stack8 != 6)
> +    abort ();
> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
> +    abort ();
> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof (underaligned)))
> +    abort ();
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  f (3, a, 6, b);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
> @@ -0,0 +1,33 @@
> +/* Test AAPCS layout (alignment) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-require-effective-target arm_neon_ok } */
> +/* { dg-options "-O -fno-inline" } */
> +/* { dg-add-options arm_neon } */
> +
> +#include <arm_neon.h>
> +
> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
> +
> +unalignedvec a = {11, 13};
> +unalignedvec b = {17, 19};
> +
> +void
> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
> +{
> +  if (r0 != 2 || s0 != 6
> +      || memcmp ( (void *) &r2, (void *) &a, 16)
> +      || memcmp ( (void *) &s8, (void *) &b, 16))
> +    abort ();
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  foo (2, a, 6, b);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
> @@ -0,0 +1,36 @@
> +/* Test AAPCS layout (alignment of varargs) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +#include <stdarg.h>
> +
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +void
> +foo (int i, ...)
> +{
> +  va_list va;
> +  va_start (va, i);
> +  /* Arguments should be passed in the same registers as if they were ints.  */
> +  while (i-- > 0)
> +    if (va_arg (va, int) != i)
> +      abort ();
> +  va_end (va);
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  alignedint a = 5;
> +  alignedint b = 4;
> +  alignedint c = 3;
> +  alignedint d = 2;
> +  alignedint e = 1;
> +  alignedint f = 0;
> +  foo (a, b, c, d, e, f);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
> @@ -0,0 +1,30 @@
> +/* Test AAPCS layout (alignment of varargs) for callee.  */
> +
> +/* { dg-do run { target arm_eabi } } */
> +/* { dg-require-effective-target arm32 } */
> +/* { dg-options "-O2 -fno-inline" } */
> +
> +#include <stdarg.h>
> +
> +extern void abort (void);
> +
> +typedef __attribute__((aligned (8))) int alignedint;
> +
> +void
> +foo (int i, ...)
> +{
> +  va_list va;
> +  va_start (va, i);
> +  /* alignedint should be pulled out of regs/stack just like an int.  */
> +  while (i-- > 0)
> +    if (va_arg (va, alignedint) != i)
> +      abort ();
> +  va_end (va);
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  foo (5, 4, 3, 2, 1, 0);
> +  return 0;
> +}
>

On Fri, Jul 03, 2015 at 04:26:02PM +0100, Alan Lawrence wrote:
> These include tests of structs, scalars, and vectors - only general-purpose
> registers are affected by the ABI rules for alignment, but we can restrict
> the vector test to use the base AAPCS.
> 
> Prior to this patch, align2.c, align3.c and align_rec1.c were failing (the
> latter showing an internal inconsistency, the first two merely that GCC did
> not obey the new ABI).
> 
> With this patch, the align_rec2.c fails, and also
> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a latent bug
> where we can emit strd/ldrd on an odd-numbered register in ARM state, fixed
> by the second patch.
> 
> gcc/ChangeLog:
> 
> 	* config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
> 	alignment attribute, exploring one level down for aggregates.

Can you please also add the testcase from
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00278.html
to your patch set?  Or I can commit it separately after it is approved
(if it is).

	Jakub

On July 3, 2015 6:11:13 PM GMT+02:00, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote:
>On 03/07/15 16:26, Alan Lawrence wrote:
>> These include tests of structs, scalars, and vectors - only
>> general-purpose registers are affected by the ABI rules for
>alignment,
>> but we can restrict the vector test to use the base AAPCS.
>> 
>> Prior to this patch, align2.c, align3.c and align_rec1.c were failing
>> (the latter showing an internal inconsistency, the first two merely
>that
>> GCC did not obey the new ABI).
>> 
>> With this patch, the align_rec2.c fails, and also
>> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a
>latent
>> bug where we can emit strd/ldrd on an odd-numbered register in ARM
>> state, fixed by the second patch.
>> 
>> gcc/ChangeLog:
>> 
>>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>>     alignment attribute, exploring one level down for aggregates.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>>     * gcc.target/arm/aapcs/align1.c: New.
>>     * gcc.target/arm/aapcs/align_rec1.c: New.
>>     * gcc.target/arm/aapcs/align2.c: New.
>>     * gcc.target/arm/aapcs/align_rec2.c: New.
>>     * gcc.target/arm/aapcs/align3.c: New.
>>     * gcc.target/arm/aapcs/align_rec3.c: New.
>>     * gcc.target/arm/aapcs/align4.c: New.
>>     * gcc.target/arm/aapcs/align_rec4.c: New.
>>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>>     * gcc.target/arm/aapcs/align_vararg2.c: New.
>> 
>> arm_overalign_1.patch
>> 
>> 
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index
>04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581
>100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS
>*pcum, tree fntype,
>>  static bool
>>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>  {
>> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>> +  if (!type)
>> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
>> +
>> +  if (!AGGREGATE_TYPE_P (type))
>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
>> +
>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
>(field))
>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
>> +      return true;
>> +

Is this behavior correct for unions or aggregates with record or union members?

>
>Technically this is incorrect since AGGREGATE_TYPE_P includes
>ARRAY_TYPE
>and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
>case though (unless there's a language that allows passing arrays by
>value).
>
>For array types I think you need to check TYPE_ALIGN (TREE_TYPE
>(type)).
>
>R.
>
>> +  return false;
>>  }
>>  
>>  
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>> @@ -0,0 +1,29 @@
>> +/* Test AAPCS layout (alignment).  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O" } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align1.c"
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +alignedint a = 11;
>> +alignedint b = 13;
>> +alignedint c = 17;
>> +alignedint d = 19;
>> +alignedint e = 23;
>> +alignedint f = 29;
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (alignedint, a, R0)
>> +  /* Attribute suggests R2, but we should use only natural
>alignment:  */
>> +  ARG (alignedint, b, R1)
>> +  ARG (alignedint, c, R2)
>> +  ARG (alignedint, d, R3)
>> +  ARG (alignedint, e, STACK)
>> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
>> +  LAST_ARG (alignedint, f, STACK + 4)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>> @@ -0,0 +1,30 @@
>> +/* Test AAPCS layout (alignment).  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O" } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align2.c"
>> +
>> +/* The underlying struct here has alignment 4.  */
>> +typedef struct __attribute__((aligned (8)))
>> +  {
>> +    int x;
>> +    int y;
>> +  } overaligned;
>> +
>> +/* A couple of instances, at 8-byte-aligned memory locations.  */
>> +overaligned a = { 2, 3 };
>> +overaligned b = { 5, 8 };
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (int, 7, R0)
>> +  /* Alignment should be 4.  */
>> +  ARG (overaligned, a, R1)
>> +  ARG (int, 9, R3)
>> +  ARG (int, 10, STACK)
>> +  /* Alignment should be 4.  */
>> +  LAST_ARG (overaligned, b, STACK + 4)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>> @@ -0,0 +1,42 @@
>> +/* Test AAPCS layout (alignment).  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O3" } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align3.c"
>> +
>> +/* Struct will be aligned to 8.  */
>> +struct s
>> +  {
>> +    int x;
>> +    /* 4 bytes padding here.  */
>> +    __attribute__((aligned (8))) int y;
>> +    /* 4 bytes padding here.  */
>> +  };
>> +
>> +typedef struct s __attribute__((aligned (4))) underaligned;
>> +
>> +#define EXPECTED_STRUCT_SIZE 16
>> +extern void link_failure (void);
>> +int
>> +foo ()
>> +{
>> +  /* Optimization gets rid of this before linking.  */
>> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
>> +    link_failure ();
>> +}
>> +
>> +underaligned a = { 1, 4 };
>> +underaligned b = { 9, 16 };
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (int, 3, R0)
>> +  /* Object alignment is 8, so split between 2 regs and 8 on stack. 
>*/
>> +  ARG (underaligned, a, R2)
>> +  ARG (int, 6, STACK + 8)
>> +  /* Object alignment is 8, so skip over STACK + 12.  */
>> +  LAST_ARG (underaligned, b, STACK + 16)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>> @@ -0,0 +1,29 @@
>> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-require-effective-target arm_neon_ok  } */
>> +/* { dg-options "-O" } */
>> +/* { dg-add-options arm_neon } */
>> +
>> +#ifndef IN_FRAMEWORK
>> +#define TESTFILE "align4.c"
>> +
>> +#define PCSATTR __attribute__((pcs("aapcs")))
>> +
>> +#include <arm_neon.h>
>> +
>> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
>> +
>> +unalignedvec a = {11, 13};
>> +unalignedvec b = {17, 19};
>> +
>> +#include "abitest.h"
>> +#else
>> +  ARG (int, 2, R0)
>> +  /* Attribute suggests R1, but we should use natural alignment:  */
>> +  ARG (unalignedvec, a, R2)
>> +  ARG (int, 6, STACK)
>> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
>> +  LAST_ARG (unalignedvec, b, STACK + 8)
>> +#endif
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>> @@ -0,0 +1,36 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +alignedint a = 11;
>> +alignedint b = 13;
>> +alignedint c = 17;
>> +alignedint d = 19;
>> +alignedint e = 23;
>> +alignedint f = 29;
>> +
>> +void
>> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
>> +     alignedint stack, alignedint stack4)
>> +{
>> +  if (r0 != a
>> +      || r1 != b
>> +      || r2 != c
>> +      || r3 != d
>> +      || stack != e
>> +      || stack4 !=f)
>> +    abort ();
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  foo (a, b, c, d, e, f);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>> @@ -0,0 +1,41 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>> +extern void abort (void);
>> +
>> +typedef struct __attribute__((aligned (8)))
>> +  {
>> +    int x;
>> +    int y;
>> +  } overaligned;
>> +
>> +overaligned a = { 2, 3 };
>> +overaligned b = { 5, 8 };
>> +
>> +void
>> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
>> +{
>> +  if (r0 != 7 || r3 != 9 || stack != 10)
>> +    abort ();
>> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
>> +    abort ();
>> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
>> +    abort ();
>> +  int addr = ((int) &stack4) & 7;
>> +  if (addr != 0)
>> +    {
>> +      __builtin_printf ("Alignment was %d\n", addr);
>> +      abort ();
>> +    }
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  f (7, a, 9, 10, b);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>> @@ -0,0 +1,43 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>> +extern void abort (void);
>> +
>> +
>> +/* Struct will be aligned to 8.  */
>> +struct s
>> +  {
>> +    int x;
>> +    /* 4 bytes padding here.  */
>> +    __attribute__((aligned (8))) int y;
>> +    /* 4 bytes padding here.  */
>> +  };
>> +
>> +typedef struct s __attribute__((aligned (4))) underaligned;
>> +
>> +underaligned a = { 1, 4 };
>> +underaligned b = { 9, 16 };
>> +
>> +void
>> +f (int r0, underaligned r2, int stack8, underaligned stack16)
>> +{
>> +  if (r0 != 3 || stack8 != 6)
>> +    abort ();
>> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
>> +    abort ();
>> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof (underaligned)))
>> +    abort ();
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  f (3, a, 6, b);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>> @@ -0,0 +1,33 @@
>> +/* Test AAPCS layout (alignment) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-require-effective-target arm_neon_ok } */
>> +/* { dg-options "-O -fno-inline" } */
>> +/* { dg-add-options arm_neon } */
>> +
>> +#include <arm_neon.h>
>> +
>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
>> +
>> +unalignedvec a = {11, 13};
>> +unalignedvec b = {17, 19};
>> +
>> +void
>> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
>> +{
>> +  if (r0 != 2 || s0 != 6
>> +      || memcmp ( (void *) &r2, (void *) &a, 16)
>> +      || memcmp ( (void *) &s8, (void *) &b, 16))
>> +    abort ();
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  foo (2, a, 6, b);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>> @@ -0,0 +1,36 @@
>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +#include <stdarg.h>
>> +
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +void
>> +foo (int i, ...)
>> +{
>> +  va_list va;
>> +  va_start (va, i);
>> +  /* Arguments should be passed in the same registers as if they
>were ints.  */
>> +  while (i-- > 0)
>> +    if (va_arg (va, int) != i)
>> +      abort ();
>> +  va_end (va);
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  alignedint a = 5;
>> +  alignedint b = 4;
>> +  alignedint c = 3;
>> +  alignedint d = 2;
>> +  alignedint e = 1;
>> +  alignedint f = 0;
>> +  foo (a, b, c, d, e, f);
>> +  return 0;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>> new file mode 100644
>> index
>0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>> @@ -0,0 +1,30 @@
>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>> +
>> +/* { dg-do run { target arm_eabi } } */
>> +/* { dg-require-effective-target arm32 } */
>> +/* { dg-options "-O2 -fno-inline" } */
>> +
>> +#include <stdarg.h>
>> +
>> +extern void abort (void);
>> +
>> +typedef __attribute__((aligned (8))) int alignedint;
>> +
>> +void
>> +foo (int i, ...)
>> +{
>> +  va_list va;
>> +  va_start (va, i);
>> +  /* alignedint should be pulled out of regs/stack just like an int.
> */
>> +  while (i-- > 0)
>> +    if (va_arg (va, alignedint) != i)
>> +      abort ();
>> +  va_end (va);
>> +}
>> +
>> +int
>> +main (int argc, char **argv)
>> +{
>> +  foo (5, 4, 3, 2, 1, 0);
>> +  return 0;
>> +}
>>

On 03/07/15 19:24, Richard Biener wrote:
> On July 3, 2015 6:11:13 PM GMT+02:00, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote:
>> On 03/07/15 16:26, Alan Lawrence wrote:
>>> These include tests of structs, scalars, and vectors - only
>>> general-purpose registers are affected by the ABI rules for
>> alignment,
>>> but we can restrict the vector test to use the base AAPCS.
>>>
>>> Prior to this patch, align2.c, align3.c and align_rec1.c were failing
>>> (the latter showing an internal inconsistency, the first two merely
>> that
>>> GCC did not obey the new ABI).
>>>
>>> With this patch, the align_rec2.c fails, and also
>>> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a
>> latent
>>> bug where we can emit strd/ldrd on an odd-numbered register in ARM
>>> state, fixed by the second patch.
>>>
>>> gcc/ChangeLog:
>>>
>>>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>>>     alignment attribute, exploring one level down for aggregates.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.target/arm/aapcs/align1.c: New.
>>>     * gcc.target/arm/aapcs/align_rec1.c: New.
>>>     * gcc.target/arm/aapcs/align2.c: New.
>>>     * gcc.target/arm/aapcs/align_rec2.c: New.
>>>     * gcc.target/arm/aapcs/align3.c: New.
>>>     * gcc.target/arm/aapcs/align_rec3.c: New.
>>>     * gcc.target/arm/aapcs/align4.c: New.
>>>     * gcc.target/arm/aapcs/align_rec4.c: New.
>>>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>>>     * gcc.target/arm/aapcs/align_vararg2.c: New.
>>>
>>> arm_overalign_1.patch
>>>
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index
>> 04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581
>> 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS
>> *pcum, tree fntype,
>>>  static bool
>>>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>>  {
>>> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>>> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>>> +  if (!type)
>>> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
>>> +
>>> +  if (!AGGREGATE_TYPE_P (type))
>>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
>>> +
>>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
>> (field))
>>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
>>> +      return true;
>>> +
> 
> Is this behavior correct for unions or aggregates with record or union members?

Yes, at least that was my intention.  It's an error in the wording of
the proposed change, which I think should say "composite types" not
"aggregate types".

R.

> 
>>
>> Technically this is incorrect since AGGREGATE_TYPE_P includes
>> ARRAY_TYPE
>> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
>> case though (unless there's a language that allows passing arrays by
>> value).
>>
>> For array types I think you need to check TYPE_ALIGN (TREE_TYPE
>> (type)).
>>
>> R.
>>
>>> +  return false;
>>>  }
>>>  
>>>  
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>> @@ -0,0 +1,29 @@
>>> +/* Test AAPCS layout (alignment).  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O" } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align1.c"
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +alignedint a = 11;
>>> +alignedint b = 13;
>>> +alignedint c = 17;
>>> +alignedint d = 19;
>>> +alignedint e = 23;
>>> +alignedint f = 29;
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (alignedint, a, R0)
>>> +  /* Attribute suggests R2, but we should use only natural
>> alignment:  */
>>> +  ARG (alignedint, b, R1)
>>> +  ARG (alignedint, c, R2)
>>> +  ARG (alignedint, d, R3)
>>> +  ARG (alignedint, e, STACK)
>>> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
>>> +  LAST_ARG (alignedint, f, STACK + 4)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>> @@ -0,0 +1,30 @@
>>> +/* Test AAPCS layout (alignment).  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O" } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align2.c"
>>> +
>>> +/* The underlying struct here has alignment 4.  */
>>> +typedef struct __attribute__((aligned (8)))
>>> +  {
>>> +    int x;
>>> +    int y;
>>> +  } overaligned;
>>> +
>>> +/* A couple of instances, at 8-byte-aligned memory locations.  */
>>> +overaligned a = { 2, 3 };
>>> +overaligned b = { 5, 8 };
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (int, 7, R0)
>>> +  /* Alignment should be 4.  */
>>> +  ARG (overaligned, a, R1)
>>> +  ARG (int, 9, R3)
>>> +  ARG (int, 10, STACK)
>>> +  /* Alignment should be 4.  */
>>> +  LAST_ARG (overaligned, b, STACK + 4)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>> @@ -0,0 +1,42 @@
>>> +/* Test AAPCS layout (alignment).  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O3" } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align3.c"
>>> +
>>> +/* Struct will be aligned to 8.  */
>>> +struct s
>>> +  {
>>> +    int x;
>>> +    /* 4 bytes padding here.  */
>>> +    __attribute__((aligned (8))) int y;
>>> +    /* 4 bytes padding here.  */
>>> +  };
>>> +
>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>> +
>>> +#define EXPECTED_STRUCT_SIZE 16
>>> +extern void link_failure (void);
>>> +int
>>> +foo ()
>>> +{
>>> +  /* Optimization gets rid of this before linking.  */
>>> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
>>> +    link_failure ();
>>> +}
>>> +
>>> +underaligned a = { 1, 4 };
>>> +underaligned b = { 9, 16 };
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (int, 3, R0)
>>> +  /* Object alignment is 8, so split between 2 regs and 8 on stack. 
>> */
>>> +  ARG (underaligned, a, R2)
>>> +  ARG (int, 6, STACK + 8)
>>> +  /* Object alignment is 8, so skip over STACK + 12.  */
>>> +  LAST_ARG (underaligned, b, STACK + 16)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>> @@ -0,0 +1,29 @@
>>> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-require-effective-target arm_neon_ok  } */
>>> +/* { dg-options "-O" } */
>>> +/* { dg-add-options arm_neon } */
>>> +
>>> +#ifndef IN_FRAMEWORK
>>> +#define TESTFILE "align4.c"
>>> +
>>> +#define PCSATTR __attribute__((pcs("aapcs")))
>>> +
>>> +#include <arm_neon.h>
>>> +
>>> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
>>> +
>>> +unalignedvec a = {11, 13};
>>> +unalignedvec b = {17, 19};
>>> +
>>> +#include "abitest.h"
>>> +#else
>>> +  ARG (int, 2, R0)
>>> +  /* Attribute suggests R1, but we should use natural alignment:  */
>>> +  ARG (unalignedvec, a, R2)
>>> +  ARG (int, 6, STACK)
>>> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
>>> +  LAST_ARG (unalignedvec, b, STACK + 8)
>>> +#endif
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>> @@ -0,0 +1,36 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +alignedint a = 11;
>>> +alignedint b = 13;
>>> +alignedint c = 17;
>>> +alignedint d = 19;
>>> +alignedint e = 23;
>>> +alignedint f = 29;
>>> +
>>> +void
>>> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
>>> +     alignedint stack, alignedint stack4)
>>> +{
>>> +  if (r0 != a
>>> +      || r1 != b
>>> +      || r2 != c
>>> +      || r3 != d
>>> +      || stack != e
>>> +      || stack4 !=f)
>>> +    abort ();
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  foo (a, b, c, d, e, f);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>> @@ -0,0 +1,41 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>>> +extern void abort (void);
>>> +
>>> +typedef struct __attribute__((aligned (8)))
>>> +  {
>>> +    int x;
>>> +    int y;
>>> +  } overaligned;
>>> +
>>> +overaligned a = { 2, 3 };
>>> +overaligned b = { 5, 8 };
>>> +
>>> +void
>>> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
>>> +{
>>> +  if (r0 != 7 || r3 != 9 || stack != 10)
>>> +    abort ();
>>> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
>>> +    abort ();
>>> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
>>> +    abort ();
>>> +  int addr = ((int) &stack4) & 7;
>>> +  if (addr != 0)
>>> +    {
>>> +      __builtin_printf ("Alignment was %d\n", addr);
>>> +      abort ();
>>> +    }
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  f (7, a, 9, 10, b);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>> @@ -0,0 +1,43 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>>> +extern void abort (void);
>>> +
>>> +
>>> +/* Struct will be aligned to 8.  */
>>> +struct s
>>> +  {
>>> +    int x;
>>> +    /* 4 bytes padding here.  */
>>> +    __attribute__((aligned (8))) int y;
>>> +    /* 4 bytes padding here.  */
>>> +  };
>>> +
>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>> +
>>> +underaligned a = { 1, 4 };
>>> +underaligned b = { 9, 16 };
>>> +
>>> +void
>>> +f (int r0, underaligned r2, int stack8, underaligned stack16)
>>> +{
>>> +  if (r0 != 3 || stack8 != 6)
>>> +    abort ();
>>> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
>>> +    abort ();
>>> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof (underaligned)))
>>> +    abort ();
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  f (3, a, 6, b);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>> @@ -0,0 +1,33 @@
>>> +/* Test AAPCS layout (alignment) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-require-effective-target arm_neon_ok } */
>>> +/* { dg-options "-O -fno-inline" } */
>>> +/* { dg-add-options arm_neon } */
>>> +
>>> +#include <arm_neon.h>
>>> +
>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n);
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
>>> +
>>> +unalignedvec a = {11, 13};
>>> +unalignedvec b = {17, 19};
>>> +
>>> +void
>>> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
>>> +{
>>> +  if (r0 != 2 || s0 != 6
>>> +      || memcmp ( (void *) &r2, (void *) &a, 16)
>>> +      || memcmp ( (void *) &s8, (void *) &b, 16))
>>> +    abort ();
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  foo (2, a, 6, b);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>> @@ -0,0 +1,36 @@
>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +#include <stdarg.h>
>>> +
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +void
>>> +foo (int i, ...)
>>> +{
>>> +  va_list va;
>>> +  va_start (va, i);
>>> +  /* Arguments should be passed in the same registers as if they
>> were ints.  */
>>> +  while (i-- > 0)
>>> +    if (va_arg (va, int) != i)
>>> +      abort ();
>>> +  va_end (va);
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  alignedint a = 5;
>>> +  alignedint b = 4;
>>> +  alignedint c = 3;
>>> +  alignedint d = 2;
>>> +  alignedint e = 1;
>>> +  alignedint f = 0;
>>> +  foo (a, b, c, d, e, f);
>>> +  return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>> @@ -0,0 +1,30 @@
>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>> +
>>> +/* { dg-do run { target arm_eabi } } */
>>> +/* { dg-require-effective-target arm32 } */
>>> +/* { dg-options "-O2 -fno-inline" } */
>>> +
>>> +#include <stdarg.h>
>>> +
>>> +extern void abort (void);
>>> +
>>> +typedef __attribute__((aligned (8))) int alignedint;
>>> +
>>> +void
>>> +foo (int i, ...)
>>> +{
>>> +  va_list va;
>>> +  va_start (va, i);
>>> +  /* alignedint should be pulled out of regs/stack just like an int.
>> */
>>> +  while (i-- > 0)
>>> +    if (va_arg (va, alignedint) != i)
>>> +      abort ();
>>> +  va_end (va);
>>> +}
>>> +
>>> +int
>>> +main (int argc, char **argv)
>>> +{
>>> +  foo (5, 4, 3, 2, 1, 0);
>>> +  return 0;
>>> +}
>>>
> 
>

On July 3, 2015 10:43:30 PM GMT+02:00, Richard Earnshaw <Richard.Earnshaw@foss.arm.com> wrote:
>On 03/07/15 19:24, Richard Biener wrote:
>> On July 3, 2015 6:11:13 PM GMT+02:00, Richard Earnshaw
><Richard.Earnshaw@foss.arm.com> wrote:
>>> On 03/07/15 16:26, Alan Lawrence wrote:
>>>> These include tests of structs, scalars, and vectors - only
>>>> general-purpose registers are affected by the ABI rules for
>>> alignment,
>>>> but we can restrict the vector test to use the base AAPCS.
>>>>
>>>> Prior to this patch, align2.c, align3.c and align_rec1.c were
>failing
>>>> (the latter showing an internal inconsistency, the first two merely
>>> that
>>>> GCC did not obey the new ABI).
>>>>
>>>> With this patch, the align_rec2.c fails, and also
>>>> gcc.c-torture/execute/20040709-1.c at -O0 only, both because of a
>>> latent
>>>> bug where we can emit strd/ldrd on an odd-numbered register in ARM
>>>> state, fixed by the second patch.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     * config/arm/arm.c (arm_needs_doubleword_align): Drop any outer
>>>>     alignment attribute, exploring one level down for aggregates.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * gcc.target/arm/aapcs/align1.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec1.c: New.
>>>>     * gcc.target/arm/aapcs/align2.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec2.c: New.
>>>>     * gcc.target/arm/aapcs/align3.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec3.c: New.
>>>>     * gcc.target/arm/aapcs/align4.c: New.
>>>>     * gcc.target/arm/aapcs/align_rec4.c: New.
>>>>     * gcc.target/arm/aapcs/align_vararg1.c: New.
>>>>     * gcc.target/arm/aapcs/align_vararg2.c: New.
>>>>
>>>> arm_overalign_1.patch
>>>>
>>>>
>>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>>> index
>>>
>04663999224c8c8eb8e2d10b0ec634db6ce5027e..ee57d30617a2f7e1cd63ca013fe5655a01027581
>>> 100644
>>>> --- a/gcc/config/arm/arm.c
>>>> +++ b/gcc/config/arm/arm.c
>>>> @@ -6020,8 +6020,17 @@ arm_init_cumulative_args (CUMULATIVE_ARGS
>>> *pcum, tree fntype,
>>>>  static bool
>>>>  arm_needs_doubleword_align (machine_mode mode, const_tree type)
>>>>  {
>>>> -  return (GET_MODE_ALIGNMENT (mode) > PARM_BOUNDARY
>>>> -	  || (type && TYPE_ALIGN (type) > PARM_BOUNDARY));
>>>> +  if (!type)
>>>> +    return PARM_BOUNDARY < GET_MODE_ALIGNMENT (mode);
>>>> +
>>>> +  if (!AGGREGATE_TYPE_P (type))
>>>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
>>>> +
>>>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
>>> (field))
>>>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
>>>> +      return true;

I also believe this loop is equivalent to checking TYPE_ALIGN of the aggregate type?

I'll double check your wording in the abi document, but it seems to be unclear whether packed and not packed structs should be passed the same (considering layout differences).  OTOH the above function is only relevant for register passing? (Likewise the abi document changes?)

>> 
>> Is this behavior correct for unions or aggregates with record or
>union members?
>
>Yes, at least that was my intention.  It's an error in the wording of
>the proposed change, which I think should say "composite types" not
>"aggregate types".
>
>R.
>
>> 
>>>
>>> Technically this is incorrect since AGGREGATE_TYPE_P includes
>>> ARRAY_TYPE
>>> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach
>that
>>> case though (unless there's a language that allows passing arrays by
>>> value).
>>>
>>> For array types I think you need to check TYPE_ALIGN (TREE_TYPE
>>> (type)).
>>>
>>> R.
>>>
>>>> +  return false;
>>>>  }
>>>>  
>>>>  
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..8981d57c3eaf0bd89d224bec79ff8a45627a0a89
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align1.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* Test AAPCS layout (alignment).  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O" } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align1.c"
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +alignedint a = 11;
>>>> +alignedint b = 13;
>>>> +alignedint c = 17;
>>>> +alignedint d = 19;
>>>> +alignedint e = 23;
>>>> +alignedint f = 29;
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (alignedint, a, R0)
>>>> +  /* Attribute suggests R2, but we should use only natural
>>> alignment:  */
>>>> +  ARG (alignedint, b, R1)
>>>> +  ARG (alignedint, c, R2)
>>>> +  ARG (alignedint, d, R3)
>>>> +  ARG (alignedint, e, STACK)
>>>> +  /* Attribute would suggest STACK + 8 but should be ignored:  */
>>>> +  LAST_ARG (alignedint, f, STACK + 4)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..992da53c606c793f25278152406582bb993719d2
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align2.c
>>>> @@ -0,0 +1,30 @@
>>>> +/* Test AAPCS layout (alignment).  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O" } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align2.c"
>>>> +
>>>> +/* The underlying struct here has alignment 4.  */
>>>> +typedef struct __attribute__((aligned (8)))
>>>> +  {
>>>> +    int x;
>>>> +    int y;
>>>> +  } overaligned;
>>>> +
>>>> +/* A couple of instances, at 8-byte-aligned memory locations.  */
>>>> +overaligned a = { 2, 3 };
>>>> +overaligned b = { 5, 8 };
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (int, 7, R0)
>>>> +  /* Alignment should be 4.  */
>>>> +  ARG (overaligned, a, R1)
>>>> +  ARG (int, 9, R3)
>>>> +  ARG (int, 10, STACK)
>>>> +  /* Alignment should be 4.  */
>>>> +  LAST_ARG (overaligned, b, STACK + 4)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..81ad3f587a95aae52ec601ce5a60b198e5351edf
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align3.c
>>>> @@ -0,0 +1,42 @@
>>>> +/* Test AAPCS layout (alignment).  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O3" } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align3.c"
>>>> +
>>>> +/* Struct will be aligned to 8.  */
>>>> +struct s
>>>> +  {
>>>> +    int x;
>>>> +    /* 4 bytes padding here.  */
>>>> +    __attribute__((aligned (8))) int y;
>>>> +    /* 4 bytes padding here.  */
>>>> +  };
>>>> +
>>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>>> +
>>>> +#define EXPECTED_STRUCT_SIZE 16
>>>> +extern void link_failure (void);
>>>> +int
>>>> +foo ()
>>>> +{
>>>> +  /* Optimization gets rid of this before linking.  */
>>>> +  if (sizeof (struct s) != EXPECTED_STRUCT_SIZE)
>>>> +    link_failure ();
>>>> +}
>>>> +
>>>> +underaligned a = { 1, 4 };
>>>> +underaligned b = { 9, 16 };
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (int, 3, R0)
>>>> +  /* Object alignment is 8, so split between 2 regs and 8 on
>stack. 
>>> */
>>>> +  ARG (underaligned, a, R2)
>>>> +  ARG (int, 6, STACK + 8)
>>>> +  /* Object alignment is 8, so skip over STACK + 12.  */
>>>> +  LAST_ARG (underaligned, b, STACK + 16)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..5535c55b8ac895ea31e468fd5474a71c232d2fea
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align4.c
>>>> @@ -0,0 +1,29 @@
>>>> +/* Test AAPCS layout (alignment) - passing vectors in GPRs.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-require-effective-target arm_neon_ok  } */
>>>> +/* { dg-options "-O" } */
>>>> +/* { dg-add-options arm_neon } */
>>>> +
>>>> +#ifndef IN_FRAMEWORK
>>>> +#define TESTFILE "align4.c"
>>>> +
>>>> +#define PCSATTR __attribute__((pcs("aapcs")))
>>>> +
>>>> +#include <arm_neon.h>
>>>> +
>>>> +typedef __attribute__((aligned (4))) int32x2_t unalignedvec;
>>>> +
>>>> +unalignedvec a = {11, 13};
>>>> +unalignedvec b = {17, 19};
>>>> +
>>>> +#include "abitest.h"
>>>> +#else
>>>> +  ARG (int, 2, R0)
>>>> +  /* Attribute suggests R1, but we should use natural alignment: 
>*/
>>>> +  ARG (unalignedvec, a, R2)
>>>> +  ARG (int, 6, STACK)
>>>> +  /* Attribute would suggest STACK + 4 but should be ignored:  */
>>>> +  LAST_ARG (unalignedvec, b, STACK + 8)
>>>> +#endif
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..2e42baefb5877f28b763cc302fd4ef728fb3f72c
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec1.c
>>>> @@ -0,0 +1,36 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +alignedint a = 11;
>>>> +alignedint b = 13;
>>>> +alignedint c = 17;
>>>> +alignedint d = 19;
>>>> +alignedint e = 23;
>>>> +alignedint f = 29;
>>>> +
>>>> +void
>>>> +foo (alignedint r0, alignedint r1, alignedint r2, alignedint r3,
>>>> +     alignedint stack, alignedint stack4)
>>>> +{
>>>> +  if (r0 != a
>>>> +      || r1 != b
>>>> +      || r2 != c
>>>> +      || r3 != d
>>>> +      || stack != e
>>>> +      || stack4 !=f)
>>>> +    abort ();
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  foo (a, b, c, d, e, f);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..a00da508443f6c350dac610851d111d0685f2853
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec2.c
>>>> @@ -0,0 +1,41 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__
>n);
>>>> +extern void abort (void);
>>>> +
>>>> +typedef struct __attribute__((aligned (8)))
>>>> +  {
>>>> +    int x;
>>>> +    int y;
>>>> +  } overaligned;
>>>> +
>>>> +overaligned a = { 2, 3 };
>>>> +overaligned b = { 5, 8 };
>>>> +
>>>> +void
>>>> +f (int r0, overaligned r1, int r3, int stack, overaligned stack4)
>>>> +{
>>>> +  if (r0 != 7 || r3 != 9 || stack != 10)
>>>> +    abort ();
>>>> +  if (memcmp ((void *) &r1, (void *)&a, sizeof (overaligned)))
>>>> +    abort ();
>>>> +  if (memcmp ((void *)&stack4, (void *)&b, sizeof (overaligned)))
>>>> +    abort ();
>>>> +  int addr = ((int) &stack4) & 7;
>>>> +  if (addr != 0)
>>>> +    {
>>>> +      __builtin_printf ("Alignment was %d\n", addr);
>>>> +      abort ();
>>>> +    }
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  f (7, a, 9, 10, b);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..2184cb76a6a7f68c59b39c12ec6472ac7b561794
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec3.c
>>>> @@ -0,0 +1,43 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__
>n);
>>>> +extern void abort (void);
>>>> +
>>>> +
>>>> +/* Struct will be aligned to 8.  */
>>>> +struct s
>>>> +  {
>>>> +    int x;
>>>> +    /* 4 bytes padding here.  */
>>>> +    __attribute__((aligned (8))) int y;
>>>> +    /* 4 bytes padding here.  */
>>>> +  };
>>>> +
>>>> +typedef struct s __attribute__((aligned (4))) underaligned;
>>>> +
>>>> +underaligned a = { 1, 4 };
>>>> +underaligned b = { 9, 16 };
>>>> +
>>>> +void
>>>> +f (int r0, underaligned r2, int stack8, underaligned stack16)
>>>> +{
>>>> +  if (r0 != 3 || stack8 != 6)
>>>> +    abort ();
>>>> +  if (memcmp ((void *) &r2, (void *)&a, sizeof (underaligned)))
>>>> +    abort ();
>>>> +  if (memcmp ((void *)&stack16, (void *)&b, sizeof
>(underaligned)))
>>>> +    abort ();
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  f (3, a, 6, b);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..907b90af70f7ce2ded456d08d6471462e64fa15c
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_rec4.c
>>>> @@ -0,0 +1,33 @@
>>>> +/* Test AAPCS layout (alignment) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-require-effective-target arm_neon_ok } */
>>>> +/* { dg-options "-O -fno-inline" } */
>>>> +/* { dg-add-options arm_neon } */
>>>> +
>>>> +#include <arm_neon.h>
>>>> +
>>>> +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__
>n);
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (4))) int32x4_t unalignedvec;
>>>> +
>>>> +unalignedvec a = {11, 13};
>>>> +unalignedvec b = {17, 19};
>>>> +
>>>> +void
>>>> +foo (int r0, unalignedvec r2, int s0, unalignedvec s8)
>>>> +{
>>>> +  if (r0 != 2 || s0 != 6
>>>> +      || memcmp ( (void *) &r2, (void *) &a, 16)
>>>> +      || memcmp ( (void *) &s8, (void *) &b, 16))
>>>> +    abort ();
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  foo (2, a, 6, b);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..daa321415998df658814d853a15284ae2125cb1e
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_1.c
>>>> @@ -0,0 +1,36 @@
>>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +#include <stdarg.h>
>>>> +
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +void
>>>> +foo (int i, ...)
>>>> +{
>>>> +  va_list va;
>>>> +  va_start (va, i);
>>>> +  /* Arguments should be passed in the same registers as if they
>>> were ints.  */
>>>> +  while (i-- > 0)
>>>> +    if (va_arg (va, int) != i)
>>>> +      abort ();
>>>> +  va_end (va);
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  alignedint a = 5;
>>>> +  alignedint b = 4;
>>>> +  alignedint c = 3;
>>>> +  alignedint d = 2;
>>>> +  alignedint e = 1;
>>>> +  alignedint f = 0;
>>>> +  foo (a, b, c, d, e, f);
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>> b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>>> new file mode 100644
>>>> index
>>>
>0000000000000000000000000000000000000000..b0c923b97edbdf7ee75ce0d2ad868a16f49485fd
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/arm/aapcs/align_vaarg_2.c
>>>> @@ -0,0 +1,30 @@
>>>> +/* Test AAPCS layout (alignment of varargs) for callee.  */
>>>> +
>>>> +/* { dg-do run { target arm_eabi } } */
>>>> +/* { dg-require-effective-target arm32 } */
>>>> +/* { dg-options "-O2 -fno-inline" } */
>>>> +
>>>> +#include <stdarg.h>
>>>> +
>>>> +extern void abort (void);
>>>> +
>>>> +typedef __attribute__((aligned (8))) int alignedint;
>>>> +
>>>> +void
>>>> +foo (int i, ...)
>>>> +{
>>>> +  va_list va;
>>>> +  va_start (va, i);
>>>> +  /* alignedint should be pulled out of regs/stack just like an
>int.
>>> */
>>>> +  while (i-- > 0)
>>>> +    if (va_arg (va, alignedint) != i)
>>>> +      abort ();
>>>> +  va_end (va);
>>>> +}
>>>> +
>>>> +int
>>>> +main (int argc, char **argv)
>>>> +{
>>>> +  foo (5, 4, 3, 2, 1, 0);
>>>> +  return 0;
>>>> +}
>>>>
>> 
>>

On Sat, Jul 04, 2015 at 12:57:36PM +0200, Richard Biener wrote:
> >>>> +  if (!AGGREGATE_TYPE_P (type))
> >>>> +    return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)) > PARM_BOUNDARY;
> >>>> +
> >>>> +  for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN
> >>> (field))
> >>>> +    if (DECL_ALIGN (field) > PARM_BOUNDARY)
> >>>> +      return true;
> 
> I also believe this loop is equivalent to checking TYPE_ALIGN of the aggregate type?

Is it?

What if you do
struct __attribute__((aligned (32))) S { char a; int b; char c; }; ?
In this case, TYPE_MAIN_VARIANT of S is S itself, and has TYPE_USER_ALIGN
and TYPE_ALIGN 256.

	Jakub

> Technically this is incorrect since AGGREGATE_TYPE_P includes ARRAY_TYPE
> and ARRAY_TYPE doesn't have TYPE_FIELDS.  I doubt we could reach that
> case though (unless there's a language that allows passing arrays by value).

Ada passes small array types by the method specified by the pass_by_reference 
hook (and large array types by reference).

Richard Biener wrote:
> 
> I also believe this loop is equivalent to checking TYPE_ALIGN of the aggregate type?

Jakub is correct: the intention is to discard any top-level alignment attribute 
on a struct declaration.

> I'll double check your wording in the abi document, but it seems to be unclear whether packed and not packed structs should be passed the same (considering layout differences).  OTOH the above function is only relevant for register passing? (Likewise the abi document changes?)

It also affects the alignment of things passed on the stack. 'Packed' structs 
are affected too: the outer 'packed' will have no effect on the position on the 
stack / in registers, as you say; layout will still be packed.

>>> Is this behavior correct for unions or aggregates with record or
>> union members?

To clarify Richard Earnshaw's statement: The intention is that 'member 
alignment' is pretty much gcc's TYPE_ALIGN (actually the source code type 
declaration - which is the same for for struct members, but ignoring cases where 
other opts like SRA figure out a larger TYPE_ALIGN). 'Natural alignment' is not 
directly available in GCC under all circumstances, hence having to compute it here.

--Alan