[llvm] [llvm/llvm-project][Coroutines] ABI Object (PR #106306)
Tyler Nowicki via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 27 16:20:08 PDT 2024
https://github.com/TylerNowicki created https://github.com/llvm/llvm-project/pull/106306
Hi All,
This is an initial change as part of a larger effort (about 8 PRs) to add a new way to separate the code for different ABIs and at the same time supporting a wider variety of users beyond Swift and C++.
Problems:
* Users such as Swift and C++ require specific behavior from the CoroSplit pass and its related helpers, especially, CoroFrame. However, that has resulted in codes specific to certain users being shared and executed by all the users.
* To address the most significant cases of the above problem an ABI enum was added and is used by the many methods, functions, classes, etc. invoked by CoroSplit. However, this has resulted in code that is very difficult to read with some parts shared by multiple ABIs and some parts not, and it did not fully solve the above problem.
* A materializable callback was added to solve functional issues with our non-C++, non-Swift user. However, this cannot be used to perform all the possible optimizations our user supports and we now have the unfortunate situation that an optimization (rematerialization) is required for to ensure correct functionality.
* ...
>From 22fe0970bb3ed0bf3c8d189f91c5ad58e7066ed2 Mon Sep 17 00:00:00 2001
From: tnowicki <tnowicki.nowicki at amd.com>
Date: Fri, 16 Aug 2024 13:50:24 -0400
Subject: [PATCH 1/5] [llvm/llvm-project][Coroutines] Improve debugging and
minor refactoring
* Fix comments in several places
* Instead of using BB-getName() which will return an empty string if the
BB has no name, getBasicBlockLabel will give the BBs label e.g. ####
this makes the IR easier to read, otherwise we get lots of blocks with
names like ".from.".
* Use RPO when dumping SuspendCrossingInfo. Without this the dump order
is determined by the ptr addresses and so is not consistent from run
to run making IR diffs difficult to read.
* Inference -> Interference
* Pull the logic that determines insertion location out of insertSpills
and into getSpillInsertionPt, to differentiate between these two
operations.
* Use Shape getters for CoroId instead of getting it manually.
*
---
llvm/lib/Transforms/Coroutines/CoroEarly.cpp | 2 +-
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 187 +++++++++++--------
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 9 +-
3 files changed, 115 insertions(+), 83 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
index d8e827e9cebcfe..13b6680264c87c 100644
--- a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
@@ -203,7 +203,7 @@ void Lowerer::lowerEarlyIntrinsics(Function &F) {
if (auto *CII = cast<CoroIdInst>(&I)) {
if (CII->getInfo().isPreSplit()) {
assert(F.isPresplitCoroutine() &&
- "The frontend uses Swtich-Resumed ABI should emit "
+ "The frontend uses Switch-Resumed ABI should emit "
"\"presplitcoroutine\" attribute for the coroutine.");
setCannotDuplicate(CII);
CII->setCoroutineSelf();
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 00f49b7bdce294..671980d8c80626 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -6,7 +6,8 @@
//
//===----------------------------------------------------------------------===//
// This file contains classes used to discover if for a particular value
-// there from sue to definition that crosses a suspend block.
+// its definition preceeds and its uses follow a suspend block. This is
+// referred to as a suspend crossing value.
//
// Using the information discovered we form a Coroutine Frame structure to
// contain those values. All uses of those values are replaced with appropriate
@@ -52,6 +53,16 @@ extern cl::opt<bool> UseNewDbgInfoFormat;
enum { SmallVectorThreshold = 32 };
+static std::string getBasicBlockLabel(const BasicBlock *BB) {
+ if (BB->hasName())
+ return BB->getName().str();
+
+ std::string S;
+ raw_string_ostream OS(S);
+ BB->printAsOperand(OS, false);
+ return OS.str().substr(1);
+}
+
// Provides two way mapping between the blocks and numbers.
namespace {
class BlockToIndexMapping {
@@ -123,8 +134,9 @@ class SuspendCrossingInfo {
public:
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
- void dump() const;
- void dump(StringRef Label, BitVector const &BV) const;
+ void dump(const ReversePostOrderTraversal<Function *> &RPOT) const;
+ void dump(StringRef Label, BitVector const &BV,
+ const ReversePostOrderTraversal<Function *> &RPOT) const;
#endif
SuspendCrossingInfo(Function &F, coro::Shape &Shape);
@@ -207,21 +219,25 @@ class SuspendCrossingInfo {
} // end anonymous namespace
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(StringRef Label,
- BitVector const &BV) const {
+LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
+ StringRef Label, BitVector const &BV,
+ const ReversePostOrderTraversal<Function *> &RPOT) const {
dbgs() << Label << ":";
- for (size_t I = 0, N = BV.size(); I < N; ++I)
- if (BV[I])
- dbgs() << " " << Mapping.indexToBlock(I)->getName();
+ for (const BasicBlock *BB : RPOT) {
+ auto BBNo = Mapping.blockToIndex(BB);
+ if (BV[BBNo])
+ dbgs() << " " << getBasicBlockLabel(BB);
+ }
dbgs() << "\n";
}
-LLVM_DUMP_METHOD void SuspendCrossingInfo::dump() const {
- for (size_t I = 0, N = Block.size(); I < N; ++I) {
- BasicBlock *const B = Mapping.indexToBlock(I);
- dbgs() << B->getName() << ":\n";
- dump(" Consumes", Block[I].Consumes);
- dump(" Kills", Block[I].Kills);
+LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
+ const ReversePostOrderTraversal<Function *> &RPOT) const {
+ for (const BasicBlock *BB : RPOT) {
+ auto BBNo = Mapping.blockToIndex(BB);
+ dbgs() << getBasicBlockLabel(BB) << ":\n";
+ dump(" Consumes", Block[BBNo].Consumes, RPOT);
+ dump(" Kills", Block[BBNo].Kills, RPOT);
}
dbgs() << "\n";
}
@@ -335,7 +351,7 @@ SuspendCrossingInfo::SuspendCrossingInfo(Function &F, coro::Shape &Shape)
while (computeBlockData</*Initialize*/ false>(RPOT))
;
- LLVM_DEBUG(dump());
+ LLVM_DEBUG(dump(RPOT));
}
namespace {
@@ -419,7 +435,7 @@ struct RematGraph {
void dump() const {
dbgs() << "Entry (";
if (EntryNode->Node->getParent()->hasName())
- dbgs() << EntryNode->Node->getParent()->getName();
+ dbgs() << getBasicBlockLabel(EntryNode->Node->getParent());
else
EntryNode->Node->getParent()->printAsOperand(dbgs(), false);
dbgs() << ") : " << *EntryNode->Node << "\n";
@@ -551,7 +567,7 @@ struct FrameDataInfo {
#ifndef NDEBUG
static void dumpSpills(StringRef Title, const SpillInfo &Spills) {
- dbgs() << "------------- " << Title << "--------------\n";
+ dbgs() << "------------- " << Title << " --------------\n";
for (const auto &E : Spills) {
E.first->dump();
dbgs() << " user: ";
@@ -813,7 +829,7 @@ void FrameTypeBuilder::addFieldForAllocas(const Function &F,
StackLifetime StackLifetimeAnalyzer(F, ExtractAllocas(),
StackLifetime::LivenessType::May);
StackLifetimeAnalyzer.run();
- auto IsAllocaInferenre = [&](const AllocaInst *AI1, const AllocaInst *AI2) {
+ auto DoAllocasInterfere = [&](const AllocaInst *AI1, const AllocaInst *AI2) {
return StackLifetimeAnalyzer.getLiveRange(AI1).overlaps(
StackLifetimeAnalyzer.getLiveRange(AI2));
};
@@ -833,13 +849,13 @@ void FrameTypeBuilder::addFieldForAllocas(const Function &F,
for (const auto &A : FrameData.Allocas) {
AllocaInst *Alloca = A.Alloca;
bool Merged = false;
- // Try to find if the Alloca is not inferenced with any existing
+ // Try to find if the Alloca does not interfere with any existing
// NonOverlappedAllocaSet. If it is true, insert the alloca to that
// NonOverlappedAllocaSet.
for (auto &AllocaSet : NonOverlapedAllocas) {
assert(!AllocaSet.empty() && "Processing Alloca Set is not empty.\n");
- bool NoInference = none_of(AllocaSet, [&](auto Iter) {
- return IsAllocaInferenre(Alloca, Iter);
+ bool NoInterference = none_of(AllocaSet, [&](auto Iter) {
+ return DoAllocasInterfere(Alloca, Iter);
});
// If the alignment of A is multiple of the alignment of B, the address
// of A should satisfy the requirement for aligning for B.
@@ -852,7 +868,7 @@ void FrameTypeBuilder::addFieldForAllocas(const Function &F,
return LargestAlloca->getAlign().value() % Alloca->getAlign().value() ==
0;
}();
- bool CouldMerge = NoInference && Alignable;
+ bool CouldMerge = NoInterference && Alignable;
if (!CouldMerge)
continue;
AllocaSet.push_back(Alloca);
@@ -1713,6 +1729,51 @@ static Instruction *splitBeforeCatchSwitch(CatchSwitchInst *CatchSwitch) {
return CleanupRet;
}
+static BasicBlock::iterator getSpillInsertionPt(const coro::Shape &Shape,
+ Value *Def,
+ const DominatorTree &DT) {
+ BasicBlock::iterator InsertPt;
+ if (auto *Arg = dyn_cast<Argument>(Def)) {
+ // For arguments, we will place the store instruction right after
+ // the coroutine frame pointer instruction, i.e. coro.begin.
+ InsertPt = Shape.getInsertPtAfterFramePtr();
+
+ // If we're spilling an Argument, make sure we clear 'nocapture'
+ // from the coroutine function.
+ Arg->getParent()->removeParamAttr(Arg->getArgNo(), Attribute::NoCapture);
+ } else if (auto *CSI = dyn_cast<AnyCoroSuspendInst>(Def)) {
+ // Don't spill immediately after a suspend; splitting assumes
+ // that the suspend will be followed by a branch.
+ InsertPt = CSI->getParent()->getSingleSuccessor()->getFirstNonPHIIt();
+ } else {
+ auto *I = cast<Instruction>(Def);
+ if (!DT.dominates(Shape.CoroBegin, I)) {
+ // If it is not dominated by CoroBegin, then spill should be
+ // inserted immediately after CoroFrame is computed.
+ InsertPt = Shape.getInsertPtAfterFramePtr();
+ } else if (auto *II = dyn_cast<InvokeInst>(I)) {
+ // If we are spilling the result of the invoke instruction, split
+ // the normal edge and insert the spill in the new block.
+ auto *NewBB = SplitEdge(II->getParent(), II->getNormalDest());
+ InsertPt = NewBB->getTerminator()->getIterator();
+ } else if (isa<PHINode>(I)) {
+ // Skip the PHINodes and EH pads instructions.
+ BasicBlock *DefBlock = I->getParent();
+ if (auto *CSI = dyn_cast<CatchSwitchInst>(DefBlock->getTerminator()))
+ InsertPt = splitBeforeCatchSwitch(CSI)->getIterator();
+ else
+ InsertPt = DefBlock->getFirstInsertionPt();
+ } else {
+ assert(!I->isTerminator() && "unexpected terminator");
+ // For all other values, the spill is placed immediately after
+ // the definition.
+ InsertPt = I->getNextNode()->getIterator();
+ }
+ }
+
+ return InsertPt;
+}
+
// Replace all alloca and SSA values that are accessed across suspend points
// with GetElementPointer from coroutine frame + loads and stores. Create an
// AllocaSpillBB that will become the new entry block for the resume parts of
@@ -1735,9 +1796,8 @@ static Instruction *splitBeforeCatchSwitch(CatchSwitchInst *CatchSwitch) {
//
//
static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
- auto *CB = Shape.CoroBegin;
- LLVMContext &C = CB->getContext();
- Function *F = CB->getFunction();
+ LLVMContext &C = Shape.CoroBegin->getContext();
+ Function *F = Shape.CoroBegin->getFunction();
IRBuilder<> Builder(C);
StructType *FrameTy = Shape.FrameTy;
Value *FramePtr = Shape.FramePtr;
@@ -1798,47 +1858,16 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
auto SpillAlignment = Align(FrameData.getAlign(Def));
// Create a store instruction storing the value into the
// coroutine frame.
- BasicBlock::iterator InsertPt;
+ BasicBlock::iterator InsertPt = getSpillInsertionPt(Shape, Def, DT);
+
Type *ByValTy = nullptr;
if (auto *Arg = dyn_cast<Argument>(Def)) {
- // For arguments, we will place the store instruction right after
- // the coroutine frame pointer instruction, i.e. coro.begin.
- InsertPt = Shape.getInsertPtAfterFramePtr();
-
// If we're spilling an Argument, make sure we clear 'nocapture'
// from the coroutine function.
Arg->getParent()->removeParamAttr(Arg->getArgNo(), Attribute::NoCapture);
if (Arg->hasByValAttr())
ByValTy = Arg->getParamByValType();
- } else if (auto *CSI = dyn_cast<AnyCoroSuspendInst>(Def)) {
- // Don't spill immediately after a suspend; splitting assumes
- // that the suspend will be followed by a branch.
- InsertPt = CSI->getParent()->getSingleSuccessor()->getFirstNonPHIIt();
- } else {
- auto *I = cast<Instruction>(Def);
- if (!DT.dominates(CB, I)) {
- // If it is not dominated by CoroBegin, then spill should be
- // inserted immediately after CoroFrame is computed.
- InsertPt = Shape.getInsertPtAfterFramePtr();
- } else if (auto *II = dyn_cast<InvokeInst>(I)) {
- // If we are spilling the result of the invoke instruction, split
- // the normal edge and insert the spill in the new block.
- auto *NewBB = SplitEdge(II->getParent(), II->getNormalDest());
- InsertPt = NewBB->getTerminator()->getIterator();
- } else if (isa<PHINode>(I)) {
- // Skip the PHINodes and EH pads instructions.
- BasicBlock *DefBlock = I->getParent();
- if (auto *CSI = dyn_cast<CatchSwitchInst>(DefBlock->getTerminator()))
- InsertPt = splitBeforeCatchSwitch(CSI)->getIterator();
- else
- InsertPt = DefBlock->getFirstInsertionPt();
- } else {
- assert(!I->isTerminator() && "unexpected terminator");
- // For all other values, the spill is placed immediately after
- // the definition.
- InsertPt = I->getNextNode()->getIterator();
- }
}
auto Index = FrameData.getFieldIndex(Def);
@@ -1980,7 +2009,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
UsersToUpdate.clear();
for (User *U : Alloca->users()) {
auto *I = cast<Instruction>(U);
- if (DT.dominates(CB, I))
+ if (DT.dominates(Shape.CoroBegin, I))
UsersToUpdate.push_back(I);
}
if (UsersToUpdate.empty())
@@ -2022,7 +2051,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
Builder.CreateStore(Value, G);
}
// For each alias to Alloca created before CoroBegin but used after
- // CoroBegin, we recreate them after CoroBegin by appplying the offset
+ // CoroBegin, we recreate them after CoroBegin by applying the offset
// to the pointer in the frame.
for (const auto &Alias : A.Aliases) {
auto *FramePtr = GetFramePointer(Alloca);
@@ -2031,7 +2060,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
auto *AliasPtr =
Builder.CreatePtrAdd(FramePtr, ConstantInt::get(ITy, Value));
Alias.first->replaceUsesWithIf(
- AliasPtr, [&](Use &U) { return DT.dominates(CB, U); });
+ AliasPtr, [&](Use &U) { return DT.dominates(Shape.CoroBegin, U); });
}
}
@@ -2044,7 +2073,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
// If there is memory accessing to promise alloca before CoroBegin;
bool HasAccessingPromiseBeforeCB = llvm::any_of(PA->uses(), [&](Use &U) {
auto *Inst = dyn_cast<Instruction>(U.getUser());
- if (!Inst || DT.dominates(CB, Inst))
+ if (!Inst || DT.dominates(Shape.CoroBegin, Inst))
return false;
if (auto *CI = dyn_cast<CallInst>(Inst)) {
@@ -2087,8 +2116,9 @@ static void movePHIValuesToInsertedBlock(BasicBlock *SuccBB,
do {
int Index = PN->getBasicBlockIndex(InsertedBB);
Value *V = PN->getIncomingValue(Index);
- PHINode *InputV = PHINode::Create(
- V->getType(), 1, V->getName() + Twine(".") + SuccBB->getName());
+ PHINode *InputV =
+ PHINode::Create(V->getType(), 1,
+ V->getName() + Twine(".") + getBasicBlockLabel(SuccBB));
InputV->insertBefore(InsertedBB->begin());
InputV->addIncoming(V, PredBB);
PN->setIncomingValue(Index, InputV);
@@ -2133,10 +2163,10 @@ static void rewritePHIsForCleanupPad(BasicBlock *CleanupPadBB,
Builder.CreateUnreachable();
// Create a new cleanuppad which will be the dispatcher.
- auto *NewCleanupPadBB =
- BasicBlock::Create(CleanupPadBB->getContext(),
- CleanupPadBB->getName() + Twine(".corodispatch"),
- CleanupPadBB->getParent(), CleanupPadBB);
+ auto *NewCleanupPadBB = BasicBlock::Create(
+ CleanupPadBB->getContext(),
+ getBasicBlockLabel(CleanupPadBB) + Twine(".corodispatch"),
+ CleanupPadBB->getParent(), CleanupPadBB);
Builder.SetInsertPoint(NewCleanupPadBB);
auto *SwitchType = Builder.getInt8Ty();
auto *SetDispatchValuePN =
@@ -2150,13 +2180,14 @@ static void rewritePHIsForCleanupPad(BasicBlock *CleanupPadBB,
SmallVector<BasicBlock *, 8> Preds(predecessors(CleanupPadBB));
for (BasicBlock *Pred : Preds) {
// Create a new cleanuppad and move the PHI values to there.
- auto *CaseBB = BasicBlock::Create(CleanupPadBB->getContext(),
- CleanupPadBB->getName() +
- Twine(".from.") + Pred->getName(),
- CleanupPadBB->getParent(), CleanupPadBB);
+ auto *CaseBB =
+ BasicBlock::Create(CleanupPadBB->getContext(),
+ getBasicBlockLabel(CleanupPadBB) + Twine(".from.") +
+ getBasicBlockLabel(Pred),
+ CleanupPadBB->getParent(), CleanupPadBB);
updatePhiNodes(CleanupPadBB, Pred, CaseBB);
- CaseBB->setName(CleanupPadBB->getName() + Twine(".from.") +
- Pred->getName());
+ CaseBB->setName(getBasicBlockLabel(CleanupPadBB) + Twine(".from.") +
+ getBasicBlockLabel(Pred));
Builder.SetInsertPoint(CaseBB);
Builder.CreateBr(CleanupPadBB);
movePHIValuesToInsertedBlock(CleanupPadBB, CaseBB, NewCleanupPadBB);
@@ -2246,7 +2277,8 @@ static void rewritePHIs(BasicBlock &BB) {
SmallVector<BasicBlock *, 8> Preds(predecessors(&BB));
for (BasicBlock *Pred : Preds) {
auto *IncomingBB = ehAwareSplitEdge(Pred, &BB, LandingPad, ReplPHI);
- IncomingBB->setName(BB.getName() + Twine(".from.") + Pred->getName());
+ IncomingBB->setName(getBasicBlockLabel(&BB) + Twine(".from.") +
+ getBasicBlockLabel(Pred));
// Stop the moving of values at ReplPHI, as this is either null or the PHI
// that replaced the landing pad.
@@ -2690,7 +2722,7 @@ static void eliminateSwiftError(Function &F, coro::Shape &Shape) {
}
}
-/// retcon and retcon.once conventions assume that all spill uses can be sunk
+/// Async and Retcon{Once} conventions assume that all spill uses can be sunk
/// after the coro.begin intrinsic.
static void sinkSpillUsesAfterCoroBegin(Function &F,
const FrameDataInfo &FrameData,
@@ -3122,7 +3154,7 @@ void coro::buildCoroutineFrame(
cleanupSinglePredPHIs(F);
// Transforms multi-edge PHI Nodes, so that any value feeding into a PHI will
- // never has its definition separated from the PHI by the suspend point.
+ // never have its definition separated from the PHI by the suspend point.
rewritePHIs(F);
// Build suspend crossing info.
@@ -3219,6 +3251,7 @@ void coro::buildCoroutineFrame(
Shape.FramePtr = Shape.CoroBegin;
// For now, this works for C++ programs only.
buildFrameDebugInfo(F, Shape, FrameData);
+ // Insert spills and reloads
insertSpills(FrameData, Shape);
lowerLocalAllocas(LocalAllocas, DeadInstructions);
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 40bc932c3e0eef..6bf3c75b95113e 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -1219,11 +1219,10 @@ static void postSplitCleanup(Function &F) {
// frame if possible.
static void handleNoSuspendCoroutine(coro::Shape &Shape) {
auto *CoroBegin = Shape.CoroBegin;
- auto *CoroId = CoroBegin->getId();
- auto *AllocInst = CoroId->getCoroAlloc();
switch (Shape.ABI) {
case coro::ABI::Switch: {
- auto SwitchId = cast<CoroIdInst>(CoroId);
+ auto SwitchId = Shape.getSwitchCoroId();
+ auto *AllocInst = SwitchId->getCoroAlloc();
coro::replaceCoroFree(SwitchId, /*Elide=*/AllocInst != nullptr);
if (AllocInst) {
IRBuilder<> Builder(AllocInst);
@@ -1687,7 +1686,7 @@ static void splitAsyncCoroutine(Function &F, coro::Shape &Shape,
auto &Context = F.getContext();
auto *Int8PtrTy = PointerType::getUnqual(Context);
- auto *Id = cast<CoroIdAsyncInst>(Shape.CoroBegin->getId());
+ auto *Id = Shape.getAsyncCoroId();
IRBuilder<> Builder(Id);
auto *FramePtr = Id->getStorage();
@@ -1781,7 +1780,7 @@ static void splitRetconCoroutine(Function &F, coro::Shape &Shape,
F.removeRetAttr(Attribute::NonNull);
// Allocate the frame.
- auto *Id = cast<AnyCoroIdRetconInst>(Shape.CoroBegin->getId());
+ auto *Id = Shape.getRetconCoroId();
Value *RawFramePtr;
if (Shape.RetconLowering.IsFrameInlineInStorage) {
RawFramePtr = Id->getStorage();
>From b3e4337877fc7559211a0bd84bb66a61b7e622c9 Mon Sep 17 00:00:00 2001
From: tnowicki <tnowicki.nowicki at amd.com>
Date: Fri, 16 Aug 2024 17:19:34 -0400
Subject: [PATCH 2/5] Fix formatting
---
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 671980d8c80626..6267a0d430d283 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1730,8 +1730,8 @@ static Instruction *splitBeforeCatchSwitch(CatchSwitchInst *CatchSwitch) {
}
static BasicBlock::iterator getSpillInsertionPt(const coro::Shape &Shape,
- Value *Def,
- const DominatorTree &DT) {
+ Value *Def,
+ const DominatorTree &DT) {
BasicBlock::iterator InsertPt;
if (auto *Arg = dyn_cast<Argument>(Def)) {
// For arguments, we will place the store instruction right after
>From 82610e5f5e283b5b57a14899c0cc50e5002f7808 Mon Sep 17 00:00:00 2001
From: tnowicki <tnowicki.nowicki at amd.com>
Date: Wed, 21 Aug 2024 17:55:19 -0400
Subject: [PATCH 3/5] [llvm/llvm-project][Coroutines] Address reviewers
feedback
---
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 63 +++++++++-----------
1 file changed, 28 insertions(+), 35 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 6267a0d430d283..7e32c19d9cfdd9 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
// This file contains classes used to discover if for a particular value
-// its definition preceeds and its uses follow a suspend block. This is
+// its definition precedes and its uses follow a suspend block. This is
// referred to as a suspend crossing value.
//
// Using the information discovered we form a Coroutine Frame structure to
@@ -53,16 +53,6 @@ extern cl::opt<bool> UseNewDbgInfoFormat;
enum { SmallVectorThreshold = 32 };
-static std::string getBasicBlockLabel(const BasicBlock *BB) {
- if (BB->hasName())
- return BB->getName().str();
-
- std::string S;
- raw_string_ostream OS(S);
- BB->printAsOperand(OS, false);
- return OS.str().substr(1);
-}
-
// Provides two way mapping between the blocks and numbers.
namespace {
class BlockToIndexMapping {
@@ -134,7 +124,7 @@ class SuspendCrossingInfo {
public:
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
- void dump(const ReversePostOrderTraversal<Function *> &RPOT) const;
+ void dump() const;
void dump(StringRef Label, BitVector const &BV,
const ReversePostOrderTraversal<Function *> &RPOT) const;
#endif
@@ -226,16 +216,22 @@ LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
for (const BasicBlock *BB : RPOT) {
auto BBNo = Mapping.blockToIndex(BB);
if (BV[BBNo])
- dbgs() << " " << getBasicBlockLabel(BB);
+ dbgs() << " " << BB->getName();
}
dbgs() << "\n";
}
-LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
- const ReversePostOrderTraversal<Function *> &RPOT) const {
+LLVM_DUMP_METHOD void SuspendCrossingInfo::dump() const {
+ if (Block.empty())
+ return;
+
+ BasicBlock *const B = Mapping.indexToBlock(0);
+ Function *F = B->getParent();
+
+ ReversePostOrderTraversal<Function *> RPOT(F);
for (const BasicBlock *BB : RPOT) {
auto BBNo = Mapping.blockToIndex(BB);
- dbgs() << getBasicBlockLabel(BB) << ":\n";
+ dbgs() << BB->getName() << ":\n";
dump(" Consumes", Block[BBNo].Consumes, RPOT);
dump(" Kills", Block[BBNo].Kills, RPOT);
}
@@ -351,7 +347,7 @@ SuspendCrossingInfo::SuspendCrossingInfo(Function &F, coro::Shape &Shape)
while (computeBlockData</*Initialize*/ false>(RPOT))
;
- LLVM_DEBUG(dump(RPOT));
+ LLVM_DEBUG(dump());
}
namespace {
@@ -435,7 +431,7 @@ struct RematGraph {
void dump() const {
dbgs() << "Entry (";
if (EntryNode->Node->getParent()->hasName())
- dbgs() << getBasicBlockLabel(EntryNode->Node->getParent());
+ dbgs() << EntryNode->Node->getParent()->getName();
else
EntryNode->Node->getParent()->printAsOperand(dbgs(), false);
dbgs() << ") : " << *EntryNode->Node << "\n";
@@ -2116,9 +2112,8 @@ static void movePHIValuesToInsertedBlock(BasicBlock *SuccBB,
do {
int Index = PN->getBasicBlockIndex(InsertedBB);
Value *V = PN->getIncomingValue(Index);
- PHINode *InputV =
- PHINode::Create(V->getType(), 1,
- V->getName() + Twine(".") + getBasicBlockLabel(SuccBB));
+ PHINode *InputV = PHINode::Create(
+ V->getType(), 1, V->getName() + Twine(".") + SuccBB->getName());
InputV->insertBefore(InsertedBB->begin());
InputV->addIncoming(V, PredBB);
PN->setIncomingValue(Index, InputV);
@@ -2163,10 +2158,10 @@ static void rewritePHIsForCleanupPad(BasicBlock *CleanupPadBB,
Builder.CreateUnreachable();
// Create a new cleanuppad which will be the dispatcher.
- auto *NewCleanupPadBB = BasicBlock::Create(
- CleanupPadBB->getContext(),
- getBasicBlockLabel(CleanupPadBB) + Twine(".corodispatch"),
- CleanupPadBB->getParent(), CleanupPadBB);
+ auto *NewCleanupPadBB =
+ BasicBlock::Create(CleanupPadBB->getContext(),
+ CleanupPadBB->getName() + Twine(".corodispatch"),
+ CleanupPadBB->getParent(), CleanupPadBB);
Builder.SetInsertPoint(NewCleanupPadBB);
auto *SwitchType = Builder.getInt8Ty();
auto *SetDispatchValuePN =
@@ -2180,14 +2175,13 @@ static void rewritePHIsForCleanupPad(BasicBlock *CleanupPadBB,
SmallVector<BasicBlock *, 8> Preds(predecessors(CleanupPadBB));
for (BasicBlock *Pred : Preds) {
// Create a new cleanuppad and move the PHI values to there.
- auto *CaseBB =
- BasicBlock::Create(CleanupPadBB->getContext(),
- getBasicBlockLabel(CleanupPadBB) + Twine(".from.") +
- getBasicBlockLabel(Pred),
- CleanupPadBB->getParent(), CleanupPadBB);
+ auto *CaseBB = BasicBlock::Create(CleanupPadBB->getContext(),
+ CleanupPadBB->getName() +
+ Twine(".from.") + Pred->getName(),
+ CleanupPadBB->getParent(), CleanupPadBB);
updatePhiNodes(CleanupPadBB, Pred, CaseBB);
- CaseBB->setName(getBasicBlockLabel(CleanupPadBB) + Twine(".from.") +
- getBasicBlockLabel(Pred));
+ CaseBB->setName(CleanupPadBB->getName() + Twine(".from.") +
+ Pred->getName());
Builder.SetInsertPoint(CaseBB);
Builder.CreateBr(CleanupPadBB);
movePHIValuesToInsertedBlock(CleanupPadBB, CaseBB, NewCleanupPadBB);
@@ -2277,8 +2271,7 @@ static void rewritePHIs(BasicBlock &BB) {
SmallVector<BasicBlock *, 8> Preds(predecessors(&BB));
for (BasicBlock *Pred : Preds) {
auto *IncomingBB = ehAwareSplitEdge(Pred, &BB, LandingPad, ReplPHI);
- IncomingBB->setName(getBasicBlockLabel(&BB) + Twine(".from.") +
- getBasicBlockLabel(Pred));
+ IncomingBB->setName(BB.getName() + Twine(".from.") + Pred->getName());
// Stop the moving of values at ReplPHI, as this is either null or the PHI
// that replaced the landing pad.
@@ -2758,7 +2751,7 @@ static void sinkSpillUsesAfterCoroBegin(Function &F,
// Sort by dominance.
SmallVector<Instruction *, 64> InsertionList(ToMove.begin(), ToMove.end());
llvm::sort(InsertionList, [&Dom](Instruction *A, Instruction *B) -> bool {
- // If a dominates b it should preceed (<) b.
+ // If a dominates b it should precede (<) b.
return Dom.dominates(A, B);
});
>From 6f6666f4e86537b0b93b0f1686ac3168eb29a5da Mon Sep 17 00:00:00 2001
From: tnowicki <tnowicki.nowicki at amd.com>
Date: Thu, 22 Aug 2024 13:47:42 -0400
Subject: [PATCH 4/5] [llvm/llvm-project][Coroutines] Add back
getBasicBlockLabel for dumping
---
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 7e32c19d9cfdd9..d63d01acd97c20 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -209,6 +209,16 @@ class SuspendCrossingInfo {
} // end anonymous namespace
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+static std::string getBasicBlockLabel(const BasicBlock *BB) {
+ if (BB->hasName())
+ return BB->getName().str();
+
+ std::string S;
+ raw_string_ostream OS(S);
+ BB->printAsOperand(OS, false);
+ return OS.str().substr(1);
+}
+
LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
StringRef Label, BitVector const &BV,
const ReversePostOrderTraversal<Function *> &RPOT) const {
@@ -216,7 +226,7 @@ LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
for (const BasicBlock *BB : RPOT) {
auto BBNo = Mapping.blockToIndex(BB);
if (BV[BBNo])
- dbgs() << " " << BB->getName();
+ dbgs() << " " << getBasicBlockLabel(BB);
}
dbgs() << "\n";
}
@@ -231,7 +241,7 @@ LLVM_DUMP_METHOD void SuspendCrossingInfo::dump() const {
ReversePostOrderTraversal<Function *> RPOT(F);
for (const BasicBlock *BB : RPOT) {
auto BBNo = Mapping.blockToIndex(BB);
- dbgs() << BB->getName() << ":\n";
+ dbgs() << getBasicBlockLabel(BB) << ":\n";
dump(" Consumes", Block[BBNo].Consumes, RPOT);
dump(" Kills", Block[BBNo].Kills, RPOT);
}
@@ -430,10 +440,7 @@ struct RematGraph {
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void dump() const {
dbgs() << "Entry (";
- if (EntryNode->Node->getParent()->hasName())
- dbgs() << EntryNode->Node->getParent()->getName();
- else
- EntryNode->Node->getParent()->printAsOperand(dbgs(), false);
+ dbgs() << getBasicBlockLabel(EntryNode->Node->getParent());
dbgs() << ") : " << *EntryNode->Node << "\n";
for (auto &E : Remats) {
dbgs() << *(E.first) << "\n";
>From 1582d121865bbf59276bdddd1e264e83227210a8 Mon Sep 17 00:00:00 2001
From: tnowicki <tnowicki.nowicki at amd.com>
Date: Fri, 23 Aug 2024 13:12:17 -0400
Subject: [PATCH 5/5] [llvm/llvm-project][Coroutines] Major refactoring of
SuspendCrossingInfo
* Move SuspendCrossingInfo to its own files to clean up CoroFrame
---
llvm/lib/Transforms/Coroutines/CMakeLists.txt | 1 +
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 322 +-----------------
.../Coroutines/SuspendCrossingInfo.cpp | 195 +++++++++++
.../Coroutines/SuspendCrossingInfo.h | 182 ++++++++++
4 files changed, 390 insertions(+), 310 deletions(-)
create mode 100644 llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.cpp
create mode 100644 llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.h
diff --git a/llvm/lib/Transforms/Coroutines/CMakeLists.txt b/llvm/lib/Transforms/Coroutines/CMakeLists.txt
index 2139446e5ff957..57359acc81ee4c 100644
--- a/llvm/lib/Transforms/Coroutines/CMakeLists.txt
+++ b/llvm/lib/Transforms/Coroutines/CMakeLists.txt
@@ -6,6 +6,7 @@ add_llvm_component_library(LLVMCoroutines
CoroElide.cpp
CoroFrame.cpp
CoroSplit.cpp
+ SuspendCrossingInfo.cpp
ADDITIONAL_HEADER_DIRS
${LLVM_MAIN_INCLUDE_DIR}/llvm/Transforms/Coroutines
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index d63d01acd97c20..36b45076fb4bb2 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -17,6 +17,7 @@
#include "CoroInternal.h"
#include "llvm/ADT/BitVector.h"
+#include "SuspendCrossingInfo.h"
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
@@ -51,315 +52,6 @@ extern cl::opt<bool> UseNewDbgInfoFormat;
// "coro-frame", which results in leaner debug spew.
#define DEBUG_TYPE "coro-suspend-crossing"
-enum { SmallVectorThreshold = 32 };
-
-// Provides two way mapping between the blocks and numbers.
-namespace {
-class BlockToIndexMapping {
- SmallVector<BasicBlock *, SmallVectorThreshold> V;
-
-public:
- size_t size() const { return V.size(); }
-
- BlockToIndexMapping(Function &F) {
- for (BasicBlock &BB : F)
- V.push_back(&BB);
- llvm::sort(V);
- }
-
- size_t blockToIndex(BasicBlock const *BB) const {
- auto *I = llvm::lower_bound(V, BB);
- assert(I != V.end() && *I == BB && "BasicBlockNumberng: Unknown block");
- return I - V.begin();
- }
-
- BasicBlock *indexToBlock(unsigned Index) const { return V[Index]; }
-};
-} // end anonymous namespace
-
-// The SuspendCrossingInfo maintains data that allows to answer a question
-// whether given two BasicBlocks A and B there is a path from A to B that
-// passes through a suspend point.
-//
-// For every basic block 'i' it maintains a BlockData that consists of:
-// Consumes: a bit vector which contains a set of indices of blocks that can
-// reach block 'i'. A block can trivially reach itself.
-// Kills: a bit vector which contains a set of indices of blocks that can
-// reach block 'i' but there is a path crossing a suspend point
-// not repeating 'i' (path to 'i' without cycles containing 'i').
-// Suspend: a boolean indicating whether block 'i' contains a suspend point.
-// End: a boolean indicating whether block 'i' contains a coro.end intrinsic.
-// KillLoop: There is a path from 'i' to 'i' not otherwise repeating 'i' that
-// crosses a suspend point.
-//
-namespace {
-class SuspendCrossingInfo {
- BlockToIndexMapping Mapping;
-
- struct BlockData {
- BitVector Consumes;
- BitVector Kills;
- bool Suspend = false;
- bool End = false;
- bool KillLoop = false;
- bool Changed = false;
- };
- SmallVector<BlockData, SmallVectorThreshold> Block;
-
- iterator_range<pred_iterator> predecessors(BlockData const &BD) const {
- BasicBlock *BB = Mapping.indexToBlock(&BD - &Block[0]);
- return llvm::predecessors(BB);
- }
-
- BlockData &getBlockData(BasicBlock *BB) {
- return Block[Mapping.blockToIndex(BB)];
- }
-
- /// Compute the BlockData for the current function in one iteration.
- /// Initialize - Whether this is the first iteration, we can optimize
- /// the initial case a little bit by manual loop switch.
- /// Returns whether the BlockData changes in this iteration.
- template <bool Initialize = false>
- bool computeBlockData(const ReversePostOrderTraversal<Function *> &RPOT);
-
-public:
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
- void dump() const;
- void dump(StringRef Label, BitVector const &BV,
- const ReversePostOrderTraversal<Function *> &RPOT) const;
-#endif
-
- SuspendCrossingInfo(Function &F, coro::Shape &Shape);
-
- /// Returns true if there is a path from \p From to \p To crossing a suspend
- /// point without crossing \p From a 2nd time.
- bool hasPathCrossingSuspendPoint(BasicBlock *From, BasicBlock *To) const {
- size_t const FromIndex = Mapping.blockToIndex(From);
- size_t const ToIndex = Mapping.blockToIndex(To);
- bool const Result = Block[ToIndex].Kills[FromIndex];
- LLVM_DEBUG(dbgs() << From->getName() << " => " << To->getName()
- << " answer is " << Result << "\n");
- return Result;
- }
-
- /// Returns true if there is a path from \p From to \p To crossing a suspend
- /// point without crossing \p From a 2nd time. If \p From is the same as \p To
- /// this will also check if there is a looping path crossing a suspend point.
- bool hasPathOrLoopCrossingSuspendPoint(BasicBlock *From,
- BasicBlock *To) const {
- size_t const FromIndex = Mapping.blockToIndex(From);
- size_t const ToIndex = Mapping.blockToIndex(To);
- bool Result = Block[ToIndex].Kills[FromIndex] ||
- (From == To && Block[ToIndex].KillLoop);
- LLVM_DEBUG(dbgs() << From->getName() << " => " << To->getName()
- << " answer is " << Result << " (path or loop)\n");
- return Result;
- }
-
- bool isDefinitionAcrossSuspend(BasicBlock *DefBB, User *U) const {
- auto *I = cast<Instruction>(U);
-
- // We rewrote PHINodes, so that only the ones with exactly one incoming
- // value need to be analyzed.
- if (auto *PN = dyn_cast<PHINode>(I))
- if (PN->getNumIncomingValues() > 1)
- return false;
-
- BasicBlock *UseBB = I->getParent();
-
- // As a special case, treat uses by an llvm.coro.suspend.retcon or an
- // llvm.coro.suspend.async as if they were uses in the suspend's single
- // predecessor: the uses conceptually occur before the suspend.
- if (isa<CoroSuspendRetconInst>(I) || isa<CoroSuspendAsyncInst>(I)) {
- UseBB = UseBB->getSinglePredecessor();
- assert(UseBB && "should have split coro.suspend into its own block");
- }
-
- return hasPathCrossingSuspendPoint(DefBB, UseBB);
- }
-
- bool isDefinitionAcrossSuspend(Argument &A, User *U) const {
- return isDefinitionAcrossSuspend(&A.getParent()->getEntryBlock(), U);
- }
-
- bool isDefinitionAcrossSuspend(Instruction &I, User *U) const {
- auto *DefBB = I.getParent();
-
- // As a special case, treat values produced by an llvm.coro.suspend.*
- // as if they were defined in the single successor: the uses
- // conceptually occur after the suspend.
- if (isa<AnyCoroSuspendInst>(I)) {
- DefBB = DefBB->getSingleSuccessor();
- assert(DefBB && "should have split coro.suspend into its own block");
- }
-
- return isDefinitionAcrossSuspend(DefBB, U);
- }
-
- bool isDefinitionAcrossSuspend(Value &V, User *U) const {
- if (auto *Arg = dyn_cast<Argument>(&V))
- return isDefinitionAcrossSuspend(*Arg, U);
- if (auto *Inst = dyn_cast<Instruction>(&V))
- return isDefinitionAcrossSuspend(*Inst, U);
-
- llvm_unreachable(
- "Coroutine could only collect Argument and Instruction now.");
- }
-};
-} // end anonymous namespace
-
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
-static std::string getBasicBlockLabel(const BasicBlock *BB) {
- if (BB->hasName())
- return BB->getName().str();
-
- std::string S;
- raw_string_ostream OS(S);
- BB->printAsOperand(OS, false);
- return OS.str().substr(1);
-}
-
-LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
- StringRef Label, BitVector const &BV,
- const ReversePostOrderTraversal<Function *> &RPOT) const {
- dbgs() << Label << ":";
- for (const BasicBlock *BB : RPOT) {
- auto BBNo = Mapping.blockToIndex(BB);
- if (BV[BBNo])
- dbgs() << " " << getBasicBlockLabel(BB);
- }
- dbgs() << "\n";
-}
-
-LLVM_DUMP_METHOD void SuspendCrossingInfo::dump() const {
- if (Block.empty())
- return;
-
- BasicBlock *const B = Mapping.indexToBlock(0);
- Function *F = B->getParent();
-
- ReversePostOrderTraversal<Function *> RPOT(F);
- for (const BasicBlock *BB : RPOT) {
- auto BBNo = Mapping.blockToIndex(BB);
- dbgs() << getBasicBlockLabel(BB) << ":\n";
- dump(" Consumes", Block[BBNo].Consumes, RPOT);
- dump(" Kills", Block[BBNo].Kills, RPOT);
- }
- dbgs() << "\n";
-}
-#endif
-
-template <bool Initialize>
-bool SuspendCrossingInfo::computeBlockData(
- const ReversePostOrderTraversal<Function *> &RPOT) {
- bool Changed = false;
-
- for (const BasicBlock *BB : RPOT) {
- auto BBNo = Mapping.blockToIndex(BB);
- auto &B = Block[BBNo];
-
- // We don't need to count the predecessors when initialization.
- if constexpr (!Initialize)
- // If all the predecessors of the current Block don't change,
- // the BlockData for the current block must not change too.
- if (all_of(predecessors(B), [this](BasicBlock *BB) {
- return !Block[Mapping.blockToIndex(BB)].Changed;
- })) {
- B.Changed = false;
- continue;
- }
-
- // Saved Consumes and Kills bitsets so that it is easy to see
- // if anything changed after propagation.
- auto SavedConsumes = B.Consumes;
- auto SavedKills = B.Kills;
-
- for (BasicBlock *PI : predecessors(B)) {
- auto PrevNo = Mapping.blockToIndex(PI);
- auto &P = Block[PrevNo];
-
- // Propagate Kills and Consumes from predecessors into B.
- B.Consumes |= P.Consumes;
- B.Kills |= P.Kills;
-
- // If block P is a suspend block, it should propagate kills into block
- // B for every block P consumes.
- if (P.Suspend)
- B.Kills |= P.Consumes;
- }
-
- if (B.Suspend) {
- // If block B is a suspend block, it should kill all of the blocks it
- // consumes.
- B.Kills |= B.Consumes;
- } else if (B.End) {
- // If block B is an end block, it should not propagate kills as the
- // blocks following coro.end() are reached during initial invocation
- // of the coroutine while all the data are still available on the
- // stack or in the registers.
- B.Kills.reset();
- } else {
- // This is reached when B block it not Suspend nor coro.end and it
- // need to make sure that it is not in the kill set.
- B.KillLoop |= B.Kills[BBNo];
- B.Kills.reset(BBNo);
- }
-
- if constexpr (!Initialize) {
- B.Changed = (B.Kills != SavedKills) || (B.Consumes != SavedConsumes);
- Changed |= B.Changed;
- }
- }
-
- return Changed;
-}
-
-SuspendCrossingInfo::SuspendCrossingInfo(Function &F, coro::Shape &Shape)
- : Mapping(F) {
- const size_t N = Mapping.size();
- Block.resize(N);
-
- // Initialize every block so that it consumes itself
- for (size_t I = 0; I < N; ++I) {
- auto &B = Block[I];
- B.Consumes.resize(N);
- B.Kills.resize(N);
- B.Consumes.set(I);
- B.Changed = true;
- }
-
- // Mark all CoroEnd Blocks. We do not propagate Kills beyond coro.ends as
- // the code beyond coro.end is reachable during initial invocation of the
- // coroutine.
- for (auto *CE : Shape.CoroEnds)
- getBlockData(CE->getParent()).End = true;
-
- // Mark all suspend blocks and indicate that they kill everything they
- // consume. Note, that crossing coro.save also requires a spill, as any code
- // between coro.save and coro.suspend may resume the coroutine and all of the
- // state needs to be saved by that time.
- auto markSuspendBlock = [&](IntrinsicInst *BarrierInst) {
- BasicBlock *SuspendBlock = BarrierInst->getParent();
- auto &B = getBlockData(SuspendBlock);
- B.Suspend = true;
- B.Kills |= B.Consumes;
- };
- for (auto *CSI : Shape.CoroSuspends) {
- markSuspendBlock(CSI);
- if (auto *Save = CSI->getCoroSave())
- markSuspendBlock(Save);
- }
-
- // It is considered to be faster to use RPO traversal for forward-edges
- // dataflow analysis.
- ReversePostOrderTraversal<Function *> RPOT(&F);
- computeBlockData</*Initialize=*/true>(RPOT);
- while (computeBlockData</*Initialize*/ false>(RPOT))
- ;
-
- LLVM_DEBUG(dump());
-}
-
namespace {
// RematGraph is used to construct a DAG for rematerializable instructions
@@ -438,6 +130,16 @@ struct RematGraph {
}
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+ static std::string getBasicBlockLabel(const BasicBlock *BB) {
+ if (BB->hasName())
+ return BB->getName().str();
+
+ std::string S;
+ raw_string_ostream OS(S);
+ BB->printAsOperand(OS, false);
+ return OS.str().substr(1);
+ }
+
void dump() const {
dbgs() << "Entry (";
dbgs() << getBasicBlockLabel(EntryNode->Node->getParent());
@@ -3158,7 +2860,7 @@ void coro::buildCoroutineFrame(
rewritePHIs(F);
// Build suspend crossing info.
- SuspendCrossingInfo Checker(F, Shape);
+ SuspendCrossingInfo Checker(F, Shape.CoroSuspends, Shape.CoroEnds);
doRematerializations(F, Checker, MaterializableCallback);
diff --git a/llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.cpp b/llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.cpp
new file mode 100644
index 00000000000000..ff3b32e958edac
--- /dev/null
+++ b/llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.cpp
@@ -0,0 +1,195 @@
+//===- SuspendCrossingInfo.cpp - Utility for suspend crossing values ------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+// The SuspendCrossingInfo maintains data that allows to answer a question
+// whether given two BasicBlocks A and B there is a path from A to B that
+// passes through a suspend point.
+//===----------------------------------------------------------------------===//
+
+#include "SuspendCrossingInfo.h"
+
+// The "coro-suspend-crossing" flag is very noisy. There is another debug type,
+// "coro-frame", which results in leaner debug spew.
+#define DEBUG_TYPE "coro-suspend-crossing"
+
+namespace llvm {
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+static std::string getBasicBlockLabel(const BasicBlock *BB) {
+ if (BB->hasName())
+ return BB->getName().str();
+
+ std::string S;
+ raw_string_ostream OS(S);
+ BB->printAsOperand(OS, false);
+ return OS.str().substr(1);
+}
+
+LLVM_DUMP_METHOD void SuspendCrossingInfo::dump(
+ StringRef Label, BitVector const &BV,
+ const ReversePostOrderTraversal<Function *> &RPOT) const {
+ dbgs() << Label << ":";
+ for (const BasicBlock *BB : RPOT) {
+ auto BBNo = Mapping.blockToIndex(BB);
+ if (BV[BBNo])
+ dbgs() << " " << getBasicBlockLabel(BB);
+ }
+ dbgs() << "\n";
+}
+
+LLVM_DUMP_METHOD void SuspendCrossingInfo::dump() const {
+ if (Block.empty())
+ return;
+
+ BasicBlock *const B = Mapping.indexToBlock(0);
+ Function *F = B->getParent();
+
+ ReversePostOrderTraversal<Function *> RPOT(F);
+ for (const BasicBlock *BB : RPOT) {
+ auto BBNo = Mapping.blockToIndex(BB);
+ dbgs() << getBasicBlockLabel(BB) << ":\n";
+ dump(" Consumes", Block[BBNo].Consumes, RPOT);
+ dump(" Kills", Block[BBNo].Kills, RPOT);
+ }
+ dbgs() << "\n";
+}
+#endif
+
+bool SuspendCrossingInfo::hasPathCrossingSuspendPoint(BasicBlock *From,
+ BasicBlock *To) const {
+ size_t const FromIndex = Mapping.blockToIndex(From);
+ size_t const ToIndex = Mapping.blockToIndex(To);
+ bool const Result = Block[ToIndex].Kills[FromIndex];
+ LLVM_DEBUG(if (Result) dbgs() << From->getName() << " => " << To->getName()
+ << " crosses suspend point\n");
+ return Result;
+}
+
+bool SuspendCrossingInfo::hasPathOrLoopCrossingSuspendPoint(
+ BasicBlock *From, BasicBlock *To) const {
+ size_t const FromIndex = Mapping.blockToIndex(From);
+ size_t const ToIndex = Mapping.blockToIndex(To);
+ bool Result = Block[ToIndex].Kills[FromIndex] ||
+ (From == To && Block[ToIndex].KillLoop);
+ LLVM_DEBUG(if (Result) dbgs() << From->getName() << " => " << To->getName()
+ << " crosses suspend point (path or loop)\n");
+ return Result;
+}
+
+template <bool Initialize>
+bool SuspendCrossingInfo::computeBlockData(
+ const ReversePostOrderTraversal<Function *> &RPOT) {
+ bool Changed = false;
+
+ for (const BasicBlock *BB : RPOT) {
+ auto BBNo = Mapping.blockToIndex(BB);
+ auto &B = Block[BBNo];
+
+ // We don't need to count the predecessors when initialization.
+ if constexpr (!Initialize)
+ // If all the predecessors of the current Block don't change,
+ // the BlockData for the current block must not change too.
+ if (all_of(predecessors(B), [this](BasicBlock *BB) {
+ return !Block[Mapping.blockToIndex(BB)].Changed;
+ })) {
+ B.Changed = false;
+ continue;
+ }
+
+ // Saved Consumes and Kills bitsets so that it is easy to see
+ // if anything changed after propagation.
+ auto SavedConsumes = B.Consumes;
+ auto SavedKills = B.Kills;
+
+ for (BasicBlock *PI : predecessors(B)) {
+ auto PrevNo = Mapping.blockToIndex(PI);
+ auto &P = Block[PrevNo];
+
+ // Propagate Kills and Consumes from predecessors into B.
+ B.Consumes |= P.Consumes;
+ B.Kills |= P.Kills;
+
+ // If block P is a suspend block, it should propagate kills into block
+ // B for every block P consumes.
+ if (P.Suspend)
+ B.Kills |= P.Consumes;
+ }
+
+ if (B.Suspend) {
+ // If block B is a suspend block, it should kill all of the blocks it
+ // consumes.
+ B.Kills |= B.Consumes;
+ } else if (B.End) {
+ // If block B is an end block, it should not propagate kills as the
+ // blocks following coro.end() are reached during initial invocation
+ // of the coroutine while all the data are still available on the
+ // stack or in the registers.
+ B.Kills.reset();
+ } else {
+ // This is reached when B block it not Suspend nor coro.end and it
+ // need to make sure that it is not in the kill set.
+ B.KillLoop |= B.Kills[BBNo];
+ B.Kills.reset(BBNo);
+ }
+
+ if constexpr (!Initialize) {
+ B.Changed = (B.Kills != SavedKills) || (B.Consumes != SavedConsumes);
+ Changed |= B.Changed;
+ }
+ }
+
+ return Changed;
+}
+
+SuspendCrossingInfo::SuspendCrossingInfo(
+ Function &F, const SmallVectorImpl<AnyCoroSuspendInst *> &CoroSuspends,
+ const SmallVectorImpl<AnyCoroEndInst *> &CoroEnds)
+ : Mapping(F) {
+ const size_t N = Mapping.size();
+ Block.resize(N);
+
+ // Initialize every block so that it consumes itself
+ for (size_t I = 0; I < N; ++I) {
+ auto &B = Block[I];
+ B.Consumes.resize(N);
+ B.Kills.resize(N);
+ B.Consumes.set(I);
+ B.Changed = true;
+ }
+
+ // Mark all CoroEnd Blocks. We do not propagate Kills beyond coro.ends as
+ // the code beyond coro.end is reachable during initial invocation of the
+ // coroutine.
+ for (auto *CE : CoroEnds)
+ getBlockData(CE->getParent()).End = true;
+
+ // Mark all suspend blocks and indicate that they kill everything they
+ // consume. Note, that crossing coro.save also requires a spill, as any code
+ // between coro.save and coro.suspend may resume the coroutine and all of the
+ // state needs to be saved by that time.
+ auto markSuspendBlock = [&](IntrinsicInst *BarrierInst) {
+ BasicBlock *SuspendBlock = BarrierInst->getParent();
+ auto &B = getBlockData(SuspendBlock);
+ B.Suspend = true;
+ B.Kills |= B.Consumes;
+ };
+ for (auto *CSI : CoroSuspends) {
+ markSuspendBlock(CSI);
+ if (auto *Save = CSI->getCoroSave())
+ markSuspendBlock(Save);
+ }
+
+ // It is considered to be faster to use RPO traversal for forward-edges
+ // dataflow analysis.
+ ReversePostOrderTraversal<Function *> RPOT(&F);
+ computeBlockData</*Initialize=*/true>(RPOT);
+ while (computeBlockData</*Initialize*/ false>(RPOT))
+ ;
+
+ LLVM_DEBUG(dump());
+}
+
+} // namespace llvm
diff --git a/llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.h b/llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.h
new file mode 100644
index 00000000000000..d1551af0b54975
--- /dev/null
+++ b/llvm/lib/Transforms/Coroutines/SuspendCrossingInfo.h
@@ -0,0 +1,182 @@
+//===- SuspendCrossingInfo.cpp - Utility for suspend crossing values ------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+// The SuspendCrossingInfo maintains data that allows to answer a question
+// whether given two BasicBlocks A and B there is a path from A to B that
+// passes through a suspend point.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TRANSFORMS_COROUTINES_SUSPENDCROSSINGINFO_H
+#define LLVM_LIB_TRANSFORMS_COROUTINES_SUSPENDCROSSINGINFO_H
+
+#include "CoroInstr.h"
+#include "llvm/ADT/BitVector.h"
+#include "llvm/ADT/PostOrderIterator.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Instruction.h"
+
+namespace llvm {
+
+// Provides two way mapping between the blocks and numbers.
+class BlockToIndexMapping {
+ SmallVector<BasicBlock *, 32> V;
+
+public:
+ size_t size() const { return V.size(); }
+
+ BlockToIndexMapping(Function &F) {
+ for (BasicBlock &BB : F)
+ V.push_back(&BB);
+ llvm::sort(V);
+ }
+
+ size_t blockToIndex(BasicBlock const *BB) const {
+ auto *I = llvm::lower_bound(V, BB);
+ assert(I != V.end() && *I == BB && "BasicBlockNumberng: Unknown block");
+ return I - V.begin();
+ }
+
+ BasicBlock *indexToBlock(unsigned Index) const { return V[Index]; }
+};
+
+// The SuspendCrossingInfo maintains data that allows to answer a question
+// whether given two BasicBlocks A and B there is a path from A to B that
+// passes through a suspend point.
+//
+// For every basic block 'i' it maintains a BlockData that consists of:
+// Consumes: a bit vector which contains a set of indices of blocks that can
+// reach block 'i'. A block can trivially reach itself.
+// Kills: a bit vector which contains a set of indices of blocks that can
+// reach block 'i' but there is a path crossing a suspend point
+// not repeating 'i' (path to 'i' without cycles containing 'i').
+// Suspend: a boolean indicating whether block 'i' contains a suspend point.
+// End: a boolean indicating whether block 'i' contains a coro.end intrinsic.
+// KillLoop: There is a path from 'i' to 'i' not otherwise repeating 'i' that
+// crosses a suspend point.
+//
+class SuspendCrossingInfo {
+ BlockToIndexMapping Mapping;
+
+ struct BlockData {
+ BitVector Consumes;
+ BitVector Kills;
+ bool Suspend = false;
+ bool End = false;
+ bool KillLoop = false;
+ bool Changed = false;
+ };
+ SmallVector<BlockData, 32> Block;
+
+ iterator_range<pred_iterator> predecessors(BlockData const &BD) const {
+ BasicBlock *BB = Mapping.indexToBlock(&BD - &Block[0]);
+ return llvm::predecessors(BB);
+ }
+
+ BlockData &getBlockData(BasicBlock *BB) {
+ return Block[Mapping.blockToIndex(BB)];
+ }
+
+ /// Compute the BlockData for the current function in one iteration.
+ /// Initialize - Whether this is the first iteration, we can optimize
+ /// the initial case a little bit by manual loop switch.
+ /// Returns whether the BlockData changes in this iteration.
+ template <bool Initialize = false>
+ bool computeBlockData(const ReversePostOrderTraversal<Function *> &RPOT);
+
+public:
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+ // Print order is in RPO
+ void dump() const;
+ void dump(StringRef Label, BitVector const &BV,
+ const ReversePostOrderTraversal<Function *> &RPOT) const;
+#endif
+
+ SuspendCrossingInfo(Function &F,
+ const SmallVectorImpl<AnyCoroSuspendInst *> &CoroSuspends,
+ const SmallVectorImpl<AnyCoroEndInst *> &CoroEnds);
+
+ /// Returns true if there is a path from \p From to \p To crossing a suspend
+ /// point without crossing \p From a 2nd time.
+ bool hasPathCrossingSuspendPoint(BasicBlock *From, BasicBlock *To) const;
+
+ /// Returns true if there is a path from \p From to \p To crossing a suspend
+ /// point without crossing \p From a 2nd time. If \p From is the same as \p To
+ /// this will also check if there is a looping path crossing a suspend point.
+ bool hasPathOrLoopCrossingSuspendPoint(BasicBlock *From,
+ BasicBlock *To) const;
+
+ bool isDefinitionAcrossSuspend(BasicBlock *DefBB, User *U) const {
+ auto *I = cast<Instruction>(U);
+
+ // We rewrote PHINodes, so that only the ones with exactly one incoming
+ // value need to be analyzed.
+ if (auto *PN = dyn_cast<PHINode>(I))
+ if (PN->getNumIncomingValues() > 1)
+ return false;
+
+ BasicBlock *UseBB = I->getParent();
+
+ // As a special case, treat uses by an llvm.coro.suspend.retcon or an
+ // llvm.coro.suspend.async as if they were uses in the suspend's single
+ // predecessor: the uses conceptually occur before the suspend.
+ if (isa<CoroSuspendRetconInst>(I) || isa<CoroSuspendAsyncInst>(I)) {
+ UseBB = UseBB->getSinglePredecessor();
+ assert(UseBB && "should have split coro.suspend into its own block");
+ }
+
+ return hasPathCrossingSuspendPoint(DefBB, UseBB);
+ }
+
+ bool isDefinitionAcrossSuspend(Argument &A, User *U) const {
+ return isDefinitionAcrossSuspend(&A.getParent()->getEntryBlock(), U);
+ }
+
+ bool isDefinitionAcrossSuspend(Instruction &I, User *U) const {
+ auto *DefBB = I.getParent();
+
+ // As a special case, treat values produced by an llvm.coro.suspend.*
+ // as if they were defined in the single successor: the uses
+ // conceptually occur after the suspend.
+ if (isa<AnyCoroSuspendInst>(I)) {
+ DefBB = DefBB->getSingleSuccessor();
+ assert(DefBB && "should have split coro.suspend into its own block");
+ }
+
+ return isDefinitionAcrossSuspend(DefBB, U);
+ }
+
+ bool isDefinitionAcrossSuspend(Value &V, User *U) const {
+ if (auto *Arg = dyn_cast<Argument>(&V))
+ return isDefinitionAcrossSuspend(*Arg, U);
+ if (auto *Inst = dyn_cast<Instruction>(&V))
+ return isDefinitionAcrossSuspend(*Inst, U);
+
+ llvm_unreachable(
+ "Coroutine could only collect Argument and Instruction now.");
+ }
+
+ bool isDefinitionAcrossSuspend(Value &V) const {
+ if (auto *Arg = dyn_cast<Argument>(&V)) {
+ for (User *U : Arg->users())
+ if (isDefinitionAcrossSuspend(*Arg, U))
+ return true;
+ } else if (auto *Inst = dyn_cast<Instruction>(&V)) {
+ for (User *U : Inst->users())
+ if (isDefinitionAcrossSuspend(*Inst, U))
+ return true;
+ }
+
+ llvm_unreachable(
+ "Coroutine could only collect Argument and Instruction now.");
+ }
+};
+
+} // namespace llvm
+
+#endif // LLVM_TRANSFORMS_COROUTINES_SUSPENDCROSSINGINFO_H
More information about the llvm-commits
mailing list