* [AARCH64 ABI PATCH] Change AARCH64 ABI to match AAPCS, provide -Wpsabi notes (PR target/77728)
@ 2017-04-25 10:04 Jakub Jelinek
2017-04-25 12:51 ` Richard Earnshaw (lists)
2017-04-26 16:02 ` [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 " Jakub Jelinek
0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2017-04-25 10:04 UTC (permalink / raw)
To: Richard Earnshaw, Ramana Radhakrishnan; +Cc: gcc-patches
Hi!
Similarly to the previous patch, just hopefully triggers less often,
because 128-bit alignment is more rare.
Ok for trunk/7.1 if it passes testing?
2017-04-25 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
Jakub Jelinek <jakub@redhat.com>
PR target/77728
* config/aarch64/aarch64.c (struct aarch64_fn_arg_alignment): New
type.
(aarch64_function_arg_alignment): Return aarch64_fn_arg_alignment
struct. Ignore DECL_ALIGN of decls other than FIELD_DECL for
the alignment computation, but return their maximum in warn_alignment.
(aarch64_layout_arg): Adjust aarch64_function_arg_alignment caller.
Emit a -Wpsabi note if warn_alignment is 16 bytes, but alignment
is smaller.
(aarch64_function_arg_boundary): Likewise. Simplify using MIN/MAX.
(aarch64_gimplify_va_arg_expr): Adjust aarch64_function_arg_alignment
caller.
testsuite/
* g++.dg/abi/pr77728-2.C: New test.
--- gcc/config/aarch64/aarch64.c.jj 2017-04-24 19:28:02.518970890 +0200
+++ gcc/config/aarch64/aarch64.c 2017-04-25 11:07:55.169532408 +0200
@@ -2256,33 +2256,58 @@ aarch64_vfp_is_call_candidate (cumulativ
NULL);
}
-/* Given MODE and TYPE of a function argument, return the alignment in
+struct aarch64_fn_arg_alignment
+{
+ /* Alignment for FIELD_DECLs in function arguments. */
+ unsigned int alignment;
+ /* Alignment for decls other than FIELD_DECLs in function arguments. */
+ unsigned int warn_alignment;
+};
+
+/* Given MODE and TYPE of a function argument, return a pair of alignments in
bits. The idea is to suppress any stronger alignment requested by
the user and opt for the natural alignment (specified in AAPCS64 \S 4.1).
This is a helper function for local use only. */
-static unsigned int
+static struct aarch64_fn_arg_alignment
aarch64_function_arg_alignment (machine_mode mode, const_tree type)
{
+ struct aarch64_fn_arg_alignment aa;
+ aa.alignment = 0;
+ aa.warn_alignment = 0;
+
if (!type)
- return GET_MODE_ALIGNMENT (mode);
+ {
+ aa.alignment = GET_MODE_ALIGNMENT (mode);
+ return aa;
+ }
+
if (integer_zerop (TYPE_SIZE (type)))
- return 0;
+ return aa;
gcc_assert (TYPE_MODE (type) == mode);
if (!AGGREGATE_TYPE_P (type))
- return TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
+ {
+ aa.alignment = TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
+ return aa;
+ }
if (TREE_CODE (type) == ARRAY_TYPE)
- return TYPE_ALIGN (TREE_TYPE (type));
-
- unsigned int alignment = 0;
+ {
+ aa.alignment = TYPE_ALIGN (TREE_TYPE (type));
+ return aa;
+ }
for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
- alignment = std::max (alignment, DECL_ALIGN (field));
+ {
+ if (TREE_CODE (field) == FIELD_DECL)
+ aa.alignment = std::max (aa.alignment, DECL_ALIGN (field));
+ else
+ aa.warn_alignment = std::max (aa.warn_alignment, DECL_ALIGN (field));
+ }
- return alignment;
+ return aa;
}
/* Layout a function argument according to the AAPCS64 rules. The rule
@@ -2369,24 +2394,35 @@ aarch64_layout_arg (cumulative_args_t pc
entirely general registers. */
if (allocate_ncrn && (ncrn + nregs <= NUM_ARG_REGS))
{
- unsigned int alignment = aarch64_function_arg_alignment (mode, type);
gcc_assert (nregs == 0 || nregs == 1 || nregs == 2);
/* C.8 if the argument has an alignment of 16 then the NGRN is
rounded up to the next even number. */
- if (nregs == 2 && alignment == 16 * BITS_PER_UNIT && ncrn % 2)
+ if (nregs == 2 && ncrn % 2)
{
- ++ncrn;
- gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
+ struct aarch64_fn_arg_alignment aa
+ = aarch64_function_arg_alignment (mode, type);
+
+ if (aa.warn_alignment == 16 * BITS_PER_UNIT
+ && aa.alignment < aa.warn_alignment
+ && warn_psabi
+ && currently_expanding_gimple_stmt)
+ inform (input_location,
+ "parameter passing for argument of type %qT "
+ "changed in GCC 7.1", type);
+ else if (aa.alignment == 16 * BITS_PER_UNIT)
+ {
+ ++ncrn;
+ gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
+ }
}
+
/* NREGS can be 0 when e.g. an empty structure is to be passed.
A reg is still generated for it, but the caller should be smart
enough not to use it. */
if (nregs == 0 || nregs == 1 || GET_MODE_CLASS (mode) == MODE_INT)
- {
- pcum->aapcs_reg = gen_rtx_REG (mode, R0_REGNUM + ncrn);
- }
+ pcum->aapcs_reg = gen_rtx_REG (mode, R0_REGNUM + ncrn);
else
{
rtx par;
@@ -2414,7 +2450,10 @@ aarch64_layout_arg (cumulative_args_t pc
this argument and align the total size if necessary. */
on_stack:
pcum->aapcs_stack_words = size / UNITS_PER_WORD;
- if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
+ struct aarch64_fn_arg_alignment aa
+ = aarch64_function_arg_alignment (mode, type);
+
+ if (aa.alignment == 16 * BITS_PER_UNIT)
pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size,
16 / UNITS_PER_WORD);
return;
@@ -2505,13 +2544,17 @@ aarch64_function_arg_regno_p (unsigned r
static unsigned int
aarch64_function_arg_boundary (machine_mode mode, const_tree type)
{
- unsigned int alignment = aarch64_function_arg_alignment (mode, type);
+ struct aarch64_fn_arg_alignment aa
+ = aarch64_function_arg_alignment (mode, type);
+ aa.alignment = MIN (MAX (aa.alignment, PARM_BOUNDARY), STACK_BOUNDARY);
+ aa.warn_alignment
+ = MIN (MAX (aa.warn_alignment, PARM_BOUNDARY), STACK_BOUNDARY);
+
+ if (warn_psabi && aa.warn_alignment > aa.alignment)
+ inform (input_location, "parameter passing for argument of type %qT "
+ "changed in GCC 7.1", type);
- if (alignment < PARM_BOUNDARY)
- alignment = PARM_BOUNDARY;
- if (alignment > STACK_BOUNDARY)
- alignment = STACK_BOUNDARY;
- return alignment;
+ return aa.alignment;
}
/* For use by FUNCTION_ARG_PADDING (MODE, TYPE).
@@ -10211,7 +10254,9 @@ aarch64_gimplify_va_arg_expr (tree valis
stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist),
f_stack, NULL_TREE);
size = int_size_in_bytes (type);
- align = aarch64_function_arg_alignment (mode, type) / BITS_PER_UNIT;
+ struct aarch64_fn_arg_alignment aa
+ = aarch64_function_arg_alignment (mode, type);
+ align = aa.alignment / BITS_PER_UNIT;
dw_align = false;
adjust = 0;
--- gcc/testsuite/g++.dg/abi/pr77728-2.C.jj 2017-04-25 10:54:08.396372117 +0200
+++ gcc/testsuite/g++.dg/abi/pr77728-2.C 2017-04-25 10:52:37.000000000 +0200
@@ -0,0 +1,171 @@
+// { dg-do compile { target { { aarch64-*-* } && c++11 } } }
+// { dg-options "-Wpsabi" }
+
+#include <stdarg.h>
+
+template <int N>
+struct alignas (16) A { char p[16]; };
+
+A<0> v;
+
+template <int N>
+struct B
+{
+ typedef A<N> T;
+ int i, j, k, l;
+};
+
+struct C : public B<0> {};
+struct D {};
+struct E : public D, C {};
+struct F : public B<1> {};
+struct G : public F { static int y alignas (16); };
+struct H : public G {};
+struct I : public D { int z alignas (16); };
+struct J : public D { static int z alignas (16); int i, j, k, l; };
+
+template <int N>
+struct K : public D { typedef A<N> T; int i, j; };
+
+struct L { static int h alignas (16); int i, j, k, l; };
+
+int
+fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+{
+ return a + b.i;
+}
+
+int
+fn2 (int a, B<1> b)
+{
+ return a + b.i;
+}
+
+int
+fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+{
+ return a + b.i;
+}
+
+int
+fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...)
+// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn5 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<1> n, ...)
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn6 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, C n, ...)
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn7 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, E n, ...)
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn8 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, H n, ...)
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn9 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, I n, ...)
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...)
+// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...)
+// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+int
+fn12 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<2> n, ...)
+{
+ va_list ap;
+ va_start (ap, n);
+ int x = va_arg (ap, int);
+ va_end (ap);
+ return x;
+}
+
+void
+test ()
+{
+ static B<0> b0;
+ static B<1> b1;
+ static L l;
+ static C c;
+ static E e;
+ static H h;
+ static I i;
+ static J j;
+ static K<0> k0;
+ static K<2> k2;
+ fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+ fn2 (1, b1);
+ fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+ fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4);
+ // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
+ fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4);
+ fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4);
+ fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4);
+ fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4);
+ fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4);
+ fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4);
+ // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
+ fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4);
+ // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
+ fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4);
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [AARCH64 ABI PATCH] Change AARCH64 ABI to match AAPCS, provide -Wpsabi notes (PR target/77728)
2017-04-25 10:04 [AARCH64 ABI PATCH] Change AARCH64 ABI to match AAPCS, provide -Wpsabi notes (PR target/77728) Jakub Jelinek
@ 2017-04-25 12:51 ` Richard Earnshaw (lists)
2017-04-25 12:58 ` Jakub Jelinek
2017-04-26 16:02 ` [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 " Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2017-04-25 12:51 UTC (permalink / raw)
To: Jakub Jelinek, Ramana Radhakrishnan; +Cc: gcc-patches
On 25/04/17 11:01, Jakub Jelinek wrote:
> Hi!
>
> Similarly to the previous patch, just hopefully triggers less often,
> because 128-bit alignment is more rare.
>
> Ok for trunk/7.1 if it passes testing?
>
> 2017-04-25 Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
> Jakub Jelinek <jakub@redhat.com>
>
> PR target/77728
> * config/aarch64/aarch64.c (struct aarch64_fn_arg_alignment): New
> type.
> (aarch64_function_arg_alignment): Return aarch64_fn_arg_alignment
> struct. Ignore DECL_ALIGN of decls other than FIELD_DECL for
> the alignment computation, but return their maximum in warn_alignment.
> (aarch64_layout_arg): Adjust aarch64_function_arg_alignment caller.
> Emit a -Wpsabi note if warn_alignment is 16 bytes, but alignment
> is smaller.
> (aarch64_function_arg_boundary): Likewise. Simplify using MIN/MAX.
> (aarch64_gimplify_va_arg_expr): Adjust aarch64_function_arg_alignment
> caller.
> testsuite/
> * g++.dg/abi/pr77728-2.C: New test.
>
> --- gcc/config/aarch64/aarch64.c.jj 2017-04-24 19:28:02.518970890 +0200
> +++ gcc/config/aarch64/aarch64.c 2017-04-25 11:07:55.169532408 +0200
> @@ -2256,33 +2256,58 @@ aarch64_vfp_is_call_candidate (cumulativ
> NULL);
> }
>
> -/* Given MODE and TYPE of a function argument, return the alignment in
> +struct aarch64_fn_arg_alignment
> +{
> + /* Alignment for FIELD_DECLs in function arguments. */
> + unsigned int alignment;
> + /* Alignment for decls other than FIELD_DECLs in function arguments. */
> + unsigned int warn_alignment;
> +};
> +
> +/* Given MODE and TYPE of a function argument, return a pair of alignments in
> bits. The idea is to suppress any stronger alignment requested by
> the user and opt for the natural alignment (specified in AAPCS64 \S 4.1).
> This is a helper function for local use only. */
>
> -static unsigned int
> +static struct aarch64_fn_arg_alignment
> aarch64_function_arg_alignment (machine_mode mode, const_tree type)
> {
> + struct aarch64_fn_arg_alignment aa;
> + aa.alignment = 0;
> + aa.warn_alignment = 0;
> +
> if (!type)
> - return GET_MODE_ALIGNMENT (mode);
> + {
> + aa.alignment = GET_MODE_ALIGNMENT (mode);
> + return aa;
> + }
> +
> if (integer_zerop (TYPE_SIZE (type)))
> - return 0;
> + return aa;
>
> gcc_assert (TYPE_MODE (type) == mode);
>
> if (!AGGREGATE_TYPE_P (type))
> - return TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
> + {
> + aa.alignment = TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
> + return aa;
> + }
>
> if (TREE_CODE (type) == ARRAY_TYPE)
> - return TYPE_ALIGN (TREE_TYPE (type));
> -
> - unsigned int alignment = 0;
> + {
> + aa.alignment = TYPE_ALIGN (TREE_TYPE (type));
> + return aa;
> + }
>
> for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> - alignment = std::max (alignment, DECL_ALIGN (field));
> + {
> + if (TREE_CODE (field) == FIELD_DECL)
> + aa.alignment = std::max (aa.alignment, DECL_ALIGN (field));
> + else
> + aa.warn_alignment = std::max (aa.warn_alignment, DECL_ALIGN (field));
> + }
>
> - return alignment;
> + return aa;
> }
>
> /* Layout a function argument according to the AAPCS64 rules. The rule
> @@ -2369,24 +2394,35 @@ aarch64_layout_arg (cumulative_args_t pc
> entirely general registers. */
> if (allocate_ncrn && (ncrn + nregs <= NUM_ARG_REGS))
> {
> - unsigned int alignment = aarch64_function_arg_alignment (mode, type);
>
> gcc_assert (nregs == 0 || nregs == 1 || nregs == 2);
>
> /* C.8 if the argument has an alignment of 16 then the NGRN is
> rounded up to the next even number. */
> - if (nregs == 2 && alignment == 16 * BITS_PER_UNIT && ncrn % 2)
> + if (nregs == 2 && ncrn % 2)
> {
> - ++ncrn;
> - gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
> + struct aarch64_fn_arg_alignment aa
> + = aarch64_function_arg_alignment (mode, type);
> +
> + if (aa.warn_alignment == 16 * BITS_PER_UNIT
I was caught out (again) when reviewing this as to why the test was for
exactly 16 bytes and not >= 16. I'd forgotten that anything with higher
alignment would have a size larger than 16 and thus be passed by
reference to a copy. Could you please add a comment to that effect as
it's not obvious at this point.
OK with that change if testing passes.
> + && aa.alignment < aa.warn_alignment
> + && warn_psabi
> + && currently_expanding_gimple_stmt)
> + inform (input_location,
> + "parameter passing for argument of type %qT "
> + "changed in GCC 7.1", type);
> + else if (aa.alignment == 16 * BITS_PER_UNIT)
> + {
> + ++ncrn;
> + gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
> + }
> }
> +
> /* NREGS can be 0 when e.g. an empty structure is to be passed.
> A reg is still generated for it, but the caller should be smart
> enough not to use it. */
> if (nregs == 0 || nregs == 1 || GET_MODE_CLASS (mode) == MODE_INT)
> - {
> - pcum->aapcs_reg = gen_rtx_REG (mode, R0_REGNUM + ncrn);
> - }
> + pcum->aapcs_reg = gen_rtx_REG (mode, R0_REGNUM + ncrn);
> else
> {
> rtx par;
> @@ -2414,7 +2450,10 @@ aarch64_layout_arg (cumulative_args_t pc
> this argument and align the total size if necessary. */
> on_stack:
> pcum->aapcs_stack_words = size / UNITS_PER_WORD;
> - if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
> + struct aarch64_fn_arg_alignment aa
> + = aarch64_function_arg_alignment (mode, type);
> +
> + if (aa.alignment == 16 * BITS_PER_UNIT)
> pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size,
> 16 / UNITS_PER_WORD);
> return;
> @@ -2505,13 +2544,17 @@ aarch64_function_arg_regno_p (unsigned r
> static unsigned int
> aarch64_function_arg_boundary (machine_mode mode, const_tree type)
> {
> - unsigned int alignment = aarch64_function_arg_alignment (mode, type);
> + struct aarch64_fn_arg_alignment aa
> + = aarch64_function_arg_alignment (mode, type);
> + aa.alignment = MIN (MAX (aa.alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> + aa.warn_alignment
> + = MIN (MAX (aa.warn_alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> +
> + if (warn_psabi && aa.warn_alignment > aa.alignment)
> + inform (input_location, "parameter passing for argument of type %qT "
> + "changed in GCC 7.1", type);
>
> - if (alignment < PARM_BOUNDARY)
> - alignment = PARM_BOUNDARY;
> - if (alignment > STACK_BOUNDARY)
> - alignment = STACK_BOUNDARY;
> - return alignment;
> + return aa.alignment;
> }
>
> /* For use by FUNCTION_ARG_PADDING (MODE, TYPE).
> @@ -10211,7 +10254,9 @@ aarch64_gimplify_va_arg_expr (tree valis
> stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist),
> f_stack, NULL_TREE);
> size = int_size_in_bytes (type);
> - align = aarch64_function_arg_alignment (mode, type) / BITS_PER_UNIT;
> + struct aarch64_fn_arg_alignment aa
> + = aarch64_function_arg_alignment (mode, type);
> + align = aa.alignment / BITS_PER_UNIT;
>
> dw_align = false;
> adjust = 0;
> --- gcc/testsuite/g++.dg/abi/pr77728-2.C.jj 2017-04-25 10:54:08.396372117 +0200
> +++ gcc/testsuite/g++.dg/abi/pr77728-2.C 2017-04-25 10:52:37.000000000 +0200
> @@ -0,0 +1,171 @@
> +// { dg-do compile { target { { aarch64-*-* } && c++11 } } }
> +// { dg-options "-Wpsabi" }
> +
> +#include <stdarg.h>
> +
> +template <int N>
> +struct alignas (16) A { char p[16]; };
> +
> +A<0> v;
> +
> +template <int N>
> +struct B
> +{
> + typedef A<N> T;
> + int i, j, k, l;
> +};
> +
> +struct C : public B<0> {};
> +struct D {};
> +struct E : public D, C {};
> +struct F : public B<1> {};
> +struct G : public F { static int y alignas (16); };
> +struct H : public G {};
> +struct I : public D { int z alignas (16); };
> +struct J : public D { static int z alignas (16); int i, j, k, l; };
> +
> +template <int N>
> +struct K : public D { typedef A<N> T; int i, j; };
> +
> +struct L { static int h alignas (16); int i, j, k, l; };
> +
> +int
> +fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> +{
> + return a + b.i;
> +}
> +
> +int
> +fn2 (int a, B<1> b)
> +{
> + return a + b.i;
> +}
> +
> +int
> +fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> +{
> + return a + b.i;
> +}
> +
> +int
> +fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...)
> +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn5 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<1> n, ...)
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn6 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, C n, ...)
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn7 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, E n, ...)
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn8 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, H n, ...)
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn9 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, I n, ...)
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...)
> +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...)
> +// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +int
> +fn12 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<2> n, ...)
> +{
> + va_list ap;
> + va_start (ap, n);
> + int x = va_arg (ap, int);
> + va_end (ap);
> + return x;
> +}
> +
> +void
> +test ()
> +{
> + static B<0> b0;
> + static B<1> b1;
> + static L l;
> + static C c;
> + static E e;
> + static H h;
> + static I i;
> + static J j;
> + static K<0> k0;
> + static K<2> k2;
> + fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> + fn2 (1, b1);
> + fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> + fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4);
> + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> + fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4);
> + fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4);
> + fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4);
> + fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4);
> + fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4);
> + fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4);
> + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> + fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4);
> + // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> + fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4);
> +}
>
> Jakub
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [AARCH64 ABI PATCH] Change AARCH64 ABI to match AAPCS, provide -Wpsabi notes (PR target/77728)
2017-04-25 12:51 ` Richard Earnshaw (lists)
@ 2017-04-25 12:58 ` Jakub Jelinek
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2017-04-25 12:58 UTC (permalink / raw)
To: Richard Earnshaw (lists); +Cc: Ramana Radhakrishnan, gcc-patches
On Tue, Apr 25, 2017 at 01:29:06PM +0100, Richard Earnshaw (lists) wrote:
> > /* C.8 if the argument has an alignment of 16 then the NGRN is
> > rounded up to the next even number. */
> > - if (nregs == 2 && alignment == 16 * BITS_PER_UNIT && ncrn % 2)
> > + if (nregs == 2 && ncrn % 2)
> > {
> > - ++ncrn;
> > - gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
> > + struct aarch64_fn_arg_alignment aa
> > + = aarch64_function_arg_alignment (mode, type);
> > +
> > + if (aa.warn_alignment == 16 * BITS_PER_UNIT
>
> I was caught out (again) when reviewing this as to why the test was for
> exactly 16 bytes and not >= 16. I'd forgotten that anything with higher
> alignment would have a size larger than 16 and thus be passed by
I admit I didn't know why it has been written that way, I was just trying
to write something that would do the same as before if ignoring the
non-FIELD_DECL alignments and warn for the cases that changed ABI;
as there was originally alignment == 16 * BITS_PER_UNIT, I kept using that.
For structs/classes indeed I think alignment of any fields > 16
bytes implies struct size of > 16. The ARRAY_TYPE case is for some
languages I'm not familiar with, in C/C++/Fortran that doesn't happen, so I
can't guess what those languages can do. And otherwise it uses mode
alignment or alignment of main variant of scalar/vector/complex types and
I believe those should never be over-aligned.
> reference to a copy. Could you please add a comment to that effect as
> it's not obvious at this point.
Ok.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 (PR target/77728)
2017-04-25 10:04 [AARCH64 ABI PATCH] Change AARCH64 ABI to match AAPCS, provide -Wpsabi notes (PR target/77728) Jakub Jelinek
2017-04-25 12:51 ` Richard Earnshaw (lists)
@ 2017-04-26 16:02 ` Jakub Jelinek
2017-04-26 16:34 ` Richard Earnshaw (lists)
2017-04-27 1:45 ` Richard Earnshaw (lists)
1 sibling, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2017-04-26 16:02 UTC (permalink / raw)
To: Richard Earnshaw, Ramana Radhakrishnan; +Cc: gcc-patches
Hi!
As discussed on IRC, apparently the -Wpsabi warning is not appropriate
for aarch64, only for ARM, because only ARM had the almost like new AAPCS,
except for static data members and typedefs (and sometimes
self-inconsistent) ABI, while on AArch64 it only changed in r237224
for GCC 7.
Thus, the following patch reverts the -Wpsabi related changes and keeps just
the ABI fix and some of the cleanups.
Ok for trunk and 7.1 if it passes bootstrap/regtest? Could somebody from ARM
bootstrap/regtest it? I can start a distro rpm build with that patch, but
that will likely be slower.
2017-04-26 Jakub Jelinek <jakub@redhat.com>
PR target/77728
* config/aarch64/aarch64.c (struct aarch64_fn_arg_alignment): Remove.
(aarch64_function_arg_alignment): Return unsigned int again, but still
ignore TYPE_FIELDS chain decls other than FIELD_DECLs.
(aarch64_layout_arg): Adjust aarch64_function_arg_alignment caller.
Don't emit -Wpsabi note.
(aarch64_function_arg_boundary): Likewise.
(aarch64_gimplify_va_arg_expr): Adjust aarch64_function_arg_alignment
caller.
testsuite/
* g++.dg/abi/pr77728-2.C: Don't expect -Wpsabi notes.
--- gcc/config/aarch64/aarch64.c.jj 2017-04-25 15:55:38.000000000 +0200
+++ gcc/config/aarch64/aarch64.c 2017-04-26 15:59:09.224554190 +0200
@@ -2256,58 +2256,34 @@ aarch64_vfp_is_call_candidate (cumulativ
NULL);
}
-struct aarch64_fn_arg_alignment
-{
- /* Alignment for FIELD_DECLs in function arguments. */
- unsigned int alignment;
- /* Alignment for decls other than FIELD_DECLs in function arguments. */
- unsigned int warn_alignment;
-};
-
-/* Given MODE and TYPE of a function argument, return a pair of alignments in
+/* Given MODE and TYPE of a function argument, return the alignment in
bits. The idea is to suppress any stronger alignment requested by
the user and opt for the natural alignment (specified in AAPCS64 \S 4.1).
This is a helper function for local use only. */
-static struct aarch64_fn_arg_alignment
+static unsigned int
aarch64_function_arg_alignment (machine_mode mode, const_tree type)
{
- struct aarch64_fn_arg_alignment aa;
- aa.alignment = 0;
- aa.warn_alignment = 0;
-
if (!type)
- {
- aa.alignment = GET_MODE_ALIGNMENT (mode);
- return aa;
- }
+ return GET_MODE_ALIGNMENT (mode);
if (integer_zerop (TYPE_SIZE (type)))
- return aa;
+ return 0;
gcc_assert (TYPE_MODE (type) == mode);
if (!AGGREGATE_TYPE_P (type))
- {
- aa.alignment = TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
- return aa;
- }
+ return TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
if (TREE_CODE (type) == ARRAY_TYPE)
- {
- aa.alignment = TYPE_ALIGN (TREE_TYPE (type));
- return aa;
- }
+ return TYPE_ALIGN (TREE_TYPE (type));
+ unsigned int alignment = 0;
for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
- {
- if (TREE_CODE (field) == FIELD_DECL)
- aa.alignment = std::max (aa.alignment, DECL_ALIGN (field));
- else
- aa.warn_alignment = std::max (aa.warn_alignment, DECL_ALIGN (field));
- }
+ if (TREE_CODE (field) == FIELD_DECL)
+ alignment = std::max (alignment, DECL_ALIGN (field));
- return aa;
+ return alignment;
}
/* Layout a function argument according to the AAPCS64 rules. The rule
@@ -2399,27 +2375,16 @@ aarch64_layout_arg (cumulative_args_t pc
/* C.8 if the argument has an alignment of 16 then the NGRN is
rounded up to the next even number. */
- if (nregs == 2 && ncrn % 2)
- {
- struct aarch64_fn_arg_alignment aa
- = aarch64_function_arg_alignment (mode, type);
-
+ if (nregs == 2
+ && ncrn % 2
/* The == 16 * BITS_PER_UNIT instead of >= 16 * BITS_PER_UNIT
- comparisons are there because for > 16 * BITS_PER_UNIT
+ comparison is there because for > 16 * BITS_PER_UNIT
alignment nregs should be > 2 and therefore it should be
passed by reference rather than value. */
- if (aa.warn_alignment == 16 * BITS_PER_UNIT
- && aa.alignment < aa.warn_alignment
- && warn_psabi
- && currently_expanding_gimple_stmt)
- inform (input_location,
- "parameter passing for argument of type %qT "
- "changed in GCC 7.1", type);
- else if (aa.alignment == 16 * BITS_PER_UNIT)
- {
- ++ncrn;
- gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
- }
+ && aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
+ {
+ ++ncrn;
+ gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
}
/* NREGS can be 0 when e.g. an empty structure is to be passed.
@@ -2454,10 +2419,8 @@ aarch64_layout_arg (cumulative_args_t pc
this argument and align the total size if necessary. */
on_stack:
pcum->aapcs_stack_words = size / UNITS_PER_WORD;
- struct aarch64_fn_arg_alignment aa
- = aarch64_function_arg_alignment (mode, type);
- if (aa.alignment == 16 * BITS_PER_UNIT)
+ if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size,
16 / UNITS_PER_WORD);
return;
@@ -2548,17 +2511,8 @@ aarch64_function_arg_regno_p (unsigned r
static unsigned int
aarch64_function_arg_boundary (machine_mode mode, const_tree type)
{
- struct aarch64_fn_arg_alignment aa
- = aarch64_function_arg_alignment (mode, type);
- aa.alignment = MIN (MAX (aa.alignment, PARM_BOUNDARY), STACK_BOUNDARY);
- aa.warn_alignment
- = MIN (MAX (aa.warn_alignment, PARM_BOUNDARY), STACK_BOUNDARY);
-
- if (warn_psabi && aa.warn_alignment > aa.alignment)
- inform (input_location, "parameter passing for argument of type %qT "
- "changed in GCC 7.1", type);
-
- return aa.alignment;
+ unsigned int alignment = aarch64_function_arg_alignment (mode, type);
+ return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
}
/* For use by FUNCTION_ARG_PADDING (MODE, TYPE).
@@ -10258,9 +10212,7 @@ aarch64_gimplify_va_arg_expr (tree valis
stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist),
f_stack, NULL_TREE);
size = int_size_in_bytes (type);
- struct aarch64_fn_arg_alignment aa
- = aarch64_function_arg_alignment (mode, type);
- align = aa.alignment / BITS_PER_UNIT;
+ align = aarch64_function_arg_alignment (mode, type) / BITS_PER_UNIT;
dw_align = false;
adjust = 0;
--- gcc/testsuite/g++.dg/abi/pr77728-2.C.jj 2017-04-25 15:55:38.000000000 +0200
+++ gcc/testsuite/g++.dg/abi/pr77728-2.C 2017-04-26 16:01:18.414878207 +0200
@@ -1,5 +1,5 @@
// { dg-do compile { target { { aarch64-*-* } && c++11 } } }
-// { dg-options "-Wpsabi" }
+// { dg-options "" }
#include <stdarg.h>
@@ -30,7 +30,7 @@ struct K : public D { typedef A<N> T; in
struct L { static int h alignas (16); int i, j, k, l; };
int
-fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+fn1 (int a, B<0> b)
{
return a + b.i;
}
@@ -42,14 +42,13 @@ fn2 (int a, B<1> b)
}
int
-fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+fn3 (int a, L b)
{
return a + b.i;
}
int
fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...)
-// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
{
va_list ap;
va_start (ap, n);
@@ -110,7 +109,6 @@ fn9 (int a, int b, int c, int d, int e,
int
fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...)
-// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
{
va_list ap;
va_start (ap, n);
@@ -121,7 +119,6 @@ fn10 (int a, int b, int c, int d, int e,
int
fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...)
-// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
{
va_list ap;
va_start (ap, n);
@@ -153,19 +150,16 @@ test ()
static J j;
static K<0> k0;
static K<2> k2;
- fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+ fn1 (1, b0);
fn2 (1, b1);
- fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
+ fn3 (1, l);
fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4);
- // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4);
fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4);
fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4);
fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4);
fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4);
fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4);
- // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4);
- // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4);
}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 (PR target/77728)
2017-04-26 16:02 ` [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 " Jakub Jelinek
@ 2017-04-26 16:34 ` Richard Earnshaw (lists)
2017-04-27 1:45 ` Richard Earnshaw (lists)
1 sibling, 0 replies; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2017-04-26 16:34 UTC (permalink / raw)
To: Jakub Jelinek, Ramana Radhakrishnan; +Cc: gcc-patches
On 26/04/17 15:29, Jakub Jelinek wrote:
> Hi!
>
> As discussed on IRC, apparently the -Wpsabi warning is not appropriate
> for aarch64, only for ARM, because only ARM had the almost like new AAPCS,
> except for static data members and typedefs (and sometimes
> self-inconsistent) ABI, while on AArch64 it only changed in r237224
> for GCC 7.
>
> Thus, the following patch reverts the -Wpsabi related changes and keeps just
> the ABI fix and some of the cleanups.
>
> Ok for trunk and 7.1 if it passes bootstrap/regtest? Could somebody from ARM
> bootstrap/regtest it? I can start a distro rpm build with that patch, but
> that will likely be slower.
I've started a bootstrap, but it will probably be tomorrow before I can
report results.
>
> 2017-04-26 Jakub Jelinek <jakub@redhat.com>
>
> PR target/77728
> * config/aarch64/aarch64.c (struct aarch64_fn_arg_alignment): Remove.
> (aarch64_function_arg_alignment): Return unsigned int again, but still
> ignore TYPE_FIELDS chain decls other than FIELD_DECLs.
> (aarch64_layout_arg): Adjust aarch64_function_arg_alignment caller.
> Don't emit -Wpsabi note.
> (aarch64_function_arg_boundary): Likewise.
> (aarch64_gimplify_va_arg_expr): Adjust aarch64_function_arg_alignment
> caller.
> testsuite/
> * g++.dg/abi/pr77728-2.C: Don't expect -Wpsabi notes.
>
> --- gcc/config/aarch64/aarch64.c.jj 2017-04-25 15:55:38.000000000 +0200
> +++ gcc/config/aarch64/aarch64.c 2017-04-26 15:59:09.224554190 +0200
> @@ -2256,58 +2256,34 @@ aarch64_vfp_is_call_candidate (cumulativ
> NULL);
> }
>
> -struct aarch64_fn_arg_alignment
> -{
> - /* Alignment for FIELD_DECLs in function arguments. */
> - unsigned int alignment;
> - /* Alignment for decls other than FIELD_DECLs in function arguments. */
> - unsigned int warn_alignment;
> -};
> -
> -/* Given MODE and TYPE of a function argument, return a pair of alignments in
> +/* Given MODE and TYPE of a function argument, return the alignment in
> bits. The idea is to suppress any stronger alignment requested by
> the user and opt for the natural alignment (specified in AAPCS64 \S 4.1).
> This is a helper function for local use only. */
>
> -static struct aarch64_fn_arg_alignment
> +static unsigned int
> aarch64_function_arg_alignment (machine_mode mode, const_tree type)
> {
> - struct aarch64_fn_arg_alignment aa;
> - aa.alignment = 0;
> - aa.warn_alignment = 0;
> -
> if (!type)
> - {
> - aa.alignment = GET_MODE_ALIGNMENT (mode);
> - return aa;
> - }
> + return GET_MODE_ALIGNMENT (mode);
>
> if (integer_zerop (TYPE_SIZE (type)))
> - return aa;
> + return 0;
>
> gcc_assert (TYPE_MODE (type) == mode);
>
> if (!AGGREGATE_TYPE_P (type))
> - {
> - aa.alignment = TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
> - return aa;
> - }
> + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
>
> if (TREE_CODE (type) == ARRAY_TYPE)
> - {
> - aa.alignment = TYPE_ALIGN (TREE_TYPE (type));
> - return aa;
> - }
> + return TYPE_ALIGN (TREE_TYPE (type));
>
> + unsigned int alignment = 0;
> for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> - {
> - if (TREE_CODE (field) == FIELD_DECL)
> - aa.alignment = std::max (aa.alignment, DECL_ALIGN (field));
> - else
> - aa.warn_alignment = std::max (aa.warn_alignment, DECL_ALIGN (field));
> - }
> + if (TREE_CODE (field) == FIELD_DECL)
> + alignment = std::max (alignment, DECL_ALIGN (field));
>
> - return aa;
> + return alignment;
> }
>
> /* Layout a function argument according to the AAPCS64 rules. The rule
> @@ -2399,27 +2375,16 @@ aarch64_layout_arg (cumulative_args_t pc
>
> /* C.8 if the argument has an alignment of 16 then the NGRN is
> rounded up to the next even number. */
> - if (nregs == 2 && ncrn % 2)
> - {
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
> -
> + if (nregs == 2
> + && ncrn % 2
> /* The == 16 * BITS_PER_UNIT instead of >= 16 * BITS_PER_UNIT
> - comparisons are there because for > 16 * BITS_PER_UNIT
> + comparison is there because for > 16 * BITS_PER_UNIT
> alignment nregs should be > 2 and therefore it should be
> passed by reference rather than value. */
> - if (aa.warn_alignment == 16 * BITS_PER_UNIT
> - && aa.alignment < aa.warn_alignment
> - && warn_psabi
> - && currently_expanding_gimple_stmt)
> - inform (input_location,
> - "parameter passing for argument of type %qT "
> - "changed in GCC 7.1", type);
> - else if (aa.alignment == 16 * BITS_PER_UNIT)
> - {
> - ++ncrn;
> - gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
> - }
> + && aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
> + {
> + ++ncrn;
> + gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
> }
>
> /* NREGS can be 0 when e.g. an empty structure is to be passed.
> @@ -2454,10 +2419,8 @@ aarch64_layout_arg (cumulative_args_t pc
> this argument and align the total size if necessary. */
> on_stack:
> pcum->aapcs_stack_words = size / UNITS_PER_WORD;
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
>
> - if (aa.alignment == 16 * BITS_PER_UNIT)
> + if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
> pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size,
> 16 / UNITS_PER_WORD);
> return;
> @@ -2548,17 +2511,8 @@ aarch64_function_arg_regno_p (unsigned r
> static unsigned int
> aarch64_function_arg_boundary (machine_mode mode, const_tree type)
> {
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
> - aa.alignment = MIN (MAX (aa.alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> - aa.warn_alignment
> - = MIN (MAX (aa.warn_alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> -
> - if (warn_psabi && aa.warn_alignment > aa.alignment)
> - inform (input_location, "parameter passing for argument of type %qT "
> - "changed in GCC 7.1", type);
> -
> - return aa.alignment;
> + unsigned int alignment = aarch64_function_arg_alignment (mode, type);
> + return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> }
>
> /* For use by FUNCTION_ARG_PADDING (MODE, TYPE).
> @@ -10258,9 +10212,7 @@ aarch64_gimplify_va_arg_expr (tree valis
> stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist),
> f_stack, NULL_TREE);
> size = int_size_in_bytes (type);
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
> - align = aa.alignment / BITS_PER_UNIT;
> + align = aarch64_function_arg_alignment (mode, type) / BITS_PER_UNIT;
>
> dw_align = false;
> adjust = 0;
> --- gcc/testsuite/g++.dg/abi/pr77728-2.C.jj 2017-04-25 15:55:38.000000000 +0200
> +++ gcc/testsuite/g++.dg/abi/pr77728-2.C 2017-04-26 16:01:18.414878207 +0200
> @@ -1,5 +1,5 @@
> // { dg-do compile { target { { aarch64-*-* } && c++11 } } }
> -// { dg-options "-Wpsabi" }
> +// { dg-options "" }
>
> #include <stdarg.h>
>
> @@ -30,7 +30,7 @@ struct K : public D { typedef A<N> T; in
> struct L { static int h alignas (16); int i, j, k, l; };
>
> int
> -fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> +fn1 (int a, B<0> b)
> {
> return a + b.i;
> }
> @@ -42,14 +42,13 @@ fn2 (int a, B<1> b)
> }
>
> int
> -fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> +fn3 (int a, L b)
> {
> return a + b.i;
> }
>
> int
> fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...)
> -// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> {
> va_list ap;
> va_start (ap, n);
> @@ -110,7 +109,6 @@ fn9 (int a, int b, int c, int d, int e,
>
> int
> fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...)
> -// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> {
> va_list ap;
> va_start (ap, n);
> @@ -121,7 +119,6 @@ fn10 (int a, int b, int c, int d, int e,
>
> int
> fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...)
> -// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> {
> va_list ap;
> va_start (ap, n);
> @@ -153,19 +150,16 @@ test ()
> static J j;
> static K<0> k0;
> static K<2> k2;
> - fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> + fn1 (1, b0);
> fn2 (1, b1);
> - fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> + fn3 (1, l);
> fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4);
> - // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4);
> fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4);
> fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4);
> fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4);
> fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4);
> fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4);
> - // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4);
> - // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4);
> }
>
> Jakub
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 (PR target/77728)
2017-04-26 16:02 ` [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 " Jakub Jelinek
2017-04-26 16:34 ` Richard Earnshaw (lists)
@ 2017-04-27 1:45 ` Richard Earnshaw (lists)
1 sibling, 0 replies; 6+ messages in thread
From: Richard Earnshaw (lists) @ 2017-04-27 1:45 UTC (permalink / raw)
To: Jakub Jelinek, Ramana Radhakrishnan; +Cc: gcc-patches
On 26/04/17 15:29, Jakub Jelinek wrote:
> Hi!
>
> As discussed on IRC, apparently the -Wpsabi warning is not appropriate
> for aarch64, only for ARM, because only ARM had the almost like new AAPCS,
> except for static data members and typedefs (and sometimes
> self-inconsistent) ABI, while on AArch64 it only changed in r237224
> for GCC 7.
>
> Thus, the following patch reverts the -Wpsabi related changes and keeps just
> the ABI fix and some of the cleanups.
>
> Ok for trunk and 7.1 if it passes bootstrap/regtest? Could somebody from ARM
> bootstrap/regtest it? I can start a distro rpm build with that patch, but
> that will likely be slower.
>
> 2017-04-26 Jakub Jelinek <jakub@redhat.com>
>
> PR target/77728
> * config/aarch64/aarch64.c (struct aarch64_fn_arg_alignment): Remove.
> (aarch64_function_arg_alignment): Return unsigned int again, but still
> ignore TYPE_FIELDS chain decls other than FIELD_DECLs.
> (aarch64_layout_arg): Adjust aarch64_function_arg_alignment caller.
> Don't emit -Wpsabi note.
> (aarch64_function_arg_boundary): Likewise.
> (aarch64_gimplify_va_arg_expr): Adjust aarch64_function_arg_alignment
> caller.
> testsuite/
> * g++.dg/abi/pr77728-2.C: Don't expect -Wpsabi notes.
OK. (tests are fine).
R.
>
> --- gcc/config/aarch64/aarch64.c.jj 2017-04-25 15:55:38.000000000 +0200
> +++ gcc/config/aarch64/aarch64.c 2017-04-26 15:59:09.224554190 +0200
> @@ -2256,58 +2256,34 @@ aarch64_vfp_is_call_candidate (cumulativ
> NULL);
> }
>
> -struct aarch64_fn_arg_alignment
> -{
> - /* Alignment for FIELD_DECLs in function arguments. */
> - unsigned int alignment;
> - /* Alignment for decls other than FIELD_DECLs in function arguments. */
> - unsigned int warn_alignment;
> -};
> -
> -/* Given MODE and TYPE of a function argument, return a pair of alignments in
> +/* Given MODE and TYPE of a function argument, return the alignment in
> bits. The idea is to suppress any stronger alignment requested by
> the user and opt for the natural alignment (specified in AAPCS64 \S 4.1).
> This is a helper function for local use only. */
>
> -static struct aarch64_fn_arg_alignment
> +static unsigned int
> aarch64_function_arg_alignment (machine_mode mode, const_tree type)
> {
> - struct aarch64_fn_arg_alignment aa;
> - aa.alignment = 0;
> - aa.warn_alignment = 0;
> -
> if (!type)
> - {
> - aa.alignment = GET_MODE_ALIGNMENT (mode);
> - return aa;
> - }
> + return GET_MODE_ALIGNMENT (mode);
>
> if (integer_zerop (TYPE_SIZE (type)))
> - return aa;
> + return 0;
>
> gcc_assert (TYPE_MODE (type) == mode);
>
> if (!AGGREGATE_TYPE_P (type))
> - {
> - aa.alignment = TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
> - return aa;
> - }
> + return TYPE_ALIGN (TYPE_MAIN_VARIANT (type));
>
> if (TREE_CODE (type) == ARRAY_TYPE)
> - {
> - aa.alignment = TYPE_ALIGN (TREE_TYPE (type));
> - return aa;
> - }
> + return TYPE_ALIGN (TREE_TYPE (type));
>
> + unsigned int alignment = 0;
> for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> - {
> - if (TREE_CODE (field) == FIELD_DECL)
> - aa.alignment = std::max (aa.alignment, DECL_ALIGN (field));
> - else
> - aa.warn_alignment = std::max (aa.warn_alignment, DECL_ALIGN (field));
> - }
> + if (TREE_CODE (field) == FIELD_DECL)
> + alignment = std::max (alignment, DECL_ALIGN (field));
>
> - return aa;
> + return alignment;
> }
>
> /* Layout a function argument according to the AAPCS64 rules. The rule
> @@ -2399,27 +2375,16 @@ aarch64_layout_arg (cumulative_args_t pc
>
> /* C.8 if the argument has an alignment of 16 then the NGRN is
> rounded up to the next even number. */
> - if (nregs == 2 && ncrn % 2)
> - {
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
> -
> + if (nregs == 2
> + && ncrn % 2
> /* The == 16 * BITS_PER_UNIT instead of >= 16 * BITS_PER_UNIT
> - comparisons are there because for > 16 * BITS_PER_UNIT
> + comparison is there because for > 16 * BITS_PER_UNIT
> alignment nregs should be > 2 and therefore it should be
> passed by reference rather than value. */
> - if (aa.warn_alignment == 16 * BITS_PER_UNIT
> - && aa.alignment < aa.warn_alignment
> - && warn_psabi
> - && currently_expanding_gimple_stmt)
> - inform (input_location,
> - "parameter passing for argument of type %qT "
> - "changed in GCC 7.1", type);
> - else if (aa.alignment == 16 * BITS_PER_UNIT)
> - {
> - ++ncrn;
> - gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
> - }
> + && aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
> + {
> + ++ncrn;
> + gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
> }
>
> /* NREGS can be 0 when e.g. an empty structure is to be passed.
> @@ -2454,10 +2419,8 @@ aarch64_layout_arg (cumulative_args_t pc
> this argument and align the total size if necessary. */
> on_stack:
> pcum->aapcs_stack_words = size / UNITS_PER_WORD;
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
>
> - if (aa.alignment == 16 * BITS_PER_UNIT)
> + if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
> pcum->aapcs_stack_size = ROUND_UP (pcum->aapcs_stack_size,
> 16 / UNITS_PER_WORD);
> return;
> @@ -2548,17 +2511,8 @@ aarch64_function_arg_regno_p (unsigned r
> static unsigned int
> aarch64_function_arg_boundary (machine_mode mode, const_tree type)
> {
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
> - aa.alignment = MIN (MAX (aa.alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> - aa.warn_alignment
> - = MIN (MAX (aa.warn_alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> -
> - if (warn_psabi && aa.warn_alignment > aa.alignment)
> - inform (input_location, "parameter passing for argument of type %qT "
> - "changed in GCC 7.1", type);
> -
> - return aa.alignment;
> + unsigned int alignment = aarch64_function_arg_alignment (mode, type);
> + return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY);
> }
>
> /* For use by FUNCTION_ARG_PADDING (MODE, TYPE).
> @@ -10258,9 +10212,7 @@ aarch64_gimplify_va_arg_expr (tree valis
> stack = build3 (COMPONENT_REF, TREE_TYPE (f_stack), unshare_expr (valist),
> f_stack, NULL_TREE);
> size = int_size_in_bytes (type);
> - struct aarch64_fn_arg_alignment aa
> - = aarch64_function_arg_alignment (mode, type);
> - align = aa.alignment / BITS_PER_UNIT;
> + align = aarch64_function_arg_alignment (mode, type) / BITS_PER_UNIT;
>
> dw_align = false;
> adjust = 0;
> --- gcc/testsuite/g++.dg/abi/pr77728-2.C.jj 2017-04-25 15:55:38.000000000 +0200
> +++ gcc/testsuite/g++.dg/abi/pr77728-2.C 2017-04-26 16:01:18.414878207 +0200
> @@ -1,5 +1,5 @@
> // { dg-do compile { target { { aarch64-*-* } && c++11 } } }
> -// { dg-options "-Wpsabi" }
> +// { dg-options "" }
>
> #include <stdarg.h>
>
> @@ -30,7 +30,7 @@ struct K : public D { typedef A<N> T; in
> struct L { static int h alignas (16); int i, j, k, l; };
>
> int
> -fn1 (int a, B<0> b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> +fn1 (int a, B<0> b)
> {
> return a + b.i;
> }
> @@ -42,14 +42,13 @@ fn2 (int a, B<1> b)
> }
>
> int
> -fn3 (int a, L b) // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> +fn3 (int a, L b)
> {
> return a + b.i;
> }
>
> int
> fn4 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, B<0> n, ...)
> -// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> {
> va_list ap;
> va_start (ap, n);
> @@ -110,7 +109,6 @@ fn9 (int a, int b, int c, int d, int e,
>
> int
> fn10 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, J n, ...)
> -// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> {
> va_list ap;
> va_start (ap, n);
> @@ -121,7 +119,6 @@ fn10 (int a, int b, int c, int d, int e,
>
> int
> fn11 (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, int k, int l, int m, K<0> n, ...)
> -// { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> {
> va_list ap;
> va_start (ap, n);
> @@ -153,19 +150,16 @@ test ()
> static J j;
> static K<0> k0;
> static K<2> k2;
> - fn1 (1, b0); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> + fn1 (1, b0);
> fn2 (1, b1);
> - fn3 (1, l); // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" }
> + fn3 (1, l);
> fn4 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b0, 1, 2, 3, 4);
> - // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> fn5 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, b1, 1, 2, 3, 4);
> fn6 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, c, 1, 2, 3, 4);
> fn7 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, e, 1, 2, 3, 4);
> fn8 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, h, 1, 2, 3, 4);
> fn9 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, i, 1, 2, 3, 4);
> fn10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, j, 1, 2, 3, 4);
> - // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> fn11 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k0, 1, 2, 3, 4);
> - // { dg-message "note: parameter passing for argument of type \[^\n\r]* changed in GCC 7\.1" "" { target *-*-* } .-1 }
> fn12 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, k2, 1, 2, 3, 4);
> }
>
> Jakub
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-26 22:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-25 10:04 [AARCH64 ABI PATCH] Change AARCH64 ABI to match AAPCS, provide -Wpsabi notes (PR target/77728) Jakub Jelinek
2017-04-25 12:51 ` Richard Earnshaw (lists)
2017-04-25 12:58 ` Jakub Jelinek
2017-04-26 16:02 ` [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 " Jakub Jelinek
2017-04-26 16:34 ` Richard Earnshaw (lists)
2017-04-27 1:45 ` Richard Earnshaw (lists)
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).