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

[Richard Earnshaw]

On 22/03/2022 16:28, Richard Earnshaw via Gcc wrote:
> 
> 
> On 21/03/2022 16:28, Jakub Jelinek via Gcc 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.
>>
>> 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
>>
> 
> Unless I've missed something subtle here, the layout of
> 
>    struct S { float a; int : 0; float b;};
> 
> is going to identical to
> 
>    struct T { float a; float b;};
> 
> on pretty much every architecture I can think of, so this is purely 
> about parameter passing rules for the former and whether the two cases 
> above should behave the same way.
> 
> The AAPCS and AAPCS64 both contain the same statement as part of the 
> definition of an HFA:
> 
> | The test for homogeneity is applied after data layout is
> | completed and without regard to access control or other source
> | language restrictions.
> 
> The access control and source language restrictions was intended to 
> cover c++-style features such as public/private, so aren't really 
> relevant to this discussion (though you might plausibly read 'source 
> language restriction' to cover this).  However, the fact that the test 
> is applied after layout has been done and because a zero-sized bit-field 
> neither
> - adds an accessible member
> - changes the layout in any case I can think of that would potentially 
> be an HFA.
> my preliminary conclusion is that for Arm and AArch64 we still have a 
> duck here (if it walks like one and quacks like one...).
> 
> I'm still awaiting final confirmation of this from our internal ABI 
> group, but I'm pretty confident that this will be our final position.

Just to confirm that this is our final position.  The 'int:0 field 
should be ignored for the purposes of determining the parameter passing 
as it has no effect on the layout of the type.

We do not feel that an update to the AAPCS or AAPCS64 is needed as the 
wording already covers this.


I'll raise an issue with the LLVM team internally.

Thanks
R.
> 
> R.
> 
> PS.  It looks like llvm and llvm++ are inconsistent on this one as well.