Skip to content

[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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jun 24, 2025

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.

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 :)
@nikic nikic requested a review from ChuanqiXu9 June 24, 2025 14:24
@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-coroutines

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/145518.diff

5 Files Affected:

  • (modified) llvm/lib/Transforms/Coroutines/CoroCleanup.cpp (+5-5)
  • (modified) llvm/lib/Transforms/Coroutines/CoroEarly.cpp (+6-5)
  • (modified) llvm/lib/Transforms/Coroutines/CoroElide.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Coroutines/CoroInternal.h (+1-2)
  • (modified) llvm/lib/Transforms/Coroutines/Coroutines.cpp (+40-55)
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;
 }
 

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

Successfully merging this pull request may close these issues.

2 participants