Revert "[CoroCleanup] Noop coroutine elision for load-and-call pattern"

[NewSigma]

There is a CI fail. Reverts #179154

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-coroutines

Author: Weibo He (NewSigma)

Changes

There is a CI fail. Reverts llvm/llvm-project#179154

---

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

3 Files Affected:

• (modified) llvm/lib/Transforms/Coroutines/CoroCleanup.cpp (+24-64)
• (removed) llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll (-51)
• (added) llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll (+24)

diff --git a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
index fa342fd4717ea..6b68cf5bc2c20 100644
--- a/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp
@@ -8,7 +8,6 @@
 
 #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"
@@ -16,7 +15,6 @@
 #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;
 
@@ -32,27 +30,9 @@ 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);
-};
 }
 
 static void lowerSubFn(IRBuilder<> &Builder, CoroSubFnInst *SubFn) {
@@ -93,7 +73,6 @@ 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)) {
@@ -121,7 +100,7 @@ bool Lowerer::lower(Function &F) {
         II->replaceAllUsesWith(ConstantTokenNone::get(Context));
         break;
       case Intrinsic::coro_noop:
-        NCE.run(II);
+        elideCoroNoop(II);
         if (!II->user_empty())
           lowerCoroNoop(II);
         break;
@@ -163,6 +142,28 @@ 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();
@@ -199,47 +200,6 @@ 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");
-    RecursivelyDeleteTriviallyDeadInstructions(V);
-  }
-}
-
-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)) {
-    Call->eraseFromParent();
-    return true;
-  }
-
-  if (auto *II = dyn_cast<InvokeInst>(I)) {
-    Builder.SetInsertPoint(II);
-    Builder.CreateBr(II->getNormalDest());
-    II->eraseFromParent();
-    return true;
-  }
-  return false;
-}
-
 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
deleted file mode 100644
index 6d9dd654b914a..0000000000000
--- a/llvm/test/Transforms/Coroutines/coro-cleanup-noop-elide.ll
+++ /dev/null
@@ -1,51 +0,0 @@
-; 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
new file mode 100644
index 0000000000000..7fd9dc900ddb2
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-cleanup-noop-erase.ll
@@ -0,0 +1,24 @@
+; 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
+}