Re: [PATCH] mips: Emit psabi diagnostic for return values affected by C++ zero-width bit-field ABI change [PR 102024]

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Apr 01, 2022 at 12:13:30AM +0800, Xi Ruoyao wrote:
> --- a/gcc/config/mips/mips.cc
> +++ b/gcc/config/mips/mips.cc
> @@ -6274,10 +6274,17 @@ mips_callee_copies (cumulative_args_t, const function_arg_info &arg)
>  
>     For n32 & n64, a structure with one or two fields is returned in
>     floating-point registers as long as every field has a floating-point
> -   type.  */
> +   type.
> +
> +   The C++ FE used to remove zero-width bit-fields in GCC 11 and earlier.
> +   To make a proper diagnostic, this function will set HAS_ZERO_WIDTH_BF
> +   to 1 once a C++ zero-width bit-field shows up, and then ignore it.
> +   Then the caller can determine if this zero-width bit-field will make a
> +   difference and emit a -Wpsabi inform.  */
>  
>  static int
> -mips_fpr_return_fields (const_tree valtype, tree *fields)
> +mips_fpr_return_fields (const_tree valtype, tree *fields,
> +			int *has_zero_width_bf)

Use bool * and = true ?

> @@ -6319,11 +6332,14 @@ static bool
>  mips_return_in_msb (const_tree valtype)
>  {
>    tree fields[2];
> +  int has_zero_width_bf = 0;

I think it would be better to match more closely what the code did before.
  if (!TARGET_NEWABI || !TARGET_BIG_ENDIAN || !AGGREGATE_TYPE_P (valtype))
    return false;

  tree fields[2];
  bool has_zero_width_bf = false;
  return (mips_fpr_return_fields (valtype, fields, &has_zero_width_bf) == 0
	  || has_zero_width_bf);

> +  int use_fpr = mips_fpr_return_fields (valtype, fields,
> +					&has_zero_width_bf);
>  
>    return (TARGET_NEWABI
>  	  && TARGET_BIG_ENDIAN
>  	  && AGGREGATE_TYPE_P (valtype)
> -	  && mips_fpr_return_fields (valtype, fields) == 0);
> +	  && (use_fpr == 0 || has_zero_width_bf));
>  }
>  
>  /* Return true if the function return value MODE will get returned in a
> @@ -6418,8 +6434,35 @@ mips_function_value_1 (const_tree valtype, const_tree fn_decl_or_type,
>  	 return values, promote the mode here too.  */
>        mode = promote_function_mode (valtype, mode, &unsigned_p, func, 1);
>  
> +      int has_zero_width_bf = 0;

See above for int -> bool and 0 -> false.

> +      int use_fpr = mips_fpr_return_fields (valtype, fields,
> +					    &has_zero_width_bf);
> +      if (TARGET_HARD_FLOAT &&
> +	  warn_psabi &&
> +	  has_zero_width_bf &&
> +	  use_fpr != 0)

&&s need to go at the start of next line, not end of line.

> +	{
> +	  static unsigned last_reported_type_uid;
> +	  unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
> +	  if (uid != last_reported_type_uid)
> +	    {
> +	      static const char *url =

Similarly for =

> +		CHANGES_ROOT_URL
> +		"gcc-12/changes.html#zero_width_bitfields";
> +	      inform (input_location,
> +		      "the ABI for returning a value containing "
> +		      "zero-width bit-fields but otherwise an aggregate "
> +		      "with only one or two floating-point fields was "
> +		      "retconned in GCC %{12.1%}", url);

In this diagnostics it is ok to talk about bit-fields as that is
the only thing that changes (compared to the other patch).
But again, was retconned -> changed ?

	Jakub