[llvm] 2672947 - Revert "[Coroutines] ABI Objects to improve code separation between different ABIs, users and utilities. (#109338)"
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 20 14:23:40 PDT 2024
Author: Thurston Dang
Date: 2024-09-20T21:23:04Z
New Revision: 2672947633865c19c332c8e39f0d11e841e2480f
URL: https://github.com/llvm/llvm-project/commit/2672947633865c19c332c8e39f0d11e841e2480f
DIFF: https://github.com/llvm/llvm-project/commit/2672947633865c19c332c8e39f0d11e841e2480f.diff
LOG: Revert "[Coroutines] ABI Objects to improve code separation between different ABIs, users and utilities. (#109338)"
This reverts commit 2e414799d0ad511cd7999895014a2cae2ea5e3e3.
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.
Added:
Modified:
llvm/include/llvm/Transforms/Coroutines/CoroSplit.h
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/lib/Transforms/Coroutines/CoroInternal.h
llvm/lib/Transforms/Coroutines/CoroShape.h
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/lib/Transforms/Coroutines/Coroutines.cpp
Removed:
llvm/lib/Transforms/Coroutines/ABI.h
################################################################################
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroSplit.h b/llvm/include/llvm/Transforms/Coroutines/CoroSplit.h
index 74ee948a339e31..a2be1099ff68fc 100644
--- 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
deleted file mode 100644
index e6c60c51c40fac..00000000000000
--- a/llvm/lib/Transforms/Coroutines/ABI.h
+++ /dev/null
@@ -1,101 +0,0 @@
-//===- ABI.h - Coroutine lowering class definitions (ABIs) ----*- C++ -*---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-// This file defines coroutine lowering classes. The interface for coroutine
-// lowering is defined by BaseABI. Each lowering method (ABI) implements the
-// interface. Note that the enum class ABI, such as ABI::Switch, determines
-// which ABI class, such as SwitchABI, is used to lower the coroutine. Both the
-// ABI enum and ABI class are used by the Coroutine passes when lowering.
-//===----------------------------------------------------------------------===//
-
-#ifndef LIB_TRANSFORMS_COROUTINES_ABI_H
-#define LIB_TRANSFORMS_COROUTINES_ABI_H
-
-#include "CoroShape.h"
-#include "SuspendCrossingInfo.h"
-#include "llvm/Analysis/TargetTransformInfo.h"
-
-namespace llvm {
-
-class Function;
-
-namespace coro {
-
-// This interface/API is to provide an object oriented way to implement ABI
-// functionality. This is intended to replace use of the ABI enum to perform
-// ABI operations. The ABIs (e.g. Switch, Async, Retcon{Once}) are the common
-// ABIs.
-
-class LLVM_LIBRARY_VISIBILITY BaseABI {
-public:
- BaseABI(Function &F, coro::Shape &S,
- std::function<bool(Instruction &)> IsMaterializable)
- : F(F), Shape(S), IsMaterializable(IsMaterializable) {}
-
- // Initialize the coroutine ABI
- virtual void init() = 0;
-
- // Allocate the coroutine frame and do spill/reload as needed.
- virtual void buildCoroutineFrame();
-
- // Perform the function splitting according to the ABI.
- virtual void splitCoroutine(Function &F, coro::Shape &Shape,
- SmallVectorImpl<Function *> &Clones,
- TargetTransformInfo &TTI) = 0;
-
- Function &F;
- coro::Shape &Shape;
-
- // Callback used by coro::BaseABI::buildCoroutineFrame for rematerialization.
- // It is provided to coro::doMaterializations(..).
- std::function<bool(Instruction &I)> IsMaterializable;
-};
-
-class LLVM_LIBRARY_VISIBILITY SwitchABI : public BaseABI {
-public:
- SwitchABI(Function &F, coro::Shape &S,
- std::function<bool(Instruction &)> IsMaterializable)
- : BaseABI(F, S, IsMaterializable) {}
-
- void init() override;
-
- void splitCoroutine(Function &F, coro::Shape &Shape,
- SmallVectorImpl<Function *> &Clones,
- TargetTransformInfo &TTI) override;
-};
-
-class LLVM_LIBRARY_VISIBILITY AsyncABI : public BaseABI {
-public:
- AsyncABI(Function &F, coro::Shape &S,
- std::function<bool(Instruction &)> IsMaterializable)
- : BaseABI(F, S, IsMaterializable) {}
-
- void init() override;
-
- void splitCoroutine(Function &F, coro::Shape &Shape,
- SmallVectorImpl<Function *> &Clones,
- TargetTransformInfo &TTI) override;
-};
-
-class LLVM_LIBRARY_VISIBILITY AnyRetconABI : public BaseABI {
-public:
- AnyRetconABI(Function &F, coro::Shape &S,
- std::function<bool(Instruction &)> IsMaterializable)
- : BaseABI(F, S, IsMaterializable) {}
-
- void init() override;
-
- void splitCoroutine(Function &F, coro::Shape &Shape,
- SmallVectorImpl<Function *> &Clones,
- TargetTransformInfo &TTI) override;
-};
-
-} // end namespace coro
-
-} // end namespace llvm
-
-#endif // LLVM_TRANSFORMS_COROUTINES_ABI_H
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 021b1f7a4156b9..c08f56b024dfc7 100644
--- 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
index 88d0f83c98c9ec..fcbd31878bdea7 100644
--- 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
index f4fb4baa6df314..dcfe94ca076bd8 100644
--- 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
index 6fdead9a9c3501..382bdfff1926f7 100644
--- 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
index c7a04b8fd965b7..10e2e410960982 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -10,7 +10,6 @@
//
//===----------------------------------------------------------------------===//
-#include "ABI.h"
#include "CoroInstr.h"
#include "CoroInternal.h"
#include "CoroShape.h"
@@ -373,10 +372,11 @@ void coro::Shape::invalidateCoroutine(
}
}
-void coro::SwitchABI::init() {
- assert(Shape.ABI == coro::ABI::Switch);
- {
- for (auto *AnySuspend : Shape.CoroSuspends) {
+// Perform semantic checking and initialization of the ABI
+void coro::Shape::initABI() {
+ switch (ABI) {
+ case coro::ABI::Switch: {
+ for (auto *AnySuspend : CoroSuspends) {
auto Suspend = dyn_cast<CoroSuspendInst>(AnySuspend);
if (!Suspend) {
#ifndef NDEBUG
@@ -386,22 +386,21 @@ void coro::SwitchABI::init() {
}
if (!Suspend->getCoroSave())
- createCoroSave(Shape.CoroBegin, Suspend);
+ createCoroSave(CoroBegin, Suspend);
}
+ break;
}
-}
-
-void coro::AsyncABI::init() { assert(Shape.ABI == coro::ABI::Async); }
-
-void coro::AnyRetconABI::init() {
- assert(Shape.ABI == coro::ABI::Retcon || Shape.ABI == coro::ABI::RetconOnce);
- {
+ case coro::ABI::Async: {
+ break;
+ };
+ case coro::ABI::Retcon:
+ case coro::ABI::RetconOnce: {
// Determine the result value types, and make sure they match up with
// the values passed to the suspends.
- auto ResultTys = Shape.getRetconResultTypes();
- auto ResumeTys = Shape.getRetconResumeTypes();
+ auto ResultTys = getRetconResultTypes();
+ auto ResumeTys = getRetconResumeTypes();
- for (auto *AnySuspend : Shape.CoroSuspends) {
+ for (auto *AnySuspend : CoroSuspends) {
auto Suspend = dyn_cast<CoroSuspendRetconInst>(AnySuspend);
if (!Suspend) {
#ifndef NDEBUG
@@ -428,7 +427,7 @@ void coro::AnyRetconABI::init() {
#ifndef NDEBUG
Suspend->dump();
- Shape.RetconLowering.ResumePrototype->getFunctionType()->dump();
+ RetconLowering.ResumePrototype->getFunctionType()->dump();
#endif
report_fatal_error("argument to coro.suspend.retcon does not "
"match corresponding prototype function result");
@@ -437,7 +436,7 @@ void coro::AnyRetconABI::init() {
if (SI != SE || RI != RE) {
#ifndef NDEBUG
Suspend->dump();
- Shape.RetconLowering.ResumePrototype->getFunctionType()->dump();
+ RetconLowering.ResumePrototype->getFunctionType()->dump();
#endif
report_fatal_error("wrong number of arguments to coro.suspend.retcon");
}
@@ -456,7 +455,7 @@ void coro::AnyRetconABI::init() {
if (SuspendResultTys.size() != ResumeTys.size()) {
#ifndef NDEBUG
Suspend->dump();
- Shape.RetconLowering.ResumePrototype->getFunctionType()->dump();
+ RetconLowering.ResumePrototype->getFunctionType()->dump();
#endif
report_fatal_error("wrong number of results from coro.suspend.retcon");
}
@@ -464,13 +463,15 @@ void coro::AnyRetconABI::init() {
if (SuspendResultTys[I] != ResumeTys[I]) {
#ifndef NDEBUG
Suspend->dump();
- Shape.RetconLowering.ResumePrototype->getFunctionType()->dump();
+ RetconLowering.ResumePrototype->getFunctionType()->dump();
#endif
report_fatal_error("result from coro.suspend.retcon does not "
"match corresponding prototype function param");
}
}
}
+ break;
+ }
}
}
More information about the llvm-commits
mailing list