This is an archive of the discontinued LLVM Phabricator instance.

[Clang][CSKY] Add support about CSKYABIInfo
ClosedPublic

Authored by zixuan-wu on May 26 2022, 1:32 AM.

Details

Summary

According to the CSKY ABIv2 document, construct the ABIInfo to handle argument passing and return of clang data type. It also includes how to emit and expand VAArg intrinsic.

Diff Detail

Event Timeline

zixuan-wu created this revision.May 26 2022, 1:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2022, 1:32 AM
zixuan-wu requested review of this revision.May 26 2022, 1:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2022, 1:32 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Can you reupload with more context? See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

I doubt I'll be able to give any in depth review but if things look good generally I'm sure you'll find the issues through your own testing as time goes on.

zixuan-wu updated this revision to Diff 432228.May 26 2022, 3:24 AM

Can you reupload with more context? See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

I doubt I'll be able to give any in depth review but if things look good generally I'm sure you'll find the issues through your own testing as time goes on.

Well, sorry that I forgot to add -U999999 option.

This looks good to me, but wait to make sure others see it, too.

My reasons are: it is largely similar to RISCV implementation, it seems to follow what I expected of the ABI (which is similar to other targets) and has a large corpus of tests.

I can't comment on the specifics of the ABI implementation (I haven't read the ABI document *that* thoroughly), but as David said, issues will be picked up by tests until the target reaches maturity.

Looks good to me. Can you just clarify how the tests are split? My guess is that one is stuff that doesn't vary with hard/soft float and the other is the bits that change when hardfloat is enabled. If so is it worth checking those situations with soft float too or have you already done that.

clang/lib/CodeGen/TargetInfo.cpp
11708

I see a "hard-float" and "hard-float-abi" in this change. Is it safe to check just for "hard-float-abi" here?

Just checking, I assume it's fine. You could have hardware with hard float but you want to compile for soft float abi. This allows that.

11710

Silly question, was/is there an fpuv1 and was it 32 bit only? (I assume so)

clang/test/CodeGen/CSKY/csky-abi.c
4

Is this file checking things that don't vary between hard float/not hard float? If so please add a comment that states that.

This looks good to me, but wait to make sure others see it, too.

My reasons are: it is largely similar to RISCV implementation, it seems to follow what I expected of the ABI (which is similar to other targets) and has a large corpus of tests.

I can't comment on the specifics of the ABI implementation (I haven't read the ABI document *that* thoroughly), but as David said, issues will be picked up by tests until the target reaches maturity.

I don't think it is largely similar to RISCV implementation except for the code structure and test reuse. And the test result is big different.

zixuan-wu added inline comments.May 26 2022, 7:19 PM
clang/lib/CodeGen/TargetInfo.cpp
11708

You could have hardware with hard float but you want to compile for soft float abi. This allows that.

Yes. As you said.

11710

I think fpuv1 is not supported anymore in llvm.

BTW, I have run llvm-test-suite, it passed.

I don't think it is largely similar to RISCV implementation except for the code structure and test reuse. And the test result is big different.

Sorry, that's what I meant. There are no new things added, except for the RISCV specific logic, which makes it very easy to review, so thanks! :D

zixuan-wu updated this revision to Diff 432820.May 29 2022, 7:44 PM
zixuan-wu marked an inline comment as done.May 30 2022, 12:39 AM
This revision is now accepted and ready to land.May 30 2022, 1:31 AM
This revision was automatically updated to reflect the committed changes.