Revert "[Coroutines] ABI Objects to improve code separation between different ABIs, users and utilities. (#109338)"

thurstond · thurstond · commit 267294763386 · 2024-09-20T21:23:04.000Z
This reverts commit 2e41479. Reason: buildbot breakage (https://lab.llvm.org/buildbot/#/builders/51/builds/4105) (This was the only new CL.) /home/b/sanitizer-aarch64-linux/build/llvm-project/llvm/lib/Transforms/Coroutines/ABI.h:71:31: error: 'llvm::coro::AsyncABI' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor] 71 | class LLVM_LIBRARY_VISIBILITY AsyncABI : public BaseABI { etc.
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroSplit.h b/llvm/include/llvm/Transforms/Coroutines/CoroSplit.h
@@ -21,26 +21,19 @@
 
 namespace llvm {
 
-namespace coro {
-class BaseABI;
-class Shape;
-} // namespace coro
-
 struct CoroSplitPass : PassInfoMixin<CoroSplitPass> {
+  const std::function<bool(Instruction &)> MaterializableCallback;
 
   CoroSplitPass(bool OptimizeFrame = false);
   CoroSplitPass(std::function<bool(Instruction &)> MaterializableCallback,
-                bool OptimizeFrame = false);
+                bool OptimizeFrame = false)
+      : MaterializableCallback(MaterializableCallback),
+        OptimizeFrame(OptimizeFrame) {}
 
   PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
                         LazyCallGraph &CG, CGSCCUpdateResult &UR);
   static bool isRequired() { return true; }
 
-  using BaseABITy =
-      std::function<std::unique_ptr<coro::BaseABI>(Function &, coro::Shape &)>;
-  // Generator for an ABI transformer
-  BaseABITy CreateAndInitABI;
-
   // Would be true if the Optimization level isn't O0.
   bool OptimizeFrame;
 };
diff --git a/llvm/lib/Transforms/Coroutines/ABI.h b/llvm/lib/Transforms/Coroutines/ABI.h
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -15,7 +15,6 @@
 // the value into the coroutine frame.
 //===----------------------------------------------------------------------===//
 
-#include "ABI.h"
 #include "CoroInternal.h"
 #include "MaterializationUtils.h"
 #include "SpillUtils.h"
@@ -2056,9 +2055,11 @@ void coro::normalizeCoroutine(Function &F, coro::Shape &Shape,
   rewritePHIs(F);
 }
 
-void coro::BaseABI::buildCoroutineFrame() {
+void coro::buildCoroutineFrame(
+    Function &F, Shape &Shape,
+    const std::function<bool(Instruction &)> &MaterializableCallback) {
   SuspendCrossingInfo Checker(F, Shape.CoroSuspends, Shape.CoroEnds);
-  doRematerializations(F, Checker, IsMaterializable);
+  doRematerializations(F, Checker, MaterializableCallback);
 
   const DominatorTree DT(F);
   if (Shape.ABI != coro::ABI::Async && Shape.ABI != coro::ABI::Retcon &&
diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -62,6 +62,9 @@ struct LowererBase {
 bool defaultMaterializable(Instruction &V);
 void normalizeCoroutine(Function &F, coro::Shape &Shape,
                         TargetTransformInfo &TTI);
+void buildCoroutineFrame(
+    Function &F, Shape &Shape,
+    const std::function<bool(Instruction &)> &MaterializableCallback);
 CallInst *createMustTailCall(DebugLoc Loc, Function *MustTailCallFn,
                              TargetTransformInfo &TTI,
                              ArrayRef<Value *> Arguments, IRBuilder<> &);
diff --git a/llvm/lib/Transforms/Coroutines/CoroShape.h b/llvm/lib/Transforms/Coroutines/CoroShape.h
@@ -275,6 +275,7 @@ struct LLVM_LIBRARY_VISIBILITY Shape {
       invalidateCoroutine(F, CoroFrames);
       return;
     }
+    initABI();
     cleanCoroutine(CoroFrames, UnusedCoroSaves);
   }
 };
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -19,10 +19,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Coroutines/CoroSplit.h"
-#include "ABI.h"
 #include "CoroInstr.h"
 #include "CoroInternal.h"
-#include "MaterializationUtils.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/PriorityWorklist.h"
 #include "llvm/ADT/SmallPtrSet.h"
@@ -1781,9 +1779,9 @@ CallInst *coro::createMustTailCall(DebugLoc Loc, Function *MustTailCallFn,
   return TailCall;
 }
 
-void coro::AsyncABI::splitCoroutine(Function &F, coro::Shape &Shape,
-                                    SmallVectorImpl<Function *> &Clones,
-                                    TargetTransformInfo &TTI) {
+static void splitAsyncCoroutine(Function &F, coro::Shape &Shape,
+                                SmallVectorImpl<Function *> &Clones,
+                                TargetTransformInfo &TTI) {
   assert(Shape.ABI == coro::ABI::Async);
   assert(Clones.empty());
   // Reset various things that the optimizer might have decided it
@@ -1876,9 +1874,9 @@ void coro::AsyncABI::splitCoroutine(Function &F, coro::Shape &Shape,
   }
 }
 
-void coro::AnyRetconABI::splitCoroutine(Function &F, coro::Shape &Shape,
-                                        SmallVectorImpl<Function *> &Clones,
-                                        TargetTransformInfo &TTI) {
+static void splitRetconCoroutine(Function &F, coro::Shape &Shape,
+                                 SmallVectorImpl<Function *> &Clones,
+                                 TargetTransformInfo &TTI) {
   assert(Shape.ABI == coro::ABI::Retcon || Shape.ABI == coro::ABI::RetconOnce);
   assert(Clones.empty());
 
@@ -2046,27 +2044,26 @@ static bool hasSafeElideCaller(Function &F) {
   return false;
 }
 
-void coro::SwitchABI::splitCoroutine(Function &F, coro::Shape &Shape,
-                                     SmallVectorImpl<Function *> &Clones,
-                                     TargetTransformInfo &TTI) {
-  SwitchCoroutineSplitter::split(F, Shape, Clones, TTI);
-}
-
-static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
-                             coro::BaseABI &ABI, TargetTransformInfo &TTI) {
+static coro::Shape
+splitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
+               TargetTransformInfo &TTI, bool OptimizeFrame,
+               std::function<bool(Instruction &)> MaterializableCallback) {
   PrettyStackTraceFunction prettyStackTrace(F);
 
-  auto &Shape = ABI.Shape;
-  assert(Shape.CoroBegin);
+  // The suspend-crossing algorithm in buildCoroutineFrame get tripped
+  // up by uses in unreachable blocks, so remove them as a first pass.
+  removeUnreachableBlocks(F);
+
+  coro::Shape Shape(F, OptimizeFrame);
+  if (!Shape.CoroBegin)
+    return Shape;
 
   lowerAwaitSuspends(F, Shape);
 
   simplifySuspendPoints(Shape);
-
   normalizeCoroutine(F, Shape, TTI);
-  ABI.buildCoroutineFrame();
+  buildCoroutineFrame(F, Shape, MaterializableCallback);
   replaceFrameSizeAndAlignment(Shape);
-
   bool isNoSuspendCoroutine = Shape.CoroSuspends.empty();
 
   bool shouldCreateNoAllocVariant = !isNoSuspendCoroutine &&
@@ -2078,7 +2075,18 @@ static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
   if (isNoSuspendCoroutine) {
     handleNoSuspendCoroutine(Shape);
   } else {
-    ABI.splitCoroutine(F, Shape, Clones, TTI);
+    switch (Shape.ABI) {
+    case coro::ABI::Switch:
+      SwitchCoroutineSplitter::split(F, Shape, Clones, TTI);
+      break;
+    case coro::ABI::Async:
+      splitAsyncCoroutine(F, Shape, Clones, TTI);
+      break;
+    case coro::ABI::Retcon:
+    case coro::ABI::RetconOnce:
+      splitRetconCoroutine(F, Shape, Clones, TTI);
+      break;
+    }
   }
 
   // Replace all the swifterror operations in the original function.
@@ -2099,6 +2107,8 @@ static void doSplitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
 
   if (shouldCreateNoAllocVariant)
     SwitchCoroutineSplitter::createNoAllocVariant(F, Shape, Clones);
+
+  return Shape;
 }
 
 static LazyCallGraph::SCC &updateCallGraphAfterCoroutineSplit(
@@ -2197,44 +2207,8 @@ static void addPrepareFunction(const Module &M,
     Fns.push_back(PrepareFn);
 }
 
-static std::unique_ptr<coro::BaseABI>
-CreateNewABI(Function &F, coro::Shape &S,
-             std::function<bool(Instruction &)> IsMatCallback) {
-  switch (S.ABI) {
-  case coro::ABI::Switch:
-    return std::unique_ptr<coro::BaseABI>(
-        new coro::SwitchABI(F, S, IsMatCallback));
-  case coro::ABI::Async:
-    return std::unique_ptr<coro::BaseABI>(
-        new coro::AsyncABI(F, S, IsMatCallback));
-  case coro::ABI::Retcon:
-    return std::unique_ptr<coro::BaseABI>(
-        new coro::AnyRetconABI(F, S, IsMatCallback));
-  case coro::ABI::RetconOnce:
-    return std::unique_ptr<coro::BaseABI>(
-        new coro::AnyRetconABI(F, S, IsMatCallback));
-  }
-  llvm_unreachable("Unknown ABI");
-}
-
 CoroSplitPass::CoroSplitPass(bool OptimizeFrame)
-    : CreateAndInitABI([](Function &F, coro::Shape &S) {
-        std::unique_ptr<coro::BaseABI> ABI =
-            CreateNewABI(F, S, coro::isTriviallyMaterializable);
-        ABI->init();
-        return std::move(ABI);
-      }),
-      OptimizeFrame(OptimizeFrame) {}
-
-// For back compatibility, constructor takes a materializable callback and
-// creates a generator for an ABI with a modified materializable callback.
-CoroSplitPass::CoroSplitPass(std::function<bool(Instruction &)> IsMatCallback,
-                             bool OptimizeFrame)
-    : CreateAndInitABI([=](Function &F, coro::Shape &S) {
-        std::unique_ptr<coro::BaseABI> ABI = CreateNewABI(F, S, IsMatCallback);
-        ABI->init();
-        return std::move(ABI);
-      }),
+    : MaterializableCallback(coro::defaultMaterializable),
       OptimizeFrame(OptimizeFrame) {}
 
 PreservedAnalyses CoroSplitPass::run(LazyCallGraph::SCC &C,
@@ -2267,23 +2241,12 @@ PreservedAnalyses CoroSplitPass::run(LazyCallGraph::SCC &C,
     Function &F = N->getFunction();
     LLVM_DEBUG(dbgs() << "CoroSplit: Processing coroutine '" << F.getName()
                       << "\n");
-
-    // The suspend-crossing algorithm in buildCoroutineFrame gets tripped up
-    // by unreachable blocks, so remove them as a first pass. Remove the
-    // unreachable blocks before collecting intrinsics into Shape.
-    removeUnreachableBlocks(F);
-
-    coro::Shape Shape(F, OptimizeFrame);
-    if (!Shape.CoroBegin)
-      continue;
-
     F.setSplittedCoroutine();
 
-    std::unique_ptr<coro::BaseABI> ABI = CreateAndInitABI(F, Shape);
-
     SmallVector<Function *, 4> Clones;
-    auto &TTI = FAM.getResult<TargetIRAnalysis>(F);
-    doSplitCoroutine(F, Clones, *ABI, TTI);
+    coro::Shape Shape =
+        splitCoroutine(F, Clones, FAM.getResult<TargetIRAnalysis>(F),
+                       OptimizeFrame, MaterializableCallback);
     CurrentSCC = &updateCallGraphAfterCoroutineSplit(
         *N, Shape, Clones, *CurrentSCC, CG, AM, UR, FAM);
 
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp

Original file line number	Diff line number	Diff line change
`@@ -275,6 +275,7 @@ struct LLVM_LIBRARY_VISIBILITY Shape {`
`275`	`275`	`invalidateCoroutine(F, CoroFrames);`
`276`	`276`	`return;`
`277`	`277`	`}`
	`278`	`+ initABI();`
`278`	`279`	`cleanCoroutine(CoroFrames, UnusedCoroSaves);`
`279`	`280`	`}`
`280`	`281`	`};`