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.

[Jonathan Wakely <jwakely@redhat.com>]

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

On Wed, 24 May 2023 at 21:15, Jason Merrill <jason@redhat.com> wrote:

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

Doh, yes.


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