From: Richard Biener <richard.guenther@gmail.com>
To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>,Alan Lawrence
<alan.lawrence@arm.com>,"gcc-patches@gcc.gnu.org"
<gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 1/2][ARM] PR/65956 AAPCS update for alignment attribute
Date: Fri, 03 Jul 2015 18:24:00 -0000 [thread overview]
Message-ID: <CDD5A22C-0AB7-490D-836C-4031AAE36799@gmail.com> (raw)
In-Reply-To: <5596B421.2030806@foss.arm.com>
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;
>> +}
>>
next prev parent reply other threads:[~2015-07-03 18:24 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-03 15:24 [PATCH 0/2][trunk+5 backport][ARM] PR/65956 Implement AAPCS updates " Alan Lawrence
2015-07-03 15:26 ` [PATCH 1/2][ARM] PR/65956 AAPCS update " Alan Lawrence
2015-07-03 16:11 ` Richard Earnshaw
2015-07-03 18:24 ` Richard Biener [this message]
2015-07-03 20:43 ` Richard Earnshaw
2015-07-04 10:57 ` Richard Biener
2015-07-04 11:13 ` Jakub Jelinek
2015-07-06 10:01 ` Alan Lawrence
2015-07-05 13:24 ` Eric Botcazou
2015-07-06 11:00 ` Alan Lawrence
2015-07-06 14:23 ` Ramana Radhakrishnan
2015-07-06 16:38 ` Alan Lawrence
2015-07-06 16:40 ` Ramana Radhakrishnan
2015-07-06 16:45 ` Alan Lawrence
2015-11-04 13:14 ` Jakub Jelinek
2015-11-04 21:30 ` Florian Weimer
2015-11-06 16:48 ` Alan Lawrence
2015-11-06 17:00 ` Jakub Jelinek
2015-11-26 14:05 ` Alan Lawrence
2015-11-27 13:45 ` Alan Lawrence
2015-11-27 18:17 ` Eric Botcazou
2015-11-30 19:40 ` Florian Weimer
2015-07-07 10:29 ` Alan Lawrence
2015-07-03 17:27 ` Jakub Jelinek
2015-07-03 15:27 ` [PATCH 2/2][ARM] fix movdi expander to avoid illegal ldrd/strd Alan Lawrence
2015-07-03 16:16 ` Richard Earnshaw
2015-07-06 17:40 ` Alan Lawrence
2015-07-03 17:12 ` [PATCH 0/2][trunk+5 backport][ARM] PR/65956 Implement AAPCS updates for alignment attribute Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CDD5A22C-0AB7-490D-836C-4031AAE36799@gmail.com \
--to=richard.guenther@gmail.com \
--cc=Richard.Earnshaw@foss.arm.com \
--cc=alan.lawrence@arm.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).