On 03/07/15 19:24, Richard Biener wrote:
> On July 3, 2015 6:11:13 PM GMT+02:00, Richard Earnshaw 
> <richard.earns...@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;
>>> +}
>>>
> 
> 

Reply via email to