-
Notifications
You must be signed in to change notification settings - Fork 157
[CIR] Implement CGM.getIntrinsic
for LLVMCallIntrinsicOp
#1710
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
base: main
Are you sure you want to change the base?
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
eac2942
to
bba1ab7
Compare
…ntroduce `IntrinsicDescriptor` struct for better intrinsic management
bba1ab7
to
71f3dc1
Compare
return true; | ||
case llvm::Intrinsic::x86_rdtsc: | ||
case llvm::Intrinsic::x86_rdtscp: | ||
case llvm::Intrinsic::ctlz: |
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.
This was one of my points regarding integer signedness, as much as I think It's hard to track these Id's, It seems to be the only way so far...
cc. @bcardosolopes |
d2c4ab8
to
8f89224
Compare
I usually don't look at draft PRs so I didn't see this. Is it still relevant? Please publish when you feel it's ready or ping me on discord to take a look |
I worked on this a while ago. Need to polish, rethink and clarify the questions I had for these changes. I'll ping you when It's ready for review |
Related: #1692
The current implementation supports non overloaded intrinsics for now...
Couple of nits so far and limitations I've found as compared to CodeGen. Would like to get some guidance based on what I've done so far:
How should we handle intrinsic-specific module operations in CIR/MLIR that LLVM normally provides (naming, uniquing, lookups)? specifically since
LLVM::Module
implements:getOrInsertFunction
. Given that codegen represents these as functions and we have a dedicated op for intrinsics, how would the global -> intrinsic conversion work?Given the fact that we need to give signedness to integer types in CIR, the facilities designated to hold intrinsic metadata(
IITDescriptor
) dont contain any signedness. One approach I've taken is hardcoding and determining signedness based on the Intrinsic ID, however that could potentially be a burden to mantain.There's also some code duplication, specially in sections that handle with
IITDescriptors