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 <ja...@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
>
