[PATCH] D151298: [clang][LoongArch] Fix the calling convention for empty struct in C++ mode

[Xi Ruoyao via Phabricator via cfe-commits]

xry111 added a comment.

In D151298#4367349 <https://reviews.llvm.org/D151298#4367349>, @wangleiat wrote:

> In D151298#4367225 <https://reviews.llvm.org/D151298#4367225>, @xry111 wrote:
>
>> In D151298#4367215 <https://reviews.llvm.org/D151298#4367215>, @wangleiat wrote:
>>
>>> In D151298#4367163 <https://reviews.llvm.org/D151298#4367163>, @xry111 wrote:
>>>
>>>> Blocking this as it's a deliberate decision made in D132285 <https://reviews.llvm.org/D132285>.
>>>>
>>>> Is there any imperative reason we must really pass the empty struct?  The C++ standard only treats struct {} as size 1 for the semantics of pointer comparison.  While there is no pointers to registers, ignoring it in the register calling convention will make no harm.
>>>>
>>>> And AFAIK it will be an undefined behavior attempting to (mis)use the padding space of/after the empty struct to pass any information.
>>>
>>> Our current modifications are closer to the description of `Itanium C++ ABI`, and we try to keep it consistent with the description of the calling convention under `reasonable premise`.
>>
>> Hmm, could you provide a link to the section saying this in the Itanium C++ ABI?
>
> http://itanium-cxx-abi.github.io/cxx-abi/abi.html#empty-parameters
> http://itanium-cxx-abi.github.io/cxx-abi/abi.html#emptty-return-values
>
>> I see it has some words about passing or returning an empty class, but there seems no words about passing a class containing an empty class.
>
> It's possible that my understanding is incorrect. There is indeed no place to see how an empty class is passed, just like there is no documentation on how to pass `class A { class B { char c;};};`.

I think the paragraph means:

  class Empty {};
  int test(Empty empty, int a);

Then we should put `a` into `a1`, not `a0`.  And we are indeed doing so.

>> And for our own psABI, it's easier to simply reword it (making it similar to the RISC-V psABI about passing args with FPRs) instead of modifying both Clang and GCC (causing the code of Clang and GCC more nasty, and both compilers slower, and we'll need to add a -Wpsabi warning at least in GCC too).  And it already needs a reword considering empty arrays and zero-width bit-fields anyway.
>
> I'm sorry, I couldn't quite understand what you meant.

I mean now GCC and Clang have the same behavior, so it's easier to just document the behavior in our psABI doc instead of making both Clang and GCC rigidly following the interpretation of psABI (which is currently unclear about zero-sized fields now) anyway.

And the current behavior of GCC and Clang treating a class containing two floating-point members and some empty fields is same as RISC-V, so it's highly unlikely we are violating the C++ standard or IA64 C++ ABI (or the entire RISC-V ecosystem would be violating them).  The only issue is our psABI is unclear about empty fields, and the easiest way to solve the issue is revising the psABI (maybe, just "borrowing" some paragraphs from RISC-V psABI if there is no copyright issue).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151298/new/

https://reviews.llvm.org/D151298