public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
@ 2022-03-21 16:28 Jakub Jelinek
  2022-03-21 21:50 ` Andreas Krebbel
  2022-03-22 16:28 ` Richard Earnshaw
  0 siblings, 2 replies; 8+ messages in thread
From: Jakub Jelinek @ 2022-03-21 16:28 UTC (permalink / raw)
  To: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov,
	Andreas Krebbel, Ulrich Weigand, Eric Botcazou
  Cc: gcc

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
  2022-03-21 16:28 Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024) Jakub Jelinek
@ 2022-03-21 21:50 ` Andreas Krebbel
  2022-03-22 16:28 ` Richard Earnshaw
  1 sibling, 0 replies; 8+ messages in thread
From: Andreas Krebbel @ 2022-03-21 21:50 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Earnshaw, Richard Sandiford,
	Kyrylo Tkachov, Ulrich Weigand, Eric Botcazou
  Cc: gcc

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
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
  2022-03-21 16:28 Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024) Jakub Jelinek
  2022-03-21 21:50 ` Andreas Krebbel
@ 2022-03-22 16:28 ` Richard Earnshaw
  2022-03-22 16:51   ` Jakub Jelinek
  2022-03-25 14:26   ` Richard Earnshaw
  1 sibling, 2 replies; 8+ messages in thread
From: Richard Earnshaw @ 2022-03-22 16:28 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Earnshaw, Richard Sandiford,
	Kyrylo Tkachov, Andreas Krebbel, Ulrich Weigand, Eric Botcazou
  Cc: gcc



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.

R.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
  2022-03-22 16:28 ` Richard Earnshaw
@ 2022-03-22 16:51   ` Jakub Jelinek
  2022-03-22 17:11     ` Richard Earnshaw
  2022-03-25 14:26   ` Richard Earnshaw
  1 sibling, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2022-03-22 16:51 UTC (permalink / raw)
  To: Richard Earnshaw
  Cc: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov,
	Andreas Krebbel, Ulrich Weigand, Eric Botcazou, gcc

On Tue, Mar 22, 2022 at 04:28:08PM +0000, Richard Earnshaw wrote:
> 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.

Layout is always done with the int : 0; bitfields in TYPE_FIELDS and
only after that is done C++ FE used to remove them.
So yes, it only can affect the passing of parameters and return values
in registers (or partially in registers, partially in memory).

> 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.
> 
> PS.  It looks like llvm and llvm++ are inconsistent on this one as well.

At least on x86_64 clang and clang++ consistently honored the zero width
bitfields during structure layout and ignored them during parameter passing
decisions (i.e. what the x86_64 psABI chose to clarify).

I guess it would be nice to include the testcases we are talking about,
like { float x; int : 0; float y; } and { float x; int : 0; } and
{ int : 0; float x; } into compat.exp testsuite so that we see ABI
differences in compat testing.

	Jakub


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
  2022-03-22 16:51   ` Jakub Jelinek
@ 2022-03-22 17:11     ` Richard Earnshaw
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Earnshaw @ 2022-03-22 17:11 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Earnshaw, Richard Sandiford, Ulrich Weigand, gcc



On 22/03/2022 16:51, Jakub Jelinek via Gcc wrote:
> On Tue, Mar 22, 2022 at 04:28:08PM +0000, Richard Earnshaw wrote:
>> 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.
> 
> Layout is always done with the int : 0; bitfields in TYPE_FIELDS and
> only after that is done C++ FE used to remove them.
> So yes, it only can affect the passing of parameters and return values
> in registers (or partially in registers, partially in memory).
> 
>> 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.
>>
>> PS.  It looks like llvm and llvm++ are inconsistent on this one as well.
> 
> At least on x86_64 clang and clang++ consistently honored the zero width
> bitfields during structure layout and ignored them during parameter passing
> decisions (i.e. what the x86_64 psABI chose to clarify).
> 
I was looking at aarch32 (arm).
Compiling

struct S { float a; int : 0; float b; };

struct S
foo (struct S x)
{
   x.b += 1.0f;
   return x;
}

with clang-10 I get

foo:
         .fnstart
         vmov.f32        s0, #1.000000e+00
         str     r1, [r0]
         vmov    s2, r2
         vadd.f32        s0, s2, s0
         vstr    s0, [r0, #4]
         bx      lr

while clang++10 gives

_Z3foo1S:
         .fnstart
         vmov.f32        s2, #1.000000e+00
         vadd.f32        s1, s1, s2
         bx      lr

Both with the options
-S -O2 abi-bf.c -o - --target=arm-none-eabi -march=armv8-a -mfpu=neon 
-mfloat-abi=hard

So for C it has passed the object in r1/r2 and returned it in memory, 
while for C++ it has passed it as an HFA.

But for AArch64 it doesn't look right either:

clang-10

foo:                                    // @foo
// %bb.0:
         lsr     x8, x0, #32
         fmov    s0, #1.00000000
         fmov    s1, w8
         fadd    s0, s1, s0
         fmov    w8, s0
         bfi     x0, x8, #32, #32
         ret

clang++-10:


_Z3foo1S:                               // @_Z3foo1S
// %bb.0:
         fmov    s2, #1.00000000
         fadd    s1, s1, s2
         ret

> I guess it would be nice to include the testcases we are talking about,
> like { float x; int : 0; float y; } and { float x; int : 0; } and
> { int : 0; float x; } into compat.exp testsuite so that we see ABI
> differences in compat testing.
> 
> 	Jakub
> 

Yes, we might also add some specific Arm ABI tests as well.

R.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
  2022-03-22 16:28 ` Richard Earnshaw
  2022-03-22 16:51   ` Jakub Jelinek
@ 2022-03-25 14:26   ` Richard Earnshaw
  2022-03-25 14:47     ` Jakub Jelinek
  1 sibling, 1 reply; 8+ messages in thread
From: Richard Earnshaw @ 2022-03-25 14:26 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Earnshaw, Richard Sandiford,
	Kyrylo Tkachov, Andreas Krebbel, Ulrich Weigand, Eric Botcazou
  Cc: gcc



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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
  2022-03-25 14:26   ` Richard Earnshaw
@ 2022-03-25 14:47     ` Jakub Jelinek
  2022-03-25 14:55       ` Richard Earnshaw
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2022-03-25 14:47 UTC (permalink / raw)
  To: Richard Earnshaw
  Cc: Richard Earnshaw, Richard Sandiford, Kyrylo Tkachov,
	Andreas Krebbel, Ulrich Weigand, Eric Botcazou, gcc

On Fri, Mar 25, 2022 at 02:26:56PM +0000, Richard Earnshaw wrote:
> 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.

Ok.  So on the GCC side you need for both arm and aarch64 something similar
to the r12-6418-g3159da6c46568a7c change (of course on the ARM/AArch64 side
it will be in different spots etc.).
But generally, if you see during TYPE_FIELDS walk for argument/return value
passing decisions (both test whether something could be passed in registers
or say alignment decisions for those) rather than layout
  DECL_BIT_FIELD (field) && integer_zerop (DECL_SIZE (field))
ignore it - if DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field) then always,
otherwise arrange for 2 invocations in which one ignores them and one
doesn't and warns if the overall decisions change.

	Jakub


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024)
  2022-03-25 14:47     ` Jakub Jelinek
@ 2022-03-25 14:55       ` Richard Earnshaw
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Earnshaw @ 2022-03-25 14:55 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Earnshaw, Richard Sandiford, Ulrich Weigand, gcc



On 25/03/2022 14:47, Jakub Jelinek via Gcc wrote:
> On Fri, Mar 25, 2022 at 02:26:56PM +0000, Richard Earnshaw wrote:
>> 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.
> 
> Ok.  So on the GCC side you need for both arm and aarch64 something similar
> to the r12-6418-g3159da6c46568a7c change (of course on the ARM/AArch64 side
> it will be in different spots etc.).
> But generally, if you see during TYPE_FIELDS walk for argument/return value
> passing decisions (both test whether something could be passed in registers
> or say alignment decisions for those) rather than layout
>    DECL_BIT_FIELD (field) && integer_zerop (DECL_SIZE (field))
> ignore it - if DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field) then always,
> otherwise arrange for 2 invocations in which one ignores them and one
> doesn't and warns if the overall decisions change.
> 
> 	Jakub
> 

Do we really need two passes?  Surely, if we find a zero-width bitfield 
in the type we just set a marker to note that it was found.  If, at the 
end of walking the type, it's still a candidate for passing in FP regs, 
then we've identified an ABI change because previously we would not have 
behaved this way.

R.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-03-25 14:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21 16:28 Urgent GCC ABI backend maintainer ping re zero width bitfield passing (PR102024) Jakub Jelinek
2022-03-21 21:50 ` Andreas Krebbel
2022-03-22 16:28 ` Richard Earnshaw
2022-03-22 16:51   ` Jakub Jelinek
2022-03-22 17:11     ` Richard Earnshaw
2022-03-25 14:26   ` Richard Earnshaw
2022-03-25 14:47     ` Jakub Jelinek
2022-03-25 14:55       ` Richard Earnshaw

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).