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.

[Lulu Cheng <chenglulu@loongson.cn>]

[-- Attachment #1: Type: text/plain, Size: 4448 bytes --]


在 2023/5/25 上午4:15, Jason Merrill 写道:
> On Wed, May 24, 2023 at 5:00 AM Jonathan Wakely via Gcc-patches 
> <gcc-patches@gcc.gnu.org <mailto:gcc-patches@gcc.gnu.org>> wrote:
>
>     On Wed, 24 May 2023 at 09:41, Xi Ruoyao <xry111@xry111.site> wrote:
>
>     > Wang Lei raised some concerns about Itanium C++ ABI, so let's
>     ask a C++
>     > expert here...
>     >
>     > Jonathan: AFAIK the standard and the Itanium ABI treats an empty
>     class
>     > as size 1
>
>     Only as a complete object, not as a subobject.
>
>
> Also as a data member subobject.
>
>     > in order to guarantee unique address, so for the following:
>     >
>     > class Empty {};
>     > class Test { Empty empty; double a, b; };
>
>     There is no need to have a unique address here, so Test::empty and
>     Test::a
>     have the same address. It's a potentially-overlapping subobject.
>
>     For the Itanium ABI, sizeof(Test) == 2 * sizeof(double).
>
>
> That would be true if Test::empty were marked [[no_unique_address]], 
> but without that attribute, sizeof(Test) is actually 3 * sizeof(double).
>
>     > When we pass "Test" via registers, we may only allocate the
>     registers
>     > for Test::a and Test::b, and complete ignore Test::empty because
>     there
>     > is no addresses of registers.  Is this correct or not?
>
>     I think that's a decision for the loongarch psABI. In principle,
>     there's no
>     reason a register has to be used to pass Test::empty, since you
>     can't read
>     from it or write to it.
>
>
> Agreed.  The Itanium C++ ABI has nothing to say about how registers 
> are allocated for parameter passing; this is a matter for the psABI.
>
> And there is no need for a psABI to allocate a register for 
> Test::empty because it contains no data.
>
> In the x86_64 psABI, Test above is passed in memory because of its 
> size ("the size of the aggregate exceeds two eightbytes...").  But
>
> struct Test2 { Empty empty; double a; };
>
> is passed in a single floating-point register; the Test2::empty 
> subobject is not passed anywhere, because its eightbyte is classified 
> as NO_CLASS, because there is no actual data there.



>
> I know nothing about the LoongArch psABI, but going out of your way to 
> assign a register to an empty class seems like a mistake.

MIPS64 and ARM64 also allocate parameter registers for empty structs. 
https://godbolt.org/z/jT4cY3T5o

Our original intention is not to pass this empty structure member, but 
to make the following two structures treat empty structure members

in the same way in the process of passing parameters.

struct st1
{
     struct empty {} e1;
     long a;
     long b;
};

struct st2
{
     struct empty {} e1;
     double f0;
     double f1;
};


>
>     > On Wed, 2023-05-24 at 14:45 +0800, Xi Ruoyao via Gcc-patches wrote:
>     > > 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.
>     > >
>     >
>     >
>