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.radhakrish...@arm.com>
> Jakub Jelinek <ja...@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
>
