From 3858dbdef80a389951a4a7d691864ee31bf68f42 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 24 Jun 2025 16:20:45 +0200 Subject: [PATCH] [Coroutines] Work on intrinsic IDs instead of names (NFCI) 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 :) --- .../lib/Transforms/Coroutines/CoroCleanup.cpp | 10 +- llvm/lib/Transforms/Coroutines/CoroEarly.cpp | 11 ++- llvm/lib/Transforms/Coroutines/CoroElide.cpp | 2 +- llvm/lib/Transforms/Coroutines/CoroInternal.h | 3 +- llvm/lib/Transforms/Coroutines/Coroutines.cpp | 95 ++++++++----------- 5 files changed, 53 insertions(+), 68 deletions(-) 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); +bool declaresIntrinsics(const Module &M, ArrayRef 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(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 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 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; }