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 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.

> 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.
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.

> Judging from the size of the structure, the size of st2 is already 
> larger than 2xGRLEN, should be passed by reference.

I'd consider it a flaw in the psABI doc because it was obviously written
w/o considering the possibility of a zero-sized field in an aggregate.

I knew this flaw when I created r12-8294 and I planned to revise the
psABI doc for it, but I've never improved my English skill enough to do
the work.  I'm considering copying some word from RISC-V psABI if there
is no copyright issue.

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