[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#4367458 <https://reviews.llvm.org/D151298#4367458>, @wangleiat wrote:

>> 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.
>
> yes. with this patch, `a` will be passed with `a1` register.
>
>> 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).
>
> If we want to ignore empty structures, it is not enough to only modify psABI, because the current implementations of gcc and clang do not ignore empty structures in all cases. For example:
>
>   struct { struct{}; int i; }; // in this case, the empty struct is not ignored.
>   struct { struct{}; float f; }; // ignored.

This is the same behavior as RISC-V.  While attempting to pass a struct through FPRs, the empty field is ignored.  But if passing through FPR does not work and it's passed through GPRs, the empty fields are not ignored:

https://godbolt.org/z/T1PKoxbYM

> Whether to ignore empty structures or not can affect the testing of gdb. @seehearfeel knows more details.

I guess we should be able to fix it for GDB because they must have fixed it for RISC-V already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151298