Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)

[Andreas Krebbel <krebbel@linux.ibm.com>]

On 3/21/22 17:28, Jakub Jelinek wrote:
> Hi!
> 
> I'd like to ping port maintainers about
> https://gcc.gnu.org/PR102024
> 
> As I wrote, the int : 0 bitfields are present early in the TYPE_FIELDS
> during structure layout and intentionally affect the layout.
> We had some code to remove those from TYPE_FIELDS chains in the C and C++
> FEs, but for C that removal never worked correctly (never removed any)
> and the non-working removal was eventually removed.  For C++ it
> didn't initially work either, but for GCC 4.5 that was fixed in PR42217,
> so on various backends where TYPE_FIELDS are analyzed for how to pass or
> return certain aggregates starting with GCC 4.5 the C++ and C ABI diverged.
> In August, I have removed that zero width bitfield removal from C++ FE
> as the FE needs to take those bitfields into account later on as well.
> 
> The x86_64 backend was changed in r12-6418-g3159da6c46 to match recently
> approved clarification of the x86-64 psABI and the zero width bitfields
> are now ignored for both C and C++ (so an ABI change for C from 11.x and
> earlier to 12.x and for C++ from GCC 4.4 and earlier to 4.5 and later)
> with a -Wpsabi diagnostics about it.
> 
> The rs6000 backend was changed in r12-3843-g16e3d6b8b2 to never ignore
> those bitfields (so no ABI change for C, for C++ ...-4.4 and 12+ are
> ABI incompatible with 4.5 through 11.x; note, it affects I think just
> ppc64le ABI, which didn't really exist before 4.8 I think) and diagnostics
> has been added about the ABI change.
> 
> As I wrote in the PR, I believe most of the GCC backends are unaffected,
> x86_64 and rs6000 are handled, riscv was changed already in GCC 10 to
> ignore those bitfields and emit a -Wpsabi diagnostics.
> 
> I can see code-generation differences certainly on armv7hl and aarch64.
> ia64, iq2000, mips, s390 and sparc are maybe affected, haven't checked.

On s390 we walk the field chain to figure out whether it is a struct with a single floating point
field. Consequently I see different code being generated by g++ for { float a; int :0; } which is
passed in a floating point register with g++ 11 and in memory with g++ 12.

I'm not sure what is best for our target. I'll try to come back with an answer soon.

Bye,

Andreas

> 
> Simple testcase could be e.g.:
> struct S { float a; int : 0; float b; };
> 
> __attribute__((noipa)) struct S
> foo (struct S x)
> {
>   return x;
> }
> 
> void
> bar (void)
> {
>   struct S s = { 0.0f, 0.0f };
>   foo (s);
> }
> where one should look at the argument and return value passing
> in GCC 11 C, GCC 11 C++, GCC trunk C, GCC trunk C++.
> 
> The FE now sets bits on the bitfields that make it possible to
> differentiate between the different cases, so each port may decide to do
> one of the 3 things:
> 1) keep ABI exactly compatible between GCC 11 and 12, which means
>    C and C++ will continue to be incompatible
> 2) keep the G++ 4.5 through 11 ABI of ignoring zero width bitfields and
>    change C ABI
> 3) keep the GCC < 11 C ABI of not ignoring zero width bitfields and
>    change the C++ ABI (which means restoring ABI compatibility in
>    this regard between G++ 4.4 and earlier with G++ 12 and later)
> Furthermore, it would be very nice to emit -Wpsabi diagnostics for the
> changed ABI unless 1) is decided.
> One should take into account psABI as well as what other compilers do.
> 
> The current state of GCC trunk is that 3) is done except that x86_64
> did 2) and riscv did 2 already for GCC 10 and all of x86_64, riscv and
> rs6000 emit -Wpsabi diagnostics (though I think rs6000 doesn't guard
> it with -Wpsabi).
> 
> I can help with the backend implementations if needed, but I can't
> decide which possibility you want to choose for each backend.
> It would be really nice to decide about this soon, because changing
> the ABI in GCC 12 only to change it again in GCC 13 doesn't look much
> desirable and even if 3) is the choice, it is really nice to have
> some diagnostics about ABI changes.
> 
> Thanks
> 
> 	Jakub
>