This is an archive of the discontinued LLVM Phabricator instance.

[Clang][LoongArch] Implement ABI lowering
ClosedPublic

Authored by SixWeining on Aug 20 2022, 1:54 AM.

Details

Summary

Reuse most of RISCV's implementation with several exceptions:

  1. Assign signext/zeroext attribute to args passed in stack.

On RISCV, integer scalars passed in registers have signext/zeroext
when promoted, but are anyext if passed on the stack. This is defined
in early RISCV ABI specification. But after this change [1], integers
should also be signext/zeroext if passed on the stack. So I think
RISCV's ABI lowering should be updated [2].

While in LoongArch ABI spec, we can see that integer scalars narrower
than GRLEN bits are zero/sign-extended no matter passed in registers
or on the stack.

  1. Zero-width bit fields are ignored.

This matches GCC's behavior but it hasn't been documented in ABI sepc.
See https://gcc.gnu.org/r12-8294.

  1. char is signed by default.

There is another difference worth mentioning is that char is signed
by default on LoongArch while it is unsigned on RISCV.

This patch also adds _BitInt type support to LoongArch and handle it
in LoongArchABIInfo::classifyArgumentType.

[1] https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/cec39a064ee0e5b0129973fffab7e3ad1710498f
[2] https://github.com/llvm/llvm-project/issues/57261

Diff Detail

Event Timeline

SixWeining created this revision.Aug 20 2022, 1:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2022, 1:54 AM
SixWeining requested review of this revision.Aug 20 2022, 1:54 AM

Are cases like

struct x { double a; __int128 : 0; double b;};
double f(struct x x) { return x.a + x.b; }

handled properly? AFAIK RISC-V clang currently does not handle it correctly:

https://godbolt.org/z/rvM99zbqc (GCC handles it properly)
https://godbolt.org/z/sWY5vs5ce (Clang does not handle it properly)

Note that we rectified the ABI to match the behavior of RISC-V GCC deliberately (https://gcc.gnu.org/r12-8294) but I didn't rewrite the document because the behavior is not easy to describe with my poor English.

Are cases like

struct x { double a; __int128 : 0; double b;};
double f(struct x x) { return x.a + x.b; }

handled properly? AFAIK RISC-V clang currently does not handle it correctly:

https://godbolt.org/z/rvM99zbqc (GCC handles it properly)
https://godbolt.org/z/sWY5vs5ce (Clang does not handle it properly)

Note that we rectified the ABI to match the behavior of RISC-V GCC deliberately (https://gcc.gnu.org/r12-8294) but I didn't rewrite the document because the behavior is not easy to describe with my poor English.

Thanks. Since current ABI doesn’t document the behavior of zero-bit fields so I pay less attention to this case. Let me check it later.

Ignore zero-width bit fields and add a test.

SixWeining edited the summary of this revision. (Show Details)Aug 21 2022, 12:13 AM

Ignore zero-width bit fields and add a test.

Thanks! There is a GCC test (https://gcc.gnu.org/r12-7951) which can be used to test if there is any ABI incompatibility between GCC and Clang, but I guess it's not possible to run it at the early development stage of Clang. Maybe we can run it later.

Ignore zero-width bit fields and add a test.

Thanks! There is a GCC test (https://gcc.gnu.org/r12-7951) which can be used to test if there is any ABI incompatibility between GCC and Clang, but I guess it's not possible to run it at the early development stage of Clang. Maybe we can run it later.

Thanks. Let me add a test for zero size arrays.

And, with the parent revison D130255 landed, we can compile .c programs to loongarch executables now. But currently since the f and d target feaures can not been passed to clang cc1 yet, we may manually pass them to clang cc1 to generate floating-point instructions. For example:

$ clang --target=loongarch64-unknown-linux-gnu --gcc-toolchain=xxx --sysroot=xxx -Xclang -target-feature -Xclang +f main.c

Add a test for struct with zero size arrays.

SixWeining retitled this revision from [LoongArch] Implement ABI lowering to [Clang][LoongArch] Implement ABI lowering.Aug 22 2022, 11:29 PM
SixWeining edited the summary of this revision. (Show Details)
rengolin accepted this revision.Sep 17 2022, 12:10 PM

I can't vouch for your ABI correctness, but you seem to have a lot of tests covering what you claim to have implemented, so looks good to me.

Thanks for opening an issue for RISC-V.

Clang's TargetInfo.cpp is really large, perhaps we should think about splitting it across different targets, but definitely NOT for this review.

Thanks!

This revision is now accepted and ready to land.Sep 17 2022, 12:10 PM
This revision was automatically updated to reflect the committed changes.