Re: [AARCH64 PATCH] Don't provide -Wpsabi notes for AARCH64 (PR target/77728)

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

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
>