Reland "[CoroCleanup] Noop coroutine elision for load-and-call pattern (#179154)"#179431
Merged
Reland "[CoroCleanup] Noop coroutine elision for load-and-call pattern (#179154)"#179431
Conversation
Member
|
@llvm/pr-subscribers-coroutines Author: Weibo He (NewSigma) ChangesThere are multiple def-use paths from Reland #179154, which was reverted in #179289 due to ASan failures. Full diff: https://github.com/llvm/llvm-project/pull/179431.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
index 6b68cf5bc2c20..c44eaddd7ee55 100644
--- a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
@@ -8,6 +8,7 @@
#include "llvm/Transforms/Coroutines/CoroCleanup.h"
#include "CoroInternal.h"
+#include "llvm/Analysis/PtrUseVisitor.h"
#include "llvm/IR/DIBuilder.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
@@ -15,6 +16,7 @@
#include "llvm/IR/Module.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Transforms/Scalar/SimplifyCFG.h"
+#include "llvm/Transforms/Utils/Local.h"
using namespace llvm;
@@ -30,9 +32,28 @@ struct Lowerer : coro::LowererBase {
bool lower(Function &F);
private:
- void elideCoroNoop(IntrinsicInst *II);
void lowerCoroNoop(IntrinsicInst *II);
};
+
+// Recursively walk and eliminate resume/destroy call on noop coro
+class NoopCoroElider : public PtrUseVisitor<NoopCoroElider> {
+ using Base = PtrUseVisitor<NoopCoroElider>;
+
+ IRBuilder<> Builder;
+
+public:
+ NoopCoroElider(const DataLayout &DL, LLVMContext &C) : Base(DL), Builder(C) {}
+
+ void run(IntrinsicInst *II);
+
+ void visitLoadInst(LoadInst &I) { enqueueUsers(I); }
+ void visitCallBase(CallBase &CB);
+ void visitIntrinsicInst(IntrinsicInst &II);
+
+private:
+ bool tryEraseCallInvoke(Instruction *I);
+ void eraseFromWorklist(Instruction *I);
+};
}
static void lowerSubFn(IRBuilder<> &Builder, CoroSubFnInst *SubFn) {
@@ -73,6 +94,7 @@ bool Lowerer::lower(Function &F) {
bool IsPrivateAndUnprocessed = F.isPresplitCoroutine() && F.hasLocalLinkage();
bool Changed = false;
+ NoopCoroElider NCE(F.getDataLayout(), F.getContext());
SmallPtrSet<Instruction *, 8> DeadInsts{};
for (Instruction &I : instructions(F)) {
if (auto *II = dyn_cast<IntrinsicInst>(&I)) {
@@ -100,7 +122,7 @@ bool Lowerer::lower(Function &F) {
II->replaceAllUsesWith(ConstantTokenNone::get(Context));
break;
case Intrinsic::coro_noop:
- elideCoroNoop(II);
+ NCE.run(II);
if (!II->user_empty())
lowerCoroNoop(II);
break;
@@ -142,28 +164,6 @@ bool Lowerer::lower(Function &F) {
return Changed;
}
-void Lowerer::elideCoroNoop(IntrinsicInst *II) {
- for (User *U : make_early_inc_range(II->users())) {
- auto *Fn = dyn_cast<CoroSubFnInst>(U);
- if (Fn == nullptr)
- continue;
-
- auto *User = Fn->getUniqueUndroppableUser();
- if (auto *Call = dyn_cast<CallInst>(User)) {
- Call->eraseFromParent();
- Fn->eraseFromParent();
- continue;
- }
-
- if (auto *I = dyn_cast<InvokeInst>(User)) {
- Builder.SetInsertPoint(I);
- Builder.CreateBr(I->getNormalDest());
- I->eraseFromParent();
- Fn->eraseFromParent();
- }
- }
-}
-
void Lowerer::lowerCoroNoop(IntrinsicInst *II) {
if (!NoopCoro) {
LLVMContext &C = Builder.getContext();
@@ -200,6 +200,60 @@ void Lowerer::lowerCoroNoop(IntrinsicInst *II) {
II->replaceAllUsesWith(NoopCoroVoidPtr);
}
+void NoopCoroElider::run(IntrinsicInst *II) {
+ visitPtr(*II);
+
+ Worklist.clear();
+ VisitedUses.clear();
+}
+
+void NoopCoroElider::visitCallBase(CallBase &CB) {
+ auto *V = U->get();
+ bool ResumeOrDestroy = V == CB.getCalledOperand();
+ if (ResumeOrDestroy) {
+ [[maybe_unused]] bool Success = tryEraseCallInvoke(&CB);
+ assert(Success && "Unexpected CallBase");
+
+ auto AboutToDeleteCallback = [this](Value *V) {
+ eraseFromWorklist(cast<Instruction>(V));
+ };
+ RecursivelyDeleteTriviallyDeadInstructions(V, nullptr, nullptr,
+ AboutToDeleteCallback);
+ }
+}
+
+void NoopCoroElider::visitIntrinsicInst(IntrinsicInst &II) {
+ if (auto *SubFn = dyn_cast<CoroSubFnInst>(&II)) {
+ auto *User = SubFn->getUniqueUndroppableUser();
+ if (!tryEraseCallInvoke(cast<Instruction>(User)))
+ return;
+ SubFn->eraseFromParent();
+ }
+}
+
+bool NoopCoroElider::tryEraseCallInvoke(Instruction *I) {
+ if (auto *Call = dyn_cast<CallInst>(I)) {
+ eraseFromWorklist(Call);
+ Call->eraseFromParent();
+ return true;
+ }
+
+ if (auto *II = dyn_cast<InvokeInst>(I)) {
+ Builder.SetInsertPoint(II);
+ Builder.CreateBr(II->getNormalDest());
+ eraseFromWorklist(II);
+ II->eraseFromParent();
+ return true;
+ }
+ return false;
+}
+
+void NoopCoroElider::eraseFromWorklist(Instruction *I) {
+ erase_if(Worklist, [I](UseToVisit &U) {
+ return I == U.UseAndIsOffsetKnown.getPointer()->getUser();
+ });
+}
+
static bool declaresCoroCleanupIntrinsics(const Module &M) {
return coro::declaresIntrinsics(
M,
diff --git a/llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll b/llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll
new file mode 100644
index 0000000000000..6d9dd654b914a
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll
@@ -0,0 +1,51 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -S -passes='coro-cleanup' | FileCheck %s
+
+; Tests that resume or destroy a no-op coroutine can be erased; Finally, erase coro.noop if it has no users.
+define void @erase() personality i32 0 {
+; CHECK-LABEL: define void @erase() personality i32 0 {
+; CHECK-NEXT: [[DONE:.*:]]
+; CHECK-NEXT: ret void
+;
+ %frame = call noundef ptr @llvm.coro.noop()
+ %resume = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 0)
+ call fastcc void %resume(ptr %frame)
+ %destroy = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 1)
+ invoke fastcc void %destroy(ptr %frame)
+ to label %done unwind label %unwind
+
+done:
+ ret void
+
+unwind:
+ %pad = landingpad { ptr, i32 }
+ catch ptr null
+ call void @terminate()
+ unreachable
+}
+
+; Tests the load-and-call pattern despite mismatched calling conventions. Prevent instcombine from breaking code.
+define void @load() personality i32 0 {
+; CHECK-LABEL: define void @load() personality i32 0 {
+; CHECK-NEXT: [[DONE:.*:]]
+; CHECK-NEXT: ret void
+;
+ %frame = call noundef ptr @llvm.coro.noop()
+ %resume = load ptr, ptr %frame, align 8
+ call void %resume(ptr %frame)
+ %destroy.addr = getelementptr inbounds nuw i8, ptr %frame, i64 8
+ %destroy = load ptr, ptr %destroy.addr, align 8
+ invoke void %destroy(ptr %frame)
+ to label %done unwind label %unwind
+
+done:
+ ret void
+
+unwind:
+ %pad = landingpad { ptr, i32 }
+ catch ptr null
+ call void @terminate()
+ unreachable
+}
+
+declare void @terminate() noreturn
diff --git a/llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll b/llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll
deleted file mode 100644
index 7fd9dc900ddb2..0000000000000
--- a/llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll
+++ /dev/null
@@ -1,24 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
-; Tests that resume or destroy a no-op coroutine can be erased; Finally, erase coro.noop if it has no users.
-; RUN: opt < %s -S -passes='coro-cleanup' | FileCheck %s
-
-define void @fn() personality i32 0 {
-; CHECK-LABEL: define void @fn() personality i32 0 {
-; CHECK-NEXT: [[DONE:.*:]]
-; CHECK-NEXT: ret void
-;
- %frame = call noundef ptr @llvm.coro.noop()
- %resume = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 0)
- call fastcc void %resume(ptr %frame)
- %destroy = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 1)
- invoke fastcc void %destroy(ptr %frame)
- to label %done unwind label %unwind
-
-done:
- ret void
-
-unwind:
- %pad = landingpad { ptr, i32 }
- catch ptr null
- unreachable
-}
|
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Weibo He (NewSigma) ChangesThere are multiple def-use paths from Reland #179154, which was reverted in #179289 due to ASan failures. Full diff: https://github.com/llvm/llvm-project/pull/179431.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
index 6b68cf5bc2c20..c44eaddd7ee55 100644
--- a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
@@ -8,6 +8,7 @@
#include "llvm/Transforms/Coroutines/CoroCleanup.h"
#include "CoroInternal.h"
+#include "llvm/Analysis/PtrUseVisitor.h"
#include "llvm/IR/DIBuilder.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
@@ -15,6 +16,7 @@
#include "llvm/IR/Module.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Transforms/Scalar/SimplifyCFG.h"
+#include "llvm/Transforms/Utils/Local.h"
using namespace llvm;
@@ -30,9 +32,28 @@ struct Lowerer : coro::LowererBase {
bool lower(Function &F);
private:
- void elideCoroNoop(IntrinsicInst *II);
void lowerCoroNoop(IntrinsicInst *II);
};
+
+// Recursively walk and eliminate resume/destroy call on noop coro
+class NoopCoroElider : public PtrUseVisitor<NoopCoroElider> {
+ using Base = PtrUseVisitor<NoopCoroElider>;
+
+ IRBuilder<> Builder;
+
+public:
+ NoopCoroElider(const DataLayout &DL, LLVMContext &C) : Base(DL), Builder(C) {}
+
+ void run(IntrinsicInst *II);
+
+ void visitLoadInst(LoadInst &I) { enqueueUsers(I); }
+ void visitCallBase(CallBase &CB);
+ void visitIntrinsicInst(IntrinsicInst &II);
+
+private:
+ bool tryEraseCallInvoke(Instruction *I);
+ void eraseFromWorklist(Instruction *I);
+};
}
static void lowerSubFn(IRBuilder<> &Builder, CoroSubFnInst *SubFn) {
@@ -73,6 +94,7 @@ bool Lowerer::lower(Function &F) {
bool IsPrivateAndUnprocessed = F.isPresplitCoroutine() && F.hasLocalLinkage();
bool Changed = false;
+ NoopCoroElider NCE(F.getDataLayout(), F.getContext());
SmallPtrSet<Instruction *, 8> DeadInsts{};
for (Instruction &I : instructions(F)) {
if (auto *II = dyn_cast<IntrinsicInst>(&I)) {
@@ -100,7 +122,7 @@ bool Lowerer::lower(Function &F) {
II->replaceAllUsesWith(ConstantTokenNone::get(Context));
break;
case Intrinsic::coro_noop:
- elideCoroNoop(II);
+ NCE.run(II);
if (!II->user_empty())
lowerCoroNoop(II);
break;
@@ -142,28 +164,6 @@ bool Lowerer::lower(Function &F) {
return Changed;
}
-void Lowerer::elideCoroNoop(IntrinsicInst *II) {
- for (User *U : make_early_inc_range(II->users())) {
- auto *Fn = dyn_cast<CoroSubFnInst>(U);
- if (Fn == nullptr)
- continue;
-
- auto *User = Fn->getUniqueUndroppableUser();
- if (auto *Call = dyn_cast<CallInst>(User)) {
- Call->eraseFromParent();
- Fn->eraseFromParent();
- continue;
- }
-
- if (auto *I = dyn_cast<InvokeInst>(User)) {
- Builder.SetInsertPoint(I);
- Builder.CreateBr(I->getNormalDest());
- I->eraseFromParent();
- Fn->eraseFromParent();
- }
- }
-}
-
void Lowerer::lowerCoroNoop(IntrinsicInst *II) {
if (!NoopCoro) {
LLVMContext &C = Builder.getContext();
@@ -200,6 +200,60 @@ void Lowerer::lowerCoroNoop(IntrinsicInst *II) {
II->replaceAllUsesWith(NoopCoroVoidPtr);
}
+void NoopCoroElider::run(IntrinsicInst *II) {
+ visitPtr(*II);
+
+ Worklist.clear();
+ VisitedUses.clear();
+}
+
+void NoopCoroElider::visitCallBase(CallBase &CB) {
+ auto *V = U->get();
+ bool ResumeOrDestroy = V == CB.getCalledOperand();
+ if (ResumeOrDestroy) {
+ [[maybe_unused]] bool Success = tryEraseCallInvoke(&CB);
+ assert(Success && "Unexpected CallBase");
+
+ auto AboutToDeleteCallback = [this](Value *V) {
+ eraseFromWorklist(cast<Instruction>(V));
+ };
+ RecursivelyDeleteTriviallyDeadInstructions(V, nullptr, nullptr,
+ AboutToDeleteCallback);
+ }
+}
+
+void NoopCoroElider::visitIntrinsicInst(IntrinsicInst &II) {
+ if (auto *SubFn = dyn_cast<CoroSubFnInst>(&II)) {
+ auto *User = SubFn->getUniqueUndroppableUser();
+ if (!tryEraseCallInvoke(cast<Instruction>(User)))
+ return;
+ SubFn->eraseFromParent();
+ }
+}
+
+bool NoopCoroElider::tryEraseCallInvoke(Instruction *I) {
+ if (auto *Call = dyn_cast<CallInst>(I)) {
+ eraseFromWorklist(Call);
+ Call->eraseFromParent();
+ return true;
+ }
+
+ if (auto *II = dyn_cast<InvokeInst>(I)) {
+ Builder.SetInsertPoint(II);
+ Builder.CreateBr(II->getNormalDest());
+ eraseFromWorklist(II);
+ II->eraseFromParent();
+ return true;
+ }
+ return false;
+}
+
+void NoopCoroElider::eraseFromWorklist(Instruction *I) {
+ erase_if(Worklist, [I](UseToVisit &U) {
+ return I == U.UseAndIsOffsetKnown.getPointer()->getUser();
+ });
+}
+
static bool declaresCoroCleanupIntrinsics(const Module &M) {
return coro::declaresIntrinsics(
M,
diff --git a/llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll b/llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll
new file mode 100644
index 0000000000000..6d9dd654b914a
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll
@@ -0,0 +1,51 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -S -passes='coro-cleanup' | FileCheck %s
+
+; Tests that resume or destroy a no-op coroutine can be erased; Finally, erase coro.noop if it has no users.
+define void @erase() personality i32 0 {
+; CHECK-LABEL: define void @erase() personality i32 0 {
+; CHECK-NEXT: [[DONE:.*:]]
+; CHECK-NEXT: ret void
+;
+ %frame = call noundef ptr @llvm.coro.noop()
+ %resume = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 0)
+ call fastcc void %resume(ptr %frame)
+ %destroy = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 1)
+ invoke fastcc void %destroy(ptr %frame)
+ to label %done unwind label %unwind
+
+done:
+ ret void
+
+unwind:
+ %pad = landingpad { ptr, i32 }
+ catch ptr null
+ call void @terminate()
+ unreachable
+}
+
+; Tests the load-and-call pattern despite mismatched calling conventions. Prevent instcombine from breaking code.
+define void @load() personality i32 0 {
+; CHECK-LABEL: define void @load() personality i32 0 {
+; CHECK-NEXT: [[DONE:.*:]]
+; CHECK-NEXT: ret void
+;
+ %frame = call noundef ptr @llvm.coro.noop()
+ %resume = load ptr, ptr %frame, align 8
+ call void %resume(ptr %frame)
+ %destroy.addr = getelementptr inbounds nuw i8, ptr %frame, i64 8
+ %destroy = load ptr, ptr %destroy.addr, align 8
+ invoke void %destroy(ptr %frame)
+ to label %done unwind label %unwind
+
+done:
+ ret void
+
+unwind:
+ %pad = landingpad { ptr, i32 }
+ catch ptr null
+ call void @terminate()
+ unreachable
+}
+
+declare void @terminate() noreturn
diff --git a/llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll b/llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll
deleted file mode 100644
index 7fd9dc900ddb2..0000000000000
--- a/llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll
+++ /dev/null
@@ -1,24 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
-; Tests that resume or destroy a no-op coroutine can be erased; Finally, erase coro.noop if it has no users.
-; RUN: opt < %s -S -passes='coro-cleanup' | FileCheck %s
-
-define void @fn() personality i32 0 {
-; CHECK-LABEL: define void @fn() personality i32 0 {
-; CHECK-NEXT: [[DONE:.*:]]
-; CHECK-NEXT: ret void
-;
- %frame = call noundef ptr @llvm.coro.noop()
- %resume = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 0)
- call fastcc void %resume(ptr %frame)
- %destroy = call ptr @llvm.coro.subfn.addr(ptr %frame, i8 1)
- invoke fastcc void %destroy(ptr %frame)
- to label %done unwind label %unwind
-
-done:
- ret void
-
-unwind:
- %pad = landingpad { ptr, i32 }
- catch ptr null
- unreachable
-}
|
Contributor
|
Thanks for the fix @NewSigma !! |
bgra8
approved these changes
Feb 3, 2026
moar55
pushed a commit
to moar55/llvm-project
that referenced
this pull request
Feb 3, 2026
llvm#179154)" (llvm#179431) There are multiple def-use paths from `coro.noop` to the calls of the resume/destroy functions. We should update the worklist before erasing instructions. The reversion passed tests with ASan enabled. Reland llvm#179154, which was reverted in llvm#179289 due to ASan failures.
rishabhmadan19
pushed a commit
to rishabhmadan19/llvm-project
that referenced
this pull request
Feb 9, 2026
llvm#179154)" (llvm#179431) There are multiple def-use paths from `coro.noop` to the calls of the resume/destroy functions. We should update the worklist before erasing instructions. The reversion passed tests with ASan enabled. Reland llvm#179154, which was reverted in llvm#179289 due to ASan failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There are multiple def-use paths from
coro.noopto the calls of the resume/destroy functions. We should update the worklist before erasing instructions. The reversion passed tests with ASan enabled.Reland #179154, which was reverted in #179289 due to ASan failures.