* [PATCH 0/2][AArch64] Implement AAPCS64 updates for alignment attribute @ 2016-01-15 14:17 Alan Lawrence 2016-01-15 14:18 ` [PATCH 1/2][AArch64] " Alan Lawrence ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Alan Lawrence @ 2016-01-15 14:17 UTC (permalink / raw) To: gcc-patches Cc: marcus.shawcroft, james.greenhalgh, richard.earnshaw, charlet, ebotcazou These parallel the updates to ARM https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00214.html following from Richard Earnshaw's proposal for updates to the AAPCS and AAPCS64, https://gcc.gnu.org/ml/gcc/2015-07/msg00040.html . On AArch64 we do not have the problem of broken profiledbootstrap (as originally on ARM), hence I do not propose to backport these to earlier GCC branches. Rather, the GCC 5 -> 6 transition, seems a better time for any ABI change. (However, as previously, there should be no changes for types that are naturally aligned, only those using types with explicit alignment specifiers, which was not previously sanctioned by the AAPCS.) Similarly to ARM, I note that Ada is affected. Indeed, with a gcc 4.9 host compiler, I saw a bootstrap miscompare iff including Ada; however, I was able to bootstrap Ada successfully, if I first built a GCC including this patch with --disable-bootstrap, and then used that as host compiler. The best explanation I can see for this is mismatched host vs built libraries and compiler being used together, something like Jakub's suggestion http://gcc.gnu.org/ml/gcc-patches/2015-11/msg00338.html. I don't feel I have the expertise for this, and am CCing the Ada maintainers in the hope they can help. Bootstrapped + check-gcc + check-g++ on aarch64-none-linux-gnu and aarch64_be-none-elf. Also bootstrapped + check-ada on aarch64-none-linux-gnu with --disable-bootstrap host compiler, as above. OK for trunk? --Alan ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-01-15 14:17 [PATCH 0/2][AArch64] Implement AAPCS64 updates for alignment attribute Alan Lawrence @ 2016-01-15 14:18 ` Alan Lawrence 2016-01-15 14:18 ` [PATCH 2/2][AArch64] Tests of " Alan Lawrence 2016-01-18 17:11 ` [PATCH 0/2][AArch64] Implement " Eric Botcazou 2 siblings, 0 replies; 14+ messages in thread From: Alan Lawrence @ 2016-01-15 14:18 UTC (permalink / raw) To: gcc-patches Cc: marcus.shawcroft, james.greenhalgh, richard.earnshaw, charlet, ebotcazou gcc/ChangeLog: * gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment): Rewrite, looking one level down for records and arrays. --- gcc/config/aarch64/aarch64.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index ae4cfb3..8eb8c3e 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1925,22 +1925,24 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, static unsigned int aarch64_function_arg_alignment (machine_mode mode, const_tree type) { - unsigned int alignment; + if (!type) + return GET_MODE_ALIGNMENT (mode); + if (integer_zerop (TYPE_SIZE (type))) + return 0; - if (type) - { - if (!integer_zerop (TYPE_SIZE (type))) - { - if (TYPE_MODE (type) == mode) - alignment = TYPE_ALIGN (type); - else - alignment = GET_MODE_ALIGNMENT (mode); - } - else - alignment = 0; - } - else - alignment = GET_MODE_ALIGNMENT (mode); + gcc_assert (TYPE_MODE (type) == mode); + + if (!AGGREGATE_TYPE_P (type)) + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); + + if (TREE_CODE (type) == ARRAY_TYPE) + return TYPE_ALIGN (TREE_TYPE (type)); + + unsigned int alignment = 0; + gcc_assert (TYPE_FIELDS (type)); + + for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) + alignment = std::max (alignment, DECL_ALIGN (field)); return alignment; } -- 1.9.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2][AArch64] Tests of AAPCS64 updates for alignment attribute 2016-01-15 14:17 [PATCH 0/2][AArch64] Implement AAPCS64 updates for alignment attribute Alan Lawrence 2016-01-15 14:18 ` [PATCH 1/2][AArch64] " Alan Lawrence @ 2016-01-15 14:18 ` Alan Lawrence 2016-06-07 11:09 ` James Greenhalgh 2016-01-18 17:11 ` [PATCH 0/2][AArch64] Implement " Eric Botcazou 2 siblings, 1 reply; 14+ messages in thread From: Alan Lawrence @ 2016-01-15 14:18 UTC (permalink / raw) To: gcc-patches Cc: marcus.shawcroft, james.greenhalgh, richard.earnshaw, charlet, ebotcazou Here I've added both tests using the abitest.h framework(which verifies values are passed in the correct registers as specified by the AAPCS64), and separate tests which verify that called functions read arguments from the same locations as they are passed. Hence, each test_align-N.c corresponds to rec_align-N.c. I've tried to stay consistent with the existing naming of e.g. test_10.c, test_align-1.c, va_arg-10.c, but would be happy to change to another scheme if preferred! (e.g. func-ret-1.c ?) Cheers, Alan gcc/testsuite/ChangeLog: * gcc.target/aarch64/aapcs64/aapcs64.exp: Also execute rec_*.c * gcc.target//aarch64/aapcs64/rec_align-5.c: New. * gcc.target//aarch64/aapcs64/rec_align-6.c: New. * gcc.target//aarch64/aapcs64/rec_align-7.c: New. * gcc.target//aarch64/aapcs64/rec_align-8.c: New. * gcc.target//aarch64/aapcs64/rec_align-9.c: New. * gcc.target//aarch64/aapcs64/test_align-5.c: New. * gcc.target//aarch64/aapcs64/test_align-6.c: New. * gcc.target//aarch64/aapcs64/test_align-7.c: New. * gcc.target//aarch64/aapcs64/test_align-8.c: New. * gcc.target//aarch64/aapcs64/test_align-9.c: New. * gcc.target//aarch64/aapcs64/rec_vaarg-1.c: New. * gcc.target//aarch64/aapcs64/rec_vaarg-2.c: New. --- .../gcc.target/aarch64/aapcs64/aapcs64.exp | 10 +++++ .../gcc.target/aarch64/aapcs64/rec_align-5.c | 44 ++++++++++++++++++++ .../gcc.target/aarch64/aapcs64/rec_align-6.c | 45 +++++++++++++++++++++ .../gcc.target/aarch64/aapcs64/rec_align-7.c | 47 ++++++++++++++++++++++ .../gcc.target/aarch64/aapcs64/rec_align-8.c | 37 +++++++++++++++++ .../gcc.target/aarch64/aapcs64/rec_align-9.c | 41 +++++++++++++++++++ .../gcc.target/aarch64/aapcs64/rec_align_vaarg-1.c | 38 +++++++++++++++++ .../gcc.target/aarch64/aapcs64/rec_align_vaarg-2.c | 28 +++++++++++++ .../gcc.target/aarch64/aapcs64/test_align-5.c | 35 ++++++++++++++++ .../gcc.target/aarch64/aapcs64/test_align-6.c | 36 +++++++++++++++++ .../gcc.target/aarch64/aapcs64/test_align-7.c | 38 +++++++++++++++++ .../gcc.target/aarch64/aapcs64/test_align-8.c | 33 +++++++++++++++ .../gcc.target/aarch64/aapcs64/test_align-9.c | 47 ++++++++++++++++++++++ 13 files changed, 479 insertions(+) create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-5.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-6.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-7.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-8.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-9.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-1.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-5.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-6.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-7.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-8.c create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-9.c diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp index ac2f089..eb297c5 100644 --- a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp @@ -38,6 +38,16 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/test_*.c]] { } } +# Test parameter receiving. +set additional_flags_for_rec $additional_flags +append additional_flags_for_rec " -fno-inline" +foreach src [lsort [glob -nocomplain $srcdir/$subdir/rec_*.c]] { + if {[runtest_file_p $runtests $src]} { + c-torture-execute [list $src] \ + $additional_flags_for_rec + } +} + # Test unnamed argument retrieval via the va_arg macro. foreach src [lsort [glob -nocomplain $srcdir/$subdir/va_arg-*.c]] { if {[runtest_file_p $runtests $src]} { diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-5.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-5.c new file mode 100644 index 0000000..1b42c92 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-5.c @@ -0,0 +1,44 @@ +/* Test AAPCS layout (alignment) for callee. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +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; +alignedint g = 31; +alignedint h = 37; +alignedint i = 41; +alignedint j = 43; + +void +test_passing_many_alignedint (alignedint x0, alignedint x1, alignedint x2, + alignedint x3, alignedint x4, alignedint x5, + alignedint x6, alignedint x7, alignedint stack, + alignedint stack8) +{ + if (x0 != a + || x1 != b + || x2 != c + || x3 != d + || x4 != e + || x5 != f + || x6 != g + || x7 != h + || stack != i + || stack8 !=j) + abort (); +} + +int +main (int argc, char **argv) +{ + test_passing_many_alignedint (a, b, c, d, e, f, g, h, i, j); + return 0; +} diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-6.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-6.c new file mode 100644 index 0000000..a8d8b1b --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-6.c @@ -0,0 +1,45 @@ +/* Test AAPCS layout (alignment) for callee. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n); +extern void abort (void); + +/* The underlying struct here has alignment 8. */ +typedef struct __attribute__ ((__aligned__ (16))) + { + long x; + long y; + } overaligned; + +overaligned a = { 2, 3 }; +overaligned b = { 5, 8 }; +overaligned c = { 13, 21 }; + +void +test_passing_overaligned_struct (int x0, overaligned x1, int x3, int x4, + overaligned x5, int x7, int stack, + overaligned stack8) +{ + if (x0 != 7 || x3 != 9 || x4 != 11 || x7 != 15 || stack != 10) + abort (); + if (memcmp ((void *) &x1, (void *)&a, sizeof (overaligned))) + abort (); + if (memcmp ((void *) &x5, (void *)&b, sizeof (overaligned))) + abort (); + if (memcmp ((void *)&stack8, (void *)&c, sizeof (overaligned))) + abort (); + long addr = ((long) &stack8) & 15; + if (addr != 0) + { + __builtin_printf ("Alignment was %d\n", addr); + abort (); + } +} + +int +main (int argc, char **argv) +{ + test_passing_overaligned_struct (7, a, 9, 11, b, 15, 10, c); + return 0; +} diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-7.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-7.c new file mode 100644 index 0000000..61e3c11 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-7.c @@ -0,0 +1,47 @@ +/* Test AAPCS layout (alignment) for callee. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n); +extern void abort (void); + +struct s + { + long x; + long y; + }; + +/* This still has size 16, so is still passed by value. */ +typedef __attribute__ ((__aligned__ (32))) struct s overaligned; + +/* A few structs, at 32-byte-aligned memory locations. */ +overaligned a = { 2, 3 }; +overaligned b = { 5, 8 }; +overaligned c = { 13, 21 }; + +void +test_pass_by_value (int x0, overaligned x1, int x3, int x4, overaligned x5, + int x7, int stack, overaligned stack8) +{ + if (x0 != 7 || x3 != 9 || x4 != 11 || x7 != 15 || stack != 10) + abort (); + if (memcmp ((void *) &x1, (void *)&a, sizeof (overaligned))) + abort (); + if (memcmp ((void *) &x5, (void *)&b, sizeof (overaligned))) + abort (); + if (memcmp ((void *)&stack8, (void *)&c, sizeof (overaligned))) + abort (); + long addr = ((long) &stack8) & 15; + if (addr != 0) + { + __builtin_printf ("Alignment was %d\n", addr); + abort (); + } +} + +int +main (int argc, char **argv) +{ + test_pass_by_value (7, a, 9, 11, b, 15, 10, c); + return 0; +} diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-8.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-8.c new file mode 100644 index 0000000..c935180 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-8.c @@ -0,0 +1,37 @@ +/* Test AAPCS layout (alignment) for callee. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n); +extern void abort (void); + +/* The alignment also gives this size 32, so will be passed by reference. */ +typedef struct __attribute__ ((__aligned__ (32))) + { + long x; + long y; + } overaligned; + +overaligned a = { 2, 3 }; + +void +test_pass_by_ref (int x0, overaligned x1, int x2) +{ + if (x0 != 7 || x2 != 9) + abort (); + if (memcmp ((void *) &x1, (void *)&a, sizeof (overaligned))) + abort (); + long addr = ((long) &x1) & 31; + if (addr != 0) + { + __builtin_printf ("Alignment was %d\n", addr); + abort (); + } +} + +int +main (int argc, char **argv) +{ + test_pass_by_ref (7, a, 9); + return 0; +} diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-9.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-9.c new file mode 100644 index 0000000..81139f5 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align-9.c @@ -0,0 +1,41 @@ +/* Test AAPCS layout (alignment) for callee. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +extern int memcmp (const void *s1, const void *s2, __SIZE_TYPE__ n); +extern void abort (void); + +struct s + { + /* This forces the alignment and size of the struct to 16. */ + __attribute__ ((__aligned__ (16))) long x; + int y; + /* 4 bytes padding. */ + }; + +typedef struct s __attribute__ ((__aligned__ (8))) underaligned; + +underaligned a = { 1, 4 }; +underaligned b = { 9, 16 }; +underaligned c = { 25, 36 }; + +void +test_underaligned_struct (int x0, underaligned x2, int x4, underaligned x6, + int stack, underaligned stack16) +{ + if (x0 != 3 || x4 != 5 || stack != 7) + abort (); + if (memcmp ((void *) &x2, (void *)&a, sizeof (underaligned))) + abort (); + if (memcmp ((void *)&x6, (void *)&b, sizeof (underaligned))) + abort (); + if (memcmp ((void *)&stack16, (void *)&c, sizeof (underaligned))) + abort (); +} + +int +main (int argc, char **argv) +{ + test_underaligned_struct (3, a, 5, b, 7, c); + return 0; +} diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-1.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-1.c new file mode 100644 index 0000000..109ddc2 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-1.c @@ -0,0 +1,38 @@ +/* Test AAPCS layout (alignment of varargs) for callee. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#include <stdarg.h> + +extern void abort (void); + +typedef __attribute__ ((__aligned__ (16))) long alignedlong; + +void +test_pass_overaligned_long_vaargs (long l, ...) +{ + va_list va; + va_start (va, l); + /* Arguments should be passed in the same registers as if they were ints. */ + while (l-- > 0) + if (va_arg (va, long) != l) + abort (); + va_end (va); +} + +int +main (int argc, char **argv) +{ + alignedlong a = 9; + alignedlong b = 8; + alignedlong c = 7; + alignedlong d = 6; + alignedlong e = 5; + alignedlong f = 4; + alignedlong g = 3; + alignedlong h = 2; + alignedlong i = 1; + alignedlong j = 0; + test_pass_overaligned_long_vaargs (a, b, c, d, e, f, g, h, i, j); + return 0; +} diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-2.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-2.c new file mode 100644 index 0000000..dc4eb2f --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/rec_align_vaarg-2.c @@ -0,0 +1,28 @@ +/* Test AAPCS layout (alignment of varargs) for callee. */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#include <stdarg.h> + +extern void abort (void); + +typedef __attribute__ ((__aligned__ (16))) int alignedint; + +void +test_pass_overaligned_int_vaargs (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) +{ + test_pass_overaligned_int_vaargs (9, 8, 7, 6, 5, 4, 3, 2, 1, 0); + return 0; +} diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-5.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-5.c new file mode 100644 index 0000000..ac5673e --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-5.c @@ -0,0 +1,35 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-5.c" + +typedef __attribute__ ((__aligned__ (16))) long alignedint; + +alignedint a = 11; +alignedint b = 13; +alignedint c = 17; +alignedint d = 19; +alignedint e = 23; +alignedint f = 29; +alignedint g = 31; +alignedint h = 37; +alignedint i = 41; +alignedint j = 43; + +#include "abitest.h" +#else + ARG (alignedint, a, X0) + /* Attribute suggests R2, but we should use only natural alignment: */ + ARG (alignedint, b, X1) + ARG (alignedint, c, X2) + ARG (alignedint, d, X3) + ARG (alignedint, e, X4) + ARG (alignedint, f, X5) + ARG (alignedint, g, X6) + ARG (alignedint, h, X7) + ARG (alignedint, i, STACK) + /* Attribute would suggest STACK + 16 but should be ignored: */ + LAST_ARG (alignedint, j, STACK + 8) +#endif diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-6.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-6.c new file mode 100644 index 0000000..20cbd94 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-6.c @@ -0,0 +1,36 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-6.c" + +/* The underlying struct here has alignment 8. */ +typedef struct __attribute__ ((__aligned__ (16))) + { + long x; + long y; + } overaligned; + +/* A couple of instances, at 16-byte-aligned memory locations. */ +overaligned a = { 2, 3 }; +overaligned b = { 5, 8 }; +overaligned c = { 13, 21 }; + +#include "abitest.h" +#else + ARG (int, 7, W0) + /* Natural alignment should be 8. */ + ARG (overaligned, a, X1) + ARG (int, 9, W3) + ARG (int, 11, W4) + ARG (overaligned, b, X5) + ARG (int, 15, W7) +#ifndef __AAPCS64_BIG_ENDIAN__ + ARG (int, 10, STACK) +#else + ARG (int, 10, STACK + 4) +#endif + /* Natural alignment should be 8. */ + LAST_ARG (overaligned, c, STACK + 8) +#endif diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-7.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-7.c new file mode 100644 index 0000000..6af422f --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-7.c @@ -0,0 +1,38 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-7.c" + +struct s + { + long x; + long y; + }; + +/* This still has size 16, so is still passed by value. */ +typedef __attribute__ ((__aligned__ (32))) struct s overaligned; + +/* A few structs, at 32-byte-aligned memory locations. */ +overaligned a = { 2, 3 }; +overaligned b = { 5, 8 }; +overaligned c = { 13, 21 }; + +#include "abitest.h" +#else + ARG (int, 7, W0) + /* Alignment should be 8. */ + ARG (overaligned, a, X1) + ARG (int, 9, W3) + ARG (int, 11, W4) + ARG (overaligned, b, X5) + ARG (int, 15, W7) +#ifndef __AAPCS64_BIG_ENDIAN__ + ARG (int, 10, STACK) +#else + ARG (int, 10, STACK + 4) +#endif + /* Natural alignment should be 8. */ + LAST_ARG (overaligned, c, STACK + 8) +#endif diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-8.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-8.c new file mode 100644 index 0000000..ad4dfe4 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-8.c @@ -0,0 +1,33 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-8.c" + +/* The alignment also gives this size 32, so will be passed by reference. */ +typedef struct __attribute__ ((__aligned__ (32))) + { + long x; + long y; + } overaligned; + +#define EXPECTED_STRUCT_SIZE 32 +extern void link_failure (void); +int +foo () +{ + /* Optimization gets rid of this before linking. */ + if (sizeof (overaligned) != EXPECTED_STRUCT_SIZE) + link_failure (); +} + +overaligned a = { 2, 3 }; + +#include "abitest.h" +#else + ARG (int, 7, W0) + /* Alignment should be 8. */ + PTR (overaligned, a, X1) + LAST_ARG (int, 9, W2) +#endif diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-9.c b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-9.c new file mode 100644 index 0000000..0f5fa35 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_align-9.c @@ -0,0 +1,47 @@ +/* Test AAPCS layout (alignment). */ + +/* { dg-do run { target aarch64*-*-* } } */ + +#ifndef IN_FRAMEWORK +#define TESTFILE "test_align-9.c" + +struct s + { + /* This forces the alignment and size of the struct to 16. */ + __attribute__ ((__aligned__ (16))) long x; + int y; + /* 4 bytes padding. */ + }; + +typedef struct s __attribute__ ((__aligned__ (8))) 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 }; +underaligned c = { 25, 36 }; + +#include "abitest.h" +#else + ARG (int, 3, W0) + /* Object alignment is 16, so skip X1. */ + ARG (underaligned, a, X2) + ARG (int, 5, W4) + /* Object alignment is 16, so skip X5. */ + ARG (underaligned, b, X6) +#ifndef __AAPCS64_BIG_ENDIAN__ + ARG (int, 7, STACK) +#else + ARG (int, 7, STACK + 4) +#endif + /* Natural alignment should be 16. */ + LAST_ARG (underaligned, c, STACK + 16) +#endif -- 1.9.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2][AArch64] Tests of AAPCS64 updates for alignment attribute 2016-01-15 14:18 ` [PATCH 2/2][AArch64] Tests of " Alan Lawrence @ 2016-06-07 11:09 ` James Greenhalgh 0 siblings, 0 replies; 14+ messages in thread From: James Greenhalgh @ 2016-06-07 11:09 UTC (permalink / raw) To: Alan Lawrence Cc: gcc-patches, marcus.shawcroft, richard.earnshaw, charlet, ebotcazou, nd On Fri, Jan 15, 2016 at 02:17:43PM +0000, Alan Lawrence wrote: > Here I've added both tests using the abitest.h framework(which verifies values > are passed in the correct registers as specified by the AAPCS64), and separate > tests which verify that called functions read arguments from the same locations > as they are passed. Hence, each test_align-N.c corresponds to rec_align-N.c. > > I've tried to stay consistent with the existing naming of e.g. test_10.c, > test_align-1.c, va_arg-10.c, but would be happy to change to another scheme > if preferred! (e.g. func-ret-1.c ?) > > Cheers, Alan These tests are OK. I'll commit them alongside patch 1/2 from this series tomorrow. Thanks, James > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/aapcs64/aapcs64.exp: Also execute rec_*.c > * gcc.target//aarch64/aapcs64/rec_align-5.c: New. > * gcc.target//aarch64/aapcs64/rec_align-6.c: New. > * gcc.target//aarch64/aapcs64/rec_align-7.c: New. > * gcc.target//aarch64/aapcs64/rec_align-8.c: New. > * gcc.target//aarch64/aapcs64/rec_align-9.c: New. > * gcc.target//aarch64/aapcs64/test_align-5.c: New. > * gcc.target//aarch64/aapcs64/test_align-6.c: New. > * gcc.target//aarch64/aapcs64/test_align-7.c: New. > * gcc.target//aarch64/aapcs64/test_align-8.c: New. > * gcc.target//aarch64/aapcs64/test_align-9.c: New. > * gcc.target//aarch64/aapcs64/rec_vaarg-1.c: New. > * gcc.target//aarch64/aapcs64/rec_vaarg-2.c: New. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-01-15 14:17 [PATCH 0/2][AArch64] Implement AAPCS64 updates for alignment attribute Alan Lawrence 2016-01-15 14:18 ` [PATCH 1/2][AArch64] " Alan Lawrence 2016-01-15 14:18 ` [PATCH 2/2][AArch64] Tests of " Alan Lawrence @ 2016-01-18 17:11 ` Eric Botcazou 2016-01-21 17:23 ` Alan Lawrence 2 siblings, 1 reply; 14+ messages in thread From: Eric Botcazou @ 2016-01-18 17:11 UTC (permalink / raw) To: Alan Lawrence Cc: gcc-patches, marcus.shawcroft, james.greenhalgh, richard.earnshaw, charlet > Similarly to ARM, I note that Ada is affected. Indeed, with a gcc 4.9 host > compiler, I saw a bootstrap miscompare iff including Ada; however, I was > able to bootstrap Ada successfully, if I first built a GCC including this > patch with --disable-bootstrap, and then used that as host compiler. The > best explanation I can see for this is mismatched host vs built libraries > and compiler being used together, something like Jakub's suggestion > http://gcc.gnu.org/ml/gcc-patches/2015-11/msg00338.html. I don't feel I have > the expertise for this, and am CCing the Ada maintainers in the hope they > can help. That's a bit weird though because this should have also occurred for ARM when the ABI was broken the same way if the Ada bootstrap is not entirely correct. Now, as far I know, this didn't occur for ARM during bootstrap but only during testing with make -k check. Or else could this be a parallel compilation bug? Could you post the list of files that differ? How do they differ exactly? -- Eric Botcazou ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-01-18 17:11 ` [PATCH 0/2][AArch64] Implement " Eric Botcazou @ 2016-01-21 17:23 ` Alan Lawrence 2016-01-22 17:16 ` [PATCH 1/2][AArch64] " Alan Lawrence 0 siblings, 1 reply; 14+ messages in thread From: Alan Lawrence @ 2016-01-21 17:23 UTC (permalink / raw) To: Eric Botcazou Cc: gcc-patches, marcus.shawcroft, james.greenhalgh, richard.earnshaw, charlet On 18/01/16 17:10, Eric Botcazou wrote: >> Similarly to ARM, I note that Ada is affected. Indeed, with a gcc 4.9 host >> compiler, I saw a bootstrap miscompare iff including Ada; however, I was >> able to bootstrap Ada successfully, if I first built a GCC including this >> patch with --disable-bootstrap, and then used that as host compiler. The >> best explanation I can see for this is mismatched host vs built libraries >> and compiler being used together, something like Jakub's suggestion >> http://gcc.gnu.org/ml/gcc-patches/2015-11/msg00338.html. I don't feel I have >> the expertise for this, and am CCing the Ada maintainers in the hope they >> can help. > > That's a bit weird though because this should have also occurred for ARM when > the ABI was broken the same way if the Ada bootstrap is not entirely correct. > Now, as far I know, this didn't occur for ARM during bootstrap but only during > testing with make -k check. Or else could this be a parallel compilation bug? > > Could you post the list of files that differ? How do they differ exactly? Hmmm. Well, I definitely had this failing to bootstrap once. I repeated that, to try to identify exactly what the differences were....and it succeeded even with my pre-AAPCS64-update host compiler. So, this is probably a false alarm; I'm bootstrapping again, after a rebase, to make sure... --Alan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-01-21 17:23 ` Alan Lawrence @ 2016-01-22 17:16 ` Alan Lawrence 2016-01-22 18:49 ` Eric Botcazou ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Alan Lawrence @ 2016-01-22 17:16 UTC (permalink / raw) To: gcc-patches Cc: Marcus.Shawcroft, Richard.Earnshaw, James.Greenhalgh, ebotcazou, charlet On 21/01/16 17:23, Alan Lawrence wrote: > On 18/01/16 17:10, Eric Botcazou wrote: >> >> Could you post the list of files that differ? How do they differ exactly? > > Hmmm. Well, I definitely had this failing to bootstrap once. I repeated that, to > try to identify exactly what the differences were....and it succeeded even with > my pre-AAPCS64-update host compiler. So, this is probably a false alarm; I'm > bootstrapping again, after a rebase, to make sure... > > --Alan Ok, rebased onto a more recent build, and bootstrapping with Ada posed no problems. Sorry for the noise. However, I had to drop the assert that TYPE_FIELDS was non-null because of some C++ testcases. Is this version OK for trunk? --Alan gcc/ChangeLog: * gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment): Rewrite, looking one level down for records and arrays. --- gcc/config/aarch64/aarch64.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 9142ac0..b084f83 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1925,22 +1925,23 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, static unsigned int aarch64_function_arg_alignment (machine_mode mode, const_tree type) { - unsigned int alignment; + if (!type) + return GET_MODE_ALIGNMENT (mode); + if (integer_zerop (TYPE_SIZE (type))) + return 0; - if (type) - { - if (!integer_zerop (TYPE_SIZE (type))) - { - if (TYPE_MODE (type) == mode) - alignment = TYPE_ALIGN (type); - else - alignment = GET_MODE_ALIGNMENT (mode); - } - else - alignment = 0; - } - else - alignment = GET_MODE_ALIGNMENT (mode); + gcc_assert (TYPE_MODE (type) == mode); + + if (!AGGREGATE_TYPE_P (type)) + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); + + if (TREE_CODE (type) == ARRAY_TYPE) + return TYPE_ALIGN (TREE_TYPE (type)); + + unsigned int alignment = 0; + + for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) + alignment = std::max (alignment, DECL_ALIGN (field)); return alignment; } -- 1.9.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-01-22 17:16 ` [PATCH 1/2][AArch64] " Alan Lawrence @ 2016-01-22 18:49 ` Eric Botcazou 2016-02-22 15:07 ` Alan Lawrence 2016-06-07 11:07 ` James Greenhalgh 2 siblings, 0 replies; 14+ messages in thread From: Eric Botcazou @ 2016-01-22 18:49 UTC (permalink / raw) To: Alan Lawrence Cc: gcc-patches, Marcus.Shawcroft, Richard.Earnshaw, James.Greenhalgh, charlet > Ok, rebased onto a more recent build, and bootstrapping with Ada posed no > problems. Sorry for the noise. Great, no problem, and thanks for double checking. -- Eric Botcazou ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-01-22 17:16 ` [PATCH 1/2][AArch64] " Alan Lawrence 2016-01-22 18:49 ` Eric Botcazou @ 2016-02-22 15:07 ` Alan Lawrence 2016-02-26 14:52 ` James Greenhalgh 2016-06-07 11:07 ` James Greenhalgh 2 siblings, 1 reply; 14+ messages in thread From: Alan Lawrence @ 2016-02-22 15:07 UTC (permalink / raw) To: gcc-patches; +Cc: Marcus.Shawcroft, Richard.Earnshaw, James.Greenhalgh On 22/01/16 17:16, Alan Lawrence wrote: > > On 21/01/16 17:23, Alan Lawrence wrote: >> On 18/01/16 17:10, Eric Botcazou wrote: >>> >>> Could you post the list of files that differ? How do they differ exactly? >> >> Hmmm. Well, I definitely had this failing to bootstrap once. I repeated that, to >> try to identify exactly what the differences were....and it succeeded even with >> my pre-AAPCS64-update host compiler. So, this is probably a false alarm; I'm >> bootstrapping again, after a rebase, to make sure... >> >> --Alan > > Ok, rebased onto a more recent build, and bootstrapping with Ada posed no > problems. Sorry for the noise. > > However, I had to drop the assert that TYPE_FIELDS was non-null because of some > C++ testcases. > > Is this version OK for trunk? > > --Alan > > gcc/ChangeLog: > > * gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment): > Rewrite, looking one level down for records and arrays. > --- > gcc/config/aarch64/aarch64.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 9142ac0..b084f83 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -1925,22 +1925,23 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, > static unsigned int > aarch64_function_arg_alignment (machine_mode mode, const_tree type) > { > - unsigned int alignment; > + if (!type) > + return GET_MODE_ALIGNMENT (mode); > + if (integer_zerop (TYPE_SIZE (type))) > + return 0; > > - if (type) > - { > - if (!integer_zerop (TYPE_SIZE (type))) > - { > - if (TYPE_MODE (type) == mode) > - alignment = TYPE_ALIGN (type); > - else > - alignment = GET_MODE_ALIGNMENT (mode); > - } > - else > - alignment = 0; > - } > - else > - alignment = GET_MODE_ALIGNMENT (mode); > + gcc_assert (TYPE_MODE (type) == mode); > + > + if (!AGGREGATE_TYPE_P (type)) > + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); > + > + if (TREE_CODE (type) == ARRAY_TYPE) > + return TYPE_ALIGN (TREE_TYPE (type)); > + > + unsigned int alignment = 0; > + > + for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) > + alignment = std::max (alignment, DECL_ALIGN (field)); > > return alignment; > } > Ping. If this is not appropriate for GCC6, then is it OK for Stage 1 of GCC 7? Thanks, Alan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-02-22 15:07 ` Alan Lawrence @ 2016-02-26 14:52 ` James Greenhalgh 2016-03-04 17:24 ` Alan Lawrence 0 siblings, 1 reply; 14+ messages in thread From: James Greenhalgh @ 2016-02-26 14:52 UTC (permalink / raw) To: Alan Lawrence; +Cc: gcc-patches On Mon, Feb 22, 2016 at 03:07:09PM +0000, Alan Lawrence wrote: > On 22/01/16 17:16, Alan Lawrence wrote: > > > >On 21/01/16 17:23, Alan Lawrence wrote: > >>On 18/01/16 17:10, Eric Botcazou wrote: > >>> > >>>Could you post the list of files that differ? How do they differ exactly? > >> > >>Hmmm. Well, I definitely had this failing to bootstrap once. I repeated that, to > >>try to identify exactly what the differences were....and it succeeded even with > >>my pre-AAPCS64-update host compiler. So, this is probably a false alarm; I'm > >>bootstrapping again, after a rebase, to make sure... > >> > >>--Alan > > > >Ok, rebased onto a more recent build, and bootstrapping with Ada posed no > >problems. Sorry for the noise. > > > >However, I had to drop the assert that TYPE_FIELDS was non-null because of some > >C++ testcases. > > > >Is this version OK for trunk? > > > >--Alan > > > >gcc/ChangeLog: > > > > * gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment): > > Rewrite, looking one level down for records and arrays. > >--- > > gcc/config/aarch64/aarch64.c | 31 ++++++++++++++++--------------- > > 1 file changed, 16 insertions(+), 15 deletions(-) > > > >diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > >index 9142ac0..b084f83 100644 > >--- a/gcc/config/aarch64/aarch64.c > >+++ b/gcc/config/aarch64/aarch64.c > >@@ -1925,22 +1925,23 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, > > static unsigned int > > aarch64_function_arg_alignment (machine_mode mode, const_tree type) > > { > >- unsigned int alignment; > >+ if (!type) > >+ return GET_MODE_ALIGNMENT (mode); > >+ if (integer_zerop (TYPE_SIZE (type))) > >+ return 0; > > > >- if (type) > >- { > >- if (!integer_zerop (TYPE_SIZE (type))) > >- { > >- if (TYPE_MODE (type) == mode) > >- alignment = TYPE_ALIGN (type); > >- else > >- alignment = GET_MODE_ALIGNMENT (mode); > >- } > >- else > >- alignment = 0; > >- } > >- else > >- alignment = GET_MODE_ALIGNMENT (mode); > >+ gcc_assert (TYPE_MODE (type) == mode); > >+ > >+ if (!AGGREGATE_TYPE_P (type)) > >+ return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); > >+ > >+ if (TREE_CODE (type) == ARRAY_TYPE) > >+ return TYPE_ALIGN (TREE_TYPE (type)); > >+ > >+ unsigned int alignment = 0; > >+ > >+ for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) > >+ alignment = std::max (alignment, DECL_ALIGN (field)); > > > > return alignment; > > } > > > > > Ping. > > If this is not appropriate for GCC6, then is it OK for Stage 1 of GCC 7? I think this needs to be a GCC 7 fix at this point. Additionally, I'd like to fully understand PR69841 before we take this patch. In particular, under what circumstances can the C++ front end set DECL_ALIGN of a type to be wider than we expect. Can we ever end up with 128-bit alignment of a template parameter when we were expecting 32- or 64-bit alignment, and if we can what are the implications on this patch? Thanks, James ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-02-26 14:52 ` James Greenhalgh @ 2016-03-04 17:24 ` Alan Lawrence 2016-03-11 10:18 ` Alan Lawrence 0 siblings, 1 reply; 14+ messages in thread From: Alan Lawrence @ 2016-03-04 17:24 UTC (permalink / raw) To: gcc-patches; +Cc: James Greenhalgh, Marcus Shawcroft, Richard Earnshaw On 26/02/16 14:52, James Greenhalgh wrote: >>> gcc/ChangeLog: >>> >>> * gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment): >>> Rewrite, looking one level down for records and arrays. >>> --- >>> gcc/config/aarch64/aarch64.c | 31 ++++++++++++++++--------------- >>> 1 file changed, 16 insertions(+), 15 deletions(-) >>> >>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >>> index 9142ac0..b084f83 100644 >>> --- a/gcc/config/aarch64/aarch64.c >>> +++ b/gcc/config/aarch64/aarch64.c >>> @@ -1925,22 +1925,23 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, >>> static unsigned int >>> aarch64_function_arg_alignment (machine_mode mode, const_tree type) >>> { >>> - unsigned int alignment; >>> + if (!type) >>> + return GET_MODE_ALIGNMENT (mode); >>> + if (integer_zerop (TYPE_SIZE (type))) >>> + return 0; >>> >>> - if (type) >>> - { >>> - if (!integer_zerop (TYPE_SIZE (type))) >>> - { >>> - if (TYPE_MODE (type) == mode) >>> - alignment = TYPE_ALIGN (type); >>> - else >>> - alignment = GET_MODE_ALIGNMENT (mode); >>> - } >>> - else >>> - alignment = 0; >>> - } >>> - else >>> - alignment = GET_MODE_ALIGNMENT (mode); >>> + gcc_assert (TYPE_MODE (type) == mode); >>> + >>> + if (!AGGREGATE_TYPE_P (type)) >>> + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); >>> + >>> + if (TREE_CODE (type) == ARRAY_TYPE) >>> + return TYPE_ALIGN (TREE_TYPE (type)); >>> + >>> + unsigned int alignment = 0; >>> + >>> + for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) >>> + alignment = std::max (alignment, DECL_ALIGN (field)); >>> >>> return alignment; >>> } >>> >> >> >> Ping. >> >> If this is not appropriate for GCC6, then is it OK for Stage 1 of GCC 7? > > I think this needs to be a GCC 7 fix at this point. > > Additionally, I'd like to fully understand PR69841 before we take this > patch. > > In particular, under what circumstances can the C++ front end set DECL_ALIGN > of a type to be wider than we expect. Can we ever end up with 128-bit > alignment of a template parameter when we were expecting 32- or 64-bit > alignment, and if we can what are the implications on this patch? OK, so IIUC, we *should* be able to rely on DECL_ALIGN for the AAPCS64, as PR/69841 occurred on gcc-5-branch because a C++ frontend change had not been backported. I'm not proposing to backport these AArch64 changes, hence: Ping^2. (For gcc 7 ?) Also tests https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01073.html . Thanks, Alan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-03-04 17:24 ` Alan Lawrence @ 2016-03-11 10:18 ` Alan Lawrence 0 siblings, 0 replies; 14+ messages in thread From: Alan Lawrence @ 2016-03-11 10:18 UTC (permalink / raw) To: gcc-patches; +Cc: James Greenhalgh, Marcus Shawcroft, Richard Earnshaw On 04/03/16 17:24, Alan Lawrence wrote: > On 26/02/16 14:52, James Greenhalgh wrote: > >>>> gcc/ChangeLog: >>>> >>>> * gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment): >>>> Rewrite, looking one level down for records and arrays. >>>> --- >>>> gcc/config/aarch64/aarch64.c | 31 ++++++++++++++++--------------- >>>> 1 file changed, 16 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >>>> index 9142ac0..b084f83 100644 >>>> --- a/gcc/config/aarch64/aarch64.c >>>> +++ b/gcc/config/aarch64/aarch64.c >>>> @@ -1925,22 +1925,23 @@ aarch64_vfp_is_call_candidate (cumulative_args_t >>>> pcum_v, machine_mode mode, >>>> static unsigned int >>>> aarch64_function_arg_alignment (machine_mode mode, const_tree type) >>>> { >>>> - unsigned int alignment; >>>> + if (!type) >>>> + return GET_MODE_ALIGNMENT (mode); >>>> + if (integer_zerop (TYPE_SIZE (type))) >>>> + return 0; >>>> >>>> - if (type) >>>> - { >>>> - if (!integer_zerop (TYPE_SIZE (type))) >>>> - { >>>> - if (TYPE_MODE (type) == mode) >>>> - alignment = TYPE_ALIGN (type); >>>> - else >>>> - alignment = GET_MODE_ALIGNMENT (mode); >>>> - } >>>> - else >>>> - alignment = 0; >>>> - } >>>> - else >>>> - alignment = GET_MODE_ALIGNMENT (mode); >>>> + gcc_assert (TYPE_MODE (type) == mode); >>>> + >>>> + if (!AGGREGATE_TYPE_P (type)) >>>> + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); >>>> + >>>> + if (TREE_CODE (type) == ARRAY_TYPE) >>>> + return TYPE_ALIGN (TREE_TYPE (type)); >>>> + >>>> + unsigned int alignment = 0; >>>> + >>>> + for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) >>>> + alignment = std::max (alignment, DECL_ALIGN (field)); >>>> >>>> return alignment; >>>> } >>>> >>> >>> >>> Ping. [snip] > > I'm not proposing to backport these AArch64 changes, hence: > > Ping^2. > > (For gcc 7 ?) > > Also tests https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01073.html . Ping^3. Cheers, Alan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-01-22 17:16 ` [PATCH 1/2][AArch64] " Alan Lawrence 2016-01-22 18:49 ` Eric Botcazou 2016-02-22 15:07 ` Alan Lawrence @ 2016-06-07 11:07 ` James Greenhalgh 2016-06-08 17:04 ` James Greenhalgh 2 siblings, 1 reply; 14+ messages in thread From: James Greenhalgh @ 2016-06-07 11:07 UTC (permalink / raw) To: Alan Lawrence Cc: gcc-patches, Marcus.Shawcroft, Richard.Earnshaw, ebotcazou, charlet, nd On Fri, Jan 22, 2016 at 05:16:00PM +0000, Alan Lawrence wrote: > > On 21/01/16 17:23, Alan Lawrence wrote: > > On 18/01/16 17:10, Eric Botcazou wrote: > >> > >> Could you post the list of files that differ? How do they differ exactly? > > > > Hmmm. Well, I definitely had this failing to bootstrap once. I repeated that, to > > try to identify exactly what the differences were....and it succeeded even with > > my pre-AAPCS64-update host compiler. So, this is probably a false alarm; I'm > > bootstrapping again, after a rebase, to make sure... > > > > --Alan > > Ok, rebased onto a more recent build, and bootstrapping with Ada posed no > problems. Sorry for the noise. > > However, I had to drop the assert that TYPE_FIELDS was non-null because of some > C++ testcases. > > Is this version OK for trunk? Now that we're in GCC7, this version of the patch is OK for trunk. From my reading of Richard's AAPCS update, this patch implements the rules as required. I'll give this a day for any last minute comments from Richard/Marcus, then commit this on your behalf tomorrow. Thanks, James > gcc/ChangeLog: > > * gcc/config/aarch64/aarch64.c (aarch64_function_arg_alignment): > Rewrite, looking one level down for records and arrays. > --- > gcc/config/aarch64/aarch64.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 9142ac0..b084f83 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -1925,22 +1925,23 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, > static unsigned int > aarch64_function_arg_alignment (machine_mode mode, const_tree type) > { > - unsigned int alignment; > + if (!type) > + return GET_MODE_ALIGNMENT (mode); > + if (integer_zerop (TYPE_SIZE (type))) > + return 0; > > - if (type) > - { > - if (!integer_zerop (TYPE_SIZE (type))) > - { > - if (TYPE_MODE (type) == mode) > - alignment = TYPE_ALIGN (type); > - else > - alignment = GET_MODE_ALIGNMENT (mode); > - } > - else > - alignment = 0; > - } > - else > - alignment = GET_MODE_ALIGNMENT (mode); > + gcc_assert (TYPE_MODE (type) == mode); > + > + if (!AGGREGATE_TYPE_P (type)) > + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type)); > + > + if (TREE_CODE (type) == ARRAY_TYPE) > + return TYPE_ALIGN (TREE_TYPE (type)); > + > + unsigned int alignment = 0; > + > + for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) > + alignment = std::max (alignment, DECL_ALIGN (field)); > > return alignment; > } > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2][AArch64] Implement AAPCS64 updates for alignment attribute 2016-06-07 11:07 ` James Greenhalgh @ 2016-06-08 17:04 ` James Greenhalgh 0 siblings, 0 replies; 14+ messages in thread From: James Greenhalgh @ 2016-06-08 17:04 UTC (permalink / raw) To: Alan Lawrence Cc: gcc-patches, Marcus.Shawcroft, Richard.Earnshaw, ebotcazou, charlet, nd On Tue, Jun 07, 2016 at 12:07:03PM +0100, James Greenhalgh wrote: > On Fri, Jan 22, 2016 at 05:16:00PM +0000, Alan Lawrence wrote: > > > > On 21/01/16 17:23, Alan Lawrence wrote: > > > On 18/01/16 17:10, Eric Botcazou wrote: > > >> > > >> Could you post the list of files that differ? How do they differ exactly? > > > > > > Hmmm. Well, I definitely had this failing to bootstrap once. I repeated that, to > > > try to identify exactly what the differences were....and it succeeded even with > > > my pre-AAPCS64-update host compiler. So, this is probably a false alarm; I'm > > > bootstrapping again, after a rebase, to make sure... > > > > > > --Alan > > > > Ok, rebased onto a more recent build, and bootstrapping with Ada posed no > > problems. Sorry for the noise. > > > > However, I had to drop the assert that TYPE_FIELDS was non-null because of some > > C++ testcases. > > > > Is this version OK for trunk? > > Now that we're in GCC7, this version of the patch is OK for trunk. > > From my reading of Richard's AAPCS update, this patch implements the > rules as required. > > I'll give this a day for any last minute comments from Richard/Marcus, > then commit this on your behalf tomorrow. I've now committed this on Alan's behalf as revisions r237224 (this patch) and r237225 (the tests) respectively. Thanks, James ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-06-08 17:04 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-15 14:17 [PATCH 0/2][AArch64] Implement AAPCS64 updates for alignment attribute Alan Lawrence 2016-01-15 14:18 ` [PATCH 1/2][AArch64] " Alan Lawrence 2016-01-15 14:18 ` [PATCH 2/2][AArch64] Tests of " Alan Lawrence 2016-06-07 11:09 ` James Greenhalgh 2016-01-18 17:11 ` [PATCH 0/2][AArch64] Implement " Eric Botcazou 2016-01-21 17:23 ` Alan Lawrence 2016-01-22 17:16 ` [PATCH 1/2][AArch64] " Alan Lawrence 2016-01-22 18:49 ` Eric Botcazou 2016-02-22 15:07 ` Alan Lawrence 2016-02-26 14:52 ` James Greenhalgh 2016-03-04 17:24 ` Alan Lawrence 2016-03-11 10:18 ` Alan Lawrence 2016-06-07 11:07 ` James Greenhalgh 2016-06-08 17:04 ` James Greenhalgh
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).