Hi!

On Tue, Sep 21, 2021 at 05:35:56PM -0500, Bill Schmidt wrote:
> Previously zero-width bit fields were removed from structs, so that 
> otherwise
> homogeneous aggregates were treated as such and passed in FPRs and VSRs.
> This was incorrect behavior per the ELFv2 ABI.  Now that these fields are no
> longer being removed, we generate the correct parameter passing code.  Alert
> the unwary user in the rare cases where this behavior changes.

> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no 
> regressions.
> Is this okay for trunk?

This can obviously not change anything for other ABIs, so it doesn't
need testing anywhere else.  Good :-)

> @@ -6227,7 +6227,7 @@ const struct altivec_builtin_types 
> altivec_overloaded_builtins[] = {
>  
>  static int
>  rs6000_aggregate_candidate (const_tree type, machine_mode *modep,
> -                         int *empty_base_seen)
> +                         int *empty_base_seen, int *zero_width_bf_seen)

The new function parameter should be described in the function comment.

> +         if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
> +           {
> +             *zero_width_bf_seen = 1;
> +             continue;
> +           }

Please add a comment here, saying what it is for?  I'd do it inside the
braces.

>           sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep,
> -                                                 empty_base_seen);
> +                                                 empty_base_seen,
> +                                                 zero_width_bf_seen);

So it is important that this function only ever sets *zero_width_bf_seen
to 1, never resets it; please document that with it as well?

> +                   inform (input_location,
> +                           "parameter passing for an argument containing "
> +                           "zero-width bit fields but that is otherwise a 
> "
> +                           "homogeneous aggregate changed in GCC 12.1");

Just "GCC 12" please.  You might want to indicate that older compilers
did the wrong thing here.  And maybe say this is only for ELFv2 somehow?
In some way that doesn't make it more confusing than not saying it :-)

> +double
> +foo (a_thing a) // { dg-message "parameter passing for an argument 
> containing zero-width bit fields but that is otherwise a homogeneous 
> aggregate changed in GCC 12.1" }

I think you used "format=flawed" again?

Okay for trunk with such comment updates.  Thanks!


Segher

Reply via email to