[CIR][CUDA] CallConvLowering for basic types in NVPTX #1468
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bcardosolopes
requested changes
Mar 11, 2025
| // getKind() > BuiltinType::Void && getKind() <= BuiltinType::NullPtr | ||
| // which is confusing and unavailable in CIR. | ||
| // Here we manually list out all corresponding types in CIR. | ||
| return isa<IntType, BoolType, SingleType, DoubleType, FP16Type, FP128Type, |
There was a problem hiding this comment.
Please add a helper function in clang/include/clang/CIR/Dialect/IR/CIRTypes.h, just like we have isAnyFloatingPointType, we should also have isScalarType, and just use it here.
clang/lib/CIR/Dialect/Transforms/TargetLowering/Targets/NVPTX.cpp
Outdated
Show resolved
Hide resolved
|
Seems like there are failing tests too |
AdUhTkJm
force-pushed
the
main
branch
2 times, most recently
from
41f78d0 to
01b8beb
Compare
bcardosolopes
changed the title
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
AdUhTkJm
force-pushed
the
main
branch
2 times, most recently
from
70af95d to
04a435e
Compare
bcardosolopes
approved these changes
Mar 17, 2025
lanza
pushed a commit
that referenced
this pull request
Mar 18, 2025
There are some subtleties here. This is the code in OG: ```cpp // note: this is different from default ABI if (!RetTy->isScalarType()) return ABIArgInfo::getDirect(); ``` which says we should return structs directly. It's correct, has have the same behaviour as `nvcc`, and it obeys the PTX ABI as well. The comment dates back to 2013 (see [this commit](llvm/llvm-project@f9329ff) -- it didn't provide any explanation either), so I believe it's outdated. I didn't include this comment in the PR.
terapines-osc-cir
pushed a commit
to Terapines/clangir
that referenced
this pull request
Sep 2, 2025
There are some subtleties here. This is the code in OG: ```cpp // note: this is different from default ABI if (!RetTy->isScalarType()) return ABIArgInfo::getDirect(); ``` which says we should return structs directly. It's correct, has have the same behaviour as `nvcc`, and it obeys the PTX ABI as well. The comment dates back to 2013 (see [this commit](llvm/llvm-project@f9329ff) -- it didn't provide any explanation either), so I believe it's outdated. I didn't include this comment in the PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are some subtleties here.
This is the code in OG:
which says we should return structs directly. It's correct, has have the same behaviour as
nvcc, and it obeys the PTX ABI as well.The comment dates back to 2013 (see this commit -- it didn't provide any explanation either), so I believe it's outdated. I didn't include this comment in the PR.