-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CIR][CUDA][NFC] Add NVPTX64 ABI info skeleton #1303
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this.
We usually go about developing new features by incrementally introducing only the needed pieces along side testcases that exercise them. Both classifyReturnType
and classifyArgumentType
should be part of TargetLowering (we don't lower those ABI details directly out of CIRGen).
If you take a closer look at ABIInfo for other targets, you're going to notice they don't do very much at CIRGen stage. It's fine if this PR only introduces a skeleton, but it gotta be reduced to a minimum functionality and be annotated with llvm_unrecheable("not yet implement")
, so that you can incrementally add more pieces as you introduce testcases.
You also need to setup clang-format and format your code according to the LLVM coding guidelines (see the code_formatter failures) |
Thank you for your explanation. I have reduced the code to a skeleton. |
Ideally this change would have come as part of something you needed to build a testcase, however given this will only introduce skeleton, it's fine to give it a pass! |
It's done. Sorry for the close and reopen, I accidentally pushed a version identical to this repository onto my fork. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will merge once it passes gh tests
Seems content of this PR is already merged, shall I close it? |
Registered ABI Info of NVPTX64 in Clang IR. The bulk of the code is taken from
CodeGen/NVPTX.cpp
, and I replaced the calls to LLVM with the calls to CIR.