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.

[Jason Merrill <jason@redhat.com>]

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

On Wed, May 24, 2023 at 5:00 AM Jonathan Wakely via Gcc-patches <
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.

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