Skip to content

[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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RiverDave
Copy link
Contributor

@RiverDave RiverDave commented Jun 23, 2025

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:

  1. 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?

  2. 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.

  3. There's also some code duplication, specially in sections that handle with IITDescriptors

Copy link

github-actions bot commented Jun 23, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@RiverDave RiverDave force-pushed the dave/get-intrinsic branch 3 times, most recently from eac2942 to bba1ab7 Compare June 23, 2025 22:20
…ntroduce `IntrinsicDescriptor` struct for better intrinsic management
@RiverDave RiverDave force-pushed the dave/get-intrinsic branch from bba1ab7 to 71f3dc1 Compare June 23, 2025 23:20
return true;
case llvm::Intrinsic::x86_rdtsc:
case llvm::Intrinsic::x86_rdtscp:
case llvm::Intrinsic::ctlz:
Copy link
Contributor Author

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...

@RiverDave
Copy link
Contributor Author

cc. @bcardosolopes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant