Re: [PATCH] LoongArch: Fix the problem of structure parameter passing in C++. This structure has empty structure members and less than three floating point members.

[Xi Ruoyao <xry111@xry111.site>]

On Wed, 2023-05-24 at 18:07 +0800, Lulu Cheng wrote:
> 
> 在 2023/5/24 下午5:25, Xi Ruoyao 写道:
> > On Wed, 2023-05-24 at 16:47 +0800, Lulu Cheng wrote:
> > > 在 2023/5/24 下午2:45, Xi Ruoyao 写道:
> > > > On Wed, 2023-05-24 at 14:04 +0800, Lulu Cheng wrote:
> > > > > An empty struct type that is not non-trivial for the purposes of
> > > > > calls
> > > > > will be treated as though it were the following C type:
> > > > > 
> > > > > struct {
> > > > >     char c;
> > > > > };
> > > > > 
> > > > > Before this patch was added, a structure parameter containing an
> > > > > empty structure and
> > > > > less than three floating-point members was passed through one or
> > > > > two floating-point
> > > > > registers, while nested empty structures are ignored. Which did
> > > > > not conform to the
> > > > > calling convention.
> > > > No, it's a deliberate decision I've made in
> > > > https://gcc.gnu.org/r12-8294.  And we already agreed "the ABI needs
> > > > to
> > > > be updated" when we applied r12-8294, but I've never improved my
> > > > English
> > > > skill to revise the ABI myself :(.
> > > > 
> > > > We are also using the same "de-facto" ABI throwing away the empty
> > > > struct
> > > > for Clang++ (https://reviews.llvm.org/D132285).  So we should update
> > > > the
> > > > spec here, instead of changing every implementation.
> > > > 
> > > > The C++ standard treats the empty struct as size 1 for ensuring the
> > > > semantics of pointer comparison operations.  When we pass it through
> > > > the
> > > > registers, there is no need to really consider the empty field
> > > > because
> > > > there is no pointers to registers.
> > > > 
> > > I think that the rules for passing parameters to empty structures or
> > > nested empty structures should be unified,
> > There is no need to unify them because "passing a struct" is already
> > different from "passing its members one by one".  Say:
> > 
> > int f1(int a, int b);
> > 
> > and
> > 
> > int f2(struct {int a, b;} ab);
> > 
> > "a" and "b" are already passed differently.
> I mean I think that empty structs in st1 and st2 should be treated the
> same way in the way of passing parameters.
> > 
> > > but the current implementation in gcc is as follows(in C++):
> > > 
> > > Compare the two structures,the current implementation is as follows:
> > > 
> > > struct st1
> > > {
> > >     struct empty {} e1;
> > >     long a;
> > >     long b;
> > > };
> > > 
> > > passed by reference.
> > > 
> > > 
> > > struct st2
> > > {
> > >     struct empty {} e1;
> > >     double f0;
> > >     double f1;
> > > };
> > > 
> > > passed through two floating-point registers.
> > Well this is nasty, but it is the same behavior as RISC-V:
> > https://godbolt.org/z/fEexq148r
> > 
> > I deliberately made our logic similar to RISC-V in r12-8294 because
> > "there seems no reason to do it differently".  Maybe I was wrong and we
> > should have ignored st1::e1 as well (but IIRC we were running out of
> > time for GCC 12 release so we didn't have time to consider this :( ).
> > 
> > But now it's better to "keep the current behavior as-is" because:
> > 
> > 1. The current behavior of GCC and Clang already matches and the
> > behavior is kept since the day one GCC and Clang supports LoongArch.  So
> > there is currently no ABI incompatibility in practice, but changing the
> > behavior will introduce an ABI incompatibility.
> 
> The parameter passing rules for a single empty structure are different
> in GCC and Clang.
> 
> eg:
> 
> void test (struct empty, int a);
> 
> In GCC, the empty structure is passed through $a0, and the variable a is 
> passed through $a1,
> 
> but Clang passes a through $a0, and the empty structure is ignored.
> 
> > 2. Changing the behavior will make the compiler more complex, and
> > slower.
> > 3. Changing the behavior will need a -Wpsabi warning according to the
> > GCC policy, leading to more boring code (and more slow-down) in the
> > compiler.
> 
> I really understand and thank you for your concerns, we have also 
> considered the issue of compatibility.
> 
> Before the modification, we made an assessment. The colleagues of the 
> operating system
> 
> built a total of 3,300 linux basic packages, and only one package was 
> affected by this modification.
> 
> This is why GCC fixes this as a bug without adding -Wpsabi.

If you are really determined to do this, then OK.  I'm in a very bad
mood and I don't want to spend my mental strength on debating (esp. on a
corner case unlikely to affect "real" code) anymore.

But remember to add a entry in GCC 14 changes.html, and test this thing:

struct Empty {};

struct Something : Empty
{
  double a, b;
};

If we are not careful enough we may introduce a ABI mismatch between -
std=c++14 and -std=c++17 here.  See https://gcc.gnu.org/PR94383.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University