Hi!
On Wed, Apr 22, 2020 at 03:11:48PM +0200, Jakub Jelinek wrote:
> The problem is that the presence or absence of the C++17
> artificial empty base fields, which have non-zero TYPE_SIZE, but zero
> DECL_SIZE, change the ABI decisions, if it is present (-std=c++17), the type
> might not be considered homogeneous, while if it is absent (-std=c++14), it
> can be.
>
> The following patch fixes that and emits a -Wpsabi inform; perhaps more
> often than it could, because the fact that
> rs6000_discover_homogeneous_aggregate
> returns true when it didn't in in GCC 7/8/9 with -std=c++17 doesn't still
> mean it will make a different ABI decision, but the warning triggered only
> on the test I've changed (the struct-layout-1.exp tests use -w -Wno-psabi
> already).
> PR target/94707
> * config/rs6000/rs6000-call.c (rs6000_aggregate_candidate): Add
> CXX17_EMPTY_BASE_SEEN argument. Pass it to recursive calls.
> Ignore cxx17_empty_base_field_p fields after setting
> *CXX17_EMPTY_BASE_SEEN to true.
Please use the literal capitalisation of symbol names? So that grep and
other search works (the ALL CAPS thing is for the function introductory
comment (and other function comments) only).
> + if (cxx17_empty_base_field_p (field))
> + {
> + *cxx17_empty_base_seen = true;
> + continue;
> + }
Is there no way to describe this without referring to "c++17" (or even
"base field")? It's a pretty gross abstraction violation.
> + inform (input_location,
> + "prior to GCC 10, parameters of type "
> + "%qT were passed incorrectly for C++17", type);
This could more explicitly say that makes the compiled code incompatible
between GCC 10 and older, which is the point of the warning? It's not
really necessary to say the old one was bad -- let's hope it was :-)
Looks fine otherwise. Okay for trunk whatever you decide to do (or not
do) with my comments. Thank you!
Segher
