-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Coroutines] Work on intrinsic IDs instead of names (NFCI) #145518
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
For the checks whether certain intrinsics are used, work with intrinsic IDs instead of intrinsic names. This also exposes that some of the checks were incorrect, because the intrinsics were overloaded. The checks are still incorrect now (because these cannot be efficiently looked up without additional information), but at least we know about it :)
@llvm/pr-subscribers-coroutines @llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesFor the checks whether certain intrinsics are used, work with intrinsic IDs instead of intrinsic names. This also exposes that some of the checks were incorrect, because the intrinsics were overloaded. There is no efficient way to determine whether these are used. This change explicitly documents which intrinsics are not checked for this reason. Full diff: https://github.com/llvm/llvm-project/pull/145518.diff 5 Files Affected:
diff --git a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
index a0a26827aa09d..c00e9c7bbee06 100644
--- a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
@@ -110,11 +110,11 @@ bool Lowerer::lower(Function &F) {
static bool declaresCoroCleanupIntrinsics(const Module &M) {
return coro::declaresIntrinsics(
- M, {"llvm.coro.alloc", "llvm.coro.begin", "llvm.coro.subfn.addr",
- "llvm.coro.free", "llvm.coro.id", "llvm.coro.id.retcon",
- "llvm.coro.id.async", "llvm.coro.id.retcon.once",
- "llvm.coro.async.size.replace", "llvm.coro.async.resume",
- "llvm.coro.begin.custom.abi"});
+ M, {Intrinsic::coro_alloc, Intrinsic::coro_begin,
+ Intrinsic::coro_subfn_addr, Intrinsic::coro_free, Intrinsic::coro_id,
+ Intrinsic::coro_id_retcon, Intrinsic::coro_id_async,
+ Intrinsic::coro_id_retcon_once, Intrinsic::coro_async_size_replace,
+ Intrinsic::coro_async_resume, Intrinsic::coro_begin_custom_abi});
}
PreservedAnalyses CoroCleanupPass::run(Module &M,
diff --git a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
index 9ac145b10fe43..e279fec18bdbc 100644
--- a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
@@ -279,12 +279,13 @@ void Lowerer::lowerEarlyIntrinsics(Function &F) {
}
static bool declaresCoroEarlyIntrinsics(const Module &M) {
+ // coro_suspend omitted as it is overloaded.
return coro::declaresIntrinsics(
- M, {"llvm.coro.id", "llvm.coro.id.retcon", "llvm.coro.id.retcon.once",
- "llvm.coro.id.async", "llvm.coro.destroy", "llvm.coro.done",
- "llvm.coro.end", "llvm.coro.end.async", "llvm.coro.noop",
- "llvm.coro.free", "llvm.coro.promise", "llvm.coro.resume",
- "llvm.coro.suspend"});
+ M, {Intrinsic::coro_id, Intrinsic::coro_id_retcon,
+ Intrinsic::coro_id_retcon_once, Intrinsic::coro_id_async,
+ Intrinsic::coro_destroy, Intrinsic::coro_done, Intrinsic::coro_end,
+ Intrinsic::coro_end_async, Intrinsic::coro_noop, Intrinsic::coro_free,
+ Intrinsic::coro_promise, Intrinsic::coro_resume});
}
PreservedAnalyses CoroEarlyPass::run(Module &M, ModuleAnalysisManager &) {
diff --git a/llvm/lib/Transforms/Coroutines/CoroElide.cpp b/llvm/lib/Transforms/Coroutines/CoroElide.cpp
index c5131aa0787ed..1c8d4a8592d60 100644
--- a/llvm/lib/Transforms/Coroutines/CoroElide.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroElide.cpp
@@ -450,7 +450,7 @@ bool CoroIdElider::attemptElide() {
PreservedAnalyses CoroElidePass::run(Function &F, FunctionAnalysisManager &AM) {
auto &M = *F.getParent();
- if (!coro::declaresIntrinsics(M, {"llvm.coro.id"}))
+ if (!coro::declaresIntrinsics(M, Intrinsic::coro_id))
return PreservedAnalyses::all();
FunctionElideInfo FEI{&F};
diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
index a0b52063aca10..b53c5a48eb10b 100644
--- a/llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -24,8 +24,7 @@ namespace coro {
bool isSuspendBlock(BasicBlock *BB);
bool declaresAnyIntrinsic(const Module &M);
-bool declaresIntrinsics(const Module &M,
- const std::initializer_list<StringRef>);
+bool declaresIntrinsics(const Module &M, ArrayRef<Intrinsic::ID> List);
void replaceCoroFree(CoroIdInst *CoroId, bool Elide);
/// Replaces all @llvm.coro.alloc intrinsics calls associated with a given
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index 02500ff778b80..59ae057cae793 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -61,71 +61,56 @@ CallInst *coro::LowererBase::makeSubFnCall(Value *Arg, int Index,
return CallInst::Create(Fn, {Arg, IndexVal}, "", InsertPt->getIterator());
}
-// NOTE: Must be sorted!
-static const char *const CoroIntrinsics[] = {
- "llvm.coro.align",
- "llvm.coro.alloc",
- "llvm.coro.async.context.alloc",
- "llvm.coro.async.context.dealloc",
- "llvm.coro.async.resume",
- "llvm.coro.async.size.replace",
- "llvm.coro.await.suspend.bool",
- "llvm.coro.await.suspend.handle",
- "llvm.coro.await.suspend.void",
- "llvm.coro.begin",
- "llvm.coro.begin.custom.abi",
- "llvm.coro.destroy",
- "llvm.coro.done",
- "llvm.coro.end",
- "llvm.coro.end.async",
- "llvm.coro.frame",
- "llvm.coro.free",
- "llvm.coro.id",
- "llvm.coro.id.async",
- "llvm.coro.id.retcon",
- "llvm.coro.id.retcon.once",
- "llvm.coro.noop",
- "llvm.coro.prepare.async",
- "llvm.coro.prepare.retcon",
- "llvm.coro.promise",
- "llvm.coro.resume",
- "llvm.coro.save",
- "llvm.coro.size",
- "llvm.coro.subfn.addr",
- "llvm.coro.suspend",
- "llvm.coro.suspend.async",
- "llvm.coro.suspend.retcon",
+// We can only efficiently check for non-overloaded intrinsics.
+// The following intrinsics are absent for that reason:
+// coro_align, coro_size, coro_suspend_async, coro_suspend_retcon
+static Intrinsic::ID NonOverloadedCoroIntrinsics[] = {
+ Intrinsic::coro_alloc,
+ Intrinsic::coro_async_context_alloc,
+ Intrinsic::coro_async_context_dealloc,
+ Intrinsic::coro_async_resume,
+ Intrinsic::coro_async_size_replace,
+ Intrinsic::coro_await_suspend_bool,
+ Intrinsic::coro_await_suspend_handle,
+ Intrinsic::coro_await_suspend_void,
+ Intrinsic::coro_begin,
+ Intrinsic::coro_begin_custom_abi,
+ Intrinsic::coro_destroy,
+ Intrinsic::coro_done,
+ Intrinsic::coro_end,
+ Intrinsic::coro_end_async,
+ Intrinsic::coro_frame,
+ Intrinsic::coro_free,
+ Intrinsic::coro_id,
+ Intrinsic::coro_id_async,
+ Intrinsic::coro_id_retcon,
+ Intrinsic::coro_id_retcon_once,
+ Intrinsic::coro_promise,
+ Intrinsic::coro_resume,
+ Intrinsic::coro_save,
+ Intrinsic::coro_subfn_addr,
+ Intrinsic::coro_suspend,
};
-#ifndef NDEBUG
-static bool isCoroutineIntrinsicName(StringRef Name) {
- return llvm::binary_search(CoroIntrinsics, Name);
-}
-#endif
-
bool coro::isSuspendBlock(BasicBlock *BB) {
return isa<AnyCoroSuspendInst>(BB->front());
}
bool coro::declaresAnyIntrinsic(const Module &M) {
- for (StringRef Name : CoroIntrinsics) {
- if (M.getNamedValue(Name))
- return true;
- }
-
- return false;
+ return declaresIntrinsics(M, NonOverloadedCoroIntrinsics);
}
-// Verifies if a module has named values listed. Also, in debug mode verifies
-// that names are intrinsic names.
-bool coro::declaresIntrinsics(const Module &M,
- const std::initializer_list<StringRef> List) {
- for (StringRef Name : List) {
- assert(isCoroutineIntrinsicName(Name) && "not a coroutine intrinsic");
- if (M.getNamedValue(Name))
- return true;
- }
+// Checks whether the module declares any of the listed intrinsics.
+bool coro::declaresIntrinsics(const Module &M, ArrayRef<Intrinsic::ID> List) {
+#ifndef NDEBUG
+ for (Intrinsic::ID ID : List)
+ assert(!Intrinsic::isOverloaded(ID) &&
+ "Only non-overloaded intrinsics supported");
+#endif
+ for (Intrinsic::ID ID : List)
+ if (Intrinsic::getDeclarationIfExists(&M, ID))
+ return true;
return false;
}
|
For the checks whether certain intrinsics are used, work with intrinsic IDs instead of intrinsic names.
This also exposes that some of the checks were incorrect, because the intrinsics were overloaded. There is no efficient way to determine whether these are used. This change explicitly documents which intrinsics are not checked for this reason.