[llvm] [RemoveDIs] Update Coroutine passes to handle DPValues (PR #74480)
Orlando Cazalet-Hyams via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 13 03:30:13 PST 2023
https://github.com/OCHyams updated https://github.com/llvm/llvm-project/pull/74480
>From 862f55356667b92b92d72a9993e7215c70bb94b0 Mon Sep 17 00:00:00 2001
From: OCHyams <orlando.hyams at sony.com>
Date: Mon, 4 Dec 2023 16:49:34 +0000
Subject: [PATCH 1/3] [RemoveDIs] Update Coroutine passes to handle DPValues
As part of the RemoveDIs project, transitioning to non-instruction debug info,
all debug intrinsic handling code needs to be duplicated to handle DPValues.
--try-experimental-debuginfo-iterators enables the new debug mode in tests if
the CMake option has been enabled.
---
llvm/lib/IR/BasicBlock.cpp | 2 +
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 182 ++++++++++++++----
llvm/lib/Transforms/Coroutines/CoroInternal.h | 15 +-
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 28 ++-
.../Transforms/Coroutines/coro-debug-O2.ll | 1 +
.../Coroutines/coro-debug-coro-frame.ll | 1 +
...coro-debug-dbg.values-not_used_in_frame.ll | 1 +
.../Coroutines/coro-debug-dbg.values.ll | 1 +
.../Coroutines/coro-debug-frame-variable.ll | 1 +
.../coro-debug-spill-dbg.declare.ll | 1 +
llvm/test/Transforms/Coroutines/coro-debug.ll | 1 +
.../Transforms/Coroutines/coro-split-dbg.ll | 1 +
.../Transforms/Coroutines/swift-async-dbg.ll | 7 +
13 files changed, 190 insertions(+), 52 deletions(-)
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index f364c56a42c528..7a84c298552f50 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -1088,6 +1088,8 @@ void BasicBlock::insertDPValueBefore(DPValue *DPV,
// shouldn't be generated at times when there's no terminator.
assert(Where != end());
assert(Where->getParent() == this);
+ if (!Where->DbgMarker)
+ createMarker(Where);
bool InsertAtHead = Where.getHeadBit();
Where->DbgMarker->insertDPValue(DPV, InsertAtHead);
}
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 6b85de15947c27..3f201b5d910b45 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -964,12 +964,17 @@ static void cacheDIVar(FrameDataInfo &FrameData,
continue;
SmallVector<DbgDeclareInst *, 1> DDIs;
- findDbgDeclares(DDIs, V);
- auto *I = llvm::find_if(DDIs, [](DbgDeclareInst *DDI) {
- return DDI->getExpression()->getNumElements() == 0;
- });
- if (I != DDIs.end())
- DIVarCache.insert({V, (*I)->getVariable()});
+ SmallVector<DPValue *, 1> DPVs;
+ findDbgDeclares(DDIs, V, &DPVs);
+ auto CacheIt = [&DIVarCache, V](auto &Container) {
+ auto *I = llvm::find_if(Container, [](auto *DDI) {
+ return DDI->getExpression()->getNumElements() == 0;
+ });
+ if (I != Container.end())
+ DIVarCache.insert({V, (*I)->getVariable()});
+ };
+ CacheIt(DDIs);
+ CacheIt(DPVs);
}
}
@@ -1121,15 +1126,25 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape,
"Coroutine with switch ABI should own Promise alloca");
SmallVector<DbgDeclareInst *, 1> DIs;
- findDbgDeclares(DIs, PromiseAlloca);
- if (DIs.empty())
+ SmallVector<DPValue *, 1> DPVs;
+ findDbgDeclares(DIs, PromiseAlloca, &DPVs);
+
+ DILocalVariable *PromiseDIVariable = nullptr;
+ DILocation *DILoc = nullptr;
+ if (!DIs.empty()) {
+ DbgDeclareInst *PromiseDDI = DIs.front();
+ PromiseDIVariable = PromiseDDI->getVariable();
+ DILoc = PromiseDDI->getDebugLoc().get();
+ } else if (!DPVs.empty()) {
+ DPValue *PromiseDPV = DPVs.front();
+ PromiseDIVariable = PromiseDPV->getVariable();
+ DILoc = PromiseDPV->getDebugLoc().get();
+ } else {
return;
+ }
- DbgDeclareInst *PromiseDDI = DIs.front();
- DILocalVariable *PromiseDIVariable = PromiseDDI->getVariable();
DILocalScope *PromiseDIScope = PromiseDIVariable->getScope();
DIFile *DFile = PromiseDIScope->getFile();
- DILocation *DILoc = PromiseDDI->getDebugLoc().get();
unsigned LineNum = PromiseDIVariable->getLine();
DICompositeType *FrameDITy = DBuilder.createStructType(
@@ -1243,7 +1258,7 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape,
auto *FrameDIVar = DBuilder.createAutoVariable(PromiseDIScope, "__coro_frame",
DFile, LineNum, FrameDITy,
true, DINode::FlagArtificial);
- assert(FrameDIVar->isValidLocationForIntrinsic(PromiseDDI->getDebugLoc()));
+ assert(FrameDIVar->isValidLocationForIntrinsic(DILoc));
// Subprogram would have ContainedNodes field which records the debug
// variables it contained. So we need to add __coro_frame to the
@@ -1261,9 +1276,17 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape,
7, (MDTuple::get(F.getContext(), RetainedNodesVec)));
}
- DBuilder.insertDeclare(Shape.FramePtr, FrameDIVar,
- DBuilder.createExpression(), DILoc,
- Shape.getInsertPtAfterFramePtr());
+ if (UseNewDbgInfoFormat) {
+ DPValue *NewDPV = new DPValue(ValueAsMetadata::get(Shape.FramePtr),
+ FrameDIVar, DBuilder.createExpression(),
+ DILoc, DPValue::LocationType::Declare);
+ BasicBlock::iterator It = Shape.getInsertPtAfterFramePtr();
+ It->getParent()->insertDPValueBefore(NewDPV, It);
+ } else {
+ DBuilder.insertDeclare(Shape.FramePtr, FrameDIVar,
+ DBuilder.createExpression(), DILoc,
+ &*Shape.getInsertPtAfterFramePtr());
+ }
}
// Build a struct that will keep state for an active coroutine.
@@ -1771,7 +1794,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
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()->getIterator();
+ InsertPt = Shape.getInsertPtAfterFramePtr();
// If we're spilling an Argument, make sure we clear 'nocapture'
// from the coroutine function.
@@ -1788,7 +1811,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
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()->getIterator();
+ 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.
@@ -1843,7 +1866,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
SpillAlignment, E.first->getName() + Twine(".reload"));
SmallVector<DbgDeclareInst *, 1> DIs;
- findDbgDeclares(DIs, Def);
+ SmallVector<DPValue *, 1> DPVs;
+ findDbgDeclares(DIs, Def, &DPVs);
// Try best to find dbg.declare. If the spill is a temp, there may not
// be a direct dbg.declare. Walk up the load chain to find one from an
// alias.
@@ -1858,24 +1882,36 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
if (!isa<AllocaInst, LoadInst>(CurDef))
break;
DIs.clear();
- findDbgDeclares(DIs, CurDef);
+ DPVs.clear();
+ findDbgDeclares(DIs, CurDef, &DPVs);
}
}
- for (DbgDeclareInst *DDI : DIs) {
+ auto SalvageOne = [&](auto *DDI) {
bool AllowUnresolved = false;
// This dbg.declare is preserved for all coro-split function
// fragments. It will be unreachable in the main function, and
// processed by coro::salvageDebugInfo() by CoroCloner.
- DIBuilder(*CurrentBlock->getParent()->getParent(), AllowUnresolved)
- .insertDeclare(CurrentReload, DDI->getVariable(),
- DDI->getExpression(), DDI->getDebugLoc(),
- &*Builder.GetInsertPoint());
+ if (UseNewDbgInfoFormat) {
+ DPValue *NewDPV =
+ new DPValue(ValueAsMetadata::get(CurrentReload),
+ DDI->getVariable(), DDI->getExpression(),
+ DDI->getDebugLoc(), DPValue::LocationType::Declare);
+ Builder.GetInsertPoint()->getParent()->insertDPValueBefore(
+ NewDPV, Builder.GetInsertPoint());
+ } else {
+ DIBuilder(*CurrentBlock->getParent()->getParent(), AllowUnresolved)
+ .insertDeclare(CurrentReload, DDI->getVariable(),
+ DDI->getExpression(), DDI->getDebugLoc(),
+ &*Builder.GetInsertPoint());
+ }
// This dbg.declare is for the main function entry point. It
// will be deleted in all coro-split functions.
coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame,
false /*UseEntryValue*/);
- }
+ };
+ for_each(DIs, SalvageOne);
+ for_each(DPVs, SalvageOne);
}
// If we have a single edge PHINode, remove it and replace it with a
@@ -1893,6 +1929,10 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
// Replace all uses of CurrentValue in the current instruction with
// reload.
U->replaceUsesOfWith(Def, CurrentReload);
+ // Instructions are added to Def's user list if the attached
+ // debug records use Def. Update those now.
+ for (auto &DPV: U->getDbgValueRange())
+ DPV.replaceVariableLocationOp(Def, CurrentReload, true);
}
}
@@ -1943,9 +1983,12 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
G->setName(Alloca->getName() + Twine(".reload.addr"));
SmallVector<DbgVariableIntrinsic *, 4> DIs;
- findDbgUsers(DIs, Alloca);
+ SmallVector<DPValue *> DPValues;
+ findDbgUsers(DIs, Alloca, &DPValues);
for (auto *DVI : DIs)
DVI->replaceUsesOfWith(Alloca, G);
+ for (auto *DPV : DPValues)
+ DPV->replaceVariableLocationOp(Alloca, G);
for (Instruction *I : UsersToUpdate) {
// It is meaningless to retain the lifetime intrinsics refer for the
@@ -1959,7 +2002,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
I->replaceUsesOfWith(Alloca, G);
}
}
- Builder.SetInsertPoint(Shape.getInsertPtAfterFramePtr());
+ Builder.SetInsertPoint(&*Shape.getInsertPtAfterFramePtr());
for (const auto &A : FrameData.Allocas) {
AllocaInst *Alloca = A.Alloca;
if (A.MayWriteBeforeCoroBegin) {
@@ -2020,7 +2063,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
isa<BitCastInst>(Inst);
});
if (HasAccessingPromiseBeforeCB) {
- Builder.SetInsertPoint(Shape.getInsertPtAfterFramePtr());
+ Builder.SetInsertPoint(&*Shape.getInsertPtAfterFramePtr());
auto *G = GetFramePointer(PA);
auto *Value = Builder.CreateLoad(PA->getAllocatedType(), PA);
Builder.CreateStore(Value, G);
@@ -2802,21 +2845,16 @@ static void collectFrameAlloca(AllocaInst *AI, coro::Shape &Shape,
Visitor.getMayWriteBeforeCoroBegin());
}
-void coro::salvageDebugInfo(
- SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool UseEntryValue) {
- Function *F = DVI->getFunction();
+static std::optional<std::pair<Value *, DIExpression *>>
+salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
+ bool OptimizeFrame, bool UseEntryValue, Function *F,
+ Value *Storage, DIExpression *Expr,
+ bool SkipOutermostLoad) {
IRBuilder<> Builder(F->getContext());
auto InsertPt = F->getEntryBlock().getFirstInsertionPt();
while (isa<IntrinsicInst>(InsertPt))
++InsertPt;
Builder.SetInsertPoint(&F->getEntryBlock(), InsertPt);
- DIExpression *Expr = DVI->getExpression();
- // Follow the pointer arithmetic all the way to the incoming
- // function argument and convert into a DIExpression.
- bool SkipOutermostLoad = !isa<DbgValueInst>(DVI);
- Value *Storage = DVI->getVariableLocationOp(0);
- Value *OriginalStorage = Storage;
while (auto *Inst = dyn_cast_or_null<Instruction>(Storage)) {
if (auto *LdInst = dyn_cast<LoadInst>(Inst)) {
@@ -2848,7 +2886,7 @@ void coro::salvageDebugInfo(
SkipOutermostLoad = false;
}
if (!Storage)
- return;
+ return std::nullopt;
auto *StorageAsArg = dyn_cast<Argument>(Storage);
const bool IsSwiftAsyncArg =
@@ -2884,6 +2922,28 @@ void coro::salvageDebugInfo(
Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);
}
+ return {{Storage, Expr}};
+}
+
+void coro::salvageDebugInfo(
+ SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
+ DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool UseEntryValue) {
+
+ Function *F = DVI->getFunction();
+ // Follow the pointer arithmetic all the way to the incoming
+ // function argument and convert into a DIExpression.
+ bool SkipOutermostLoad = !isa<DbgValueInst>(DVI);
+ Value *OriginalStorage = DVI->getVariableLocationOp(0);
+
+ auto SalvagedInfo = ::salvageDebugInfoImpl(
+ ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
+ DVI->getExpression(), SkipOutermostLoad);
+ if (!SalvagedInfo)
+ return;
+
+ Value *Storage = SalvagedInfo->first;
+ DIExpression *Expr = SalvagedInfo->second;
+
DVI->replaceVariableLocationOp(OriginalStorage, Storage);
DVI->setExpression(Expr);
// We only hoist dbg.declare today since it doesn't make sense to hoist
@@ -2900,6 +2960,43 @@ void coro::salvageDebugInfo(
}
}
+void coro::salvageDebugInfo(
+ SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap, DPValue *DPV,
+ bool OptimizeFrame, bool UseEntryValue) {
+
+ Function *F = DPV->getFunction();
+ // Follow the pointer arithmetic all the way to the incoming
+ // function argument and convert into a DIExpression.
+ bool SkipOutermostLoad = DPV->getType() == DPValue::LocationType::Declare;
+ Value *OriginalStorage = DPV->getVariableLocationOp(0);
+
+ auto SalvagedInfo = ::salvageDebugInfoImpl(
+ ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
+ DPV->getExpression(), SkipOutermostLoad);
+ if (!SalvagedInfo)
+ return;
+
+ Value *Storage = SalvagedInfo->first;
+ DIExpression *Expr = SalvagedInfo->second;
+
+ DPV->replaceVariableLocationOp(OriginalStorage, Storage);
+ DPV->setExpression(Expr);
+ // We only hoist dbg.declare today since it doesn't make sense to hoist
+ // dbg.value since it does not have the same function wide guarantees that
+ // dbg.declare does.
+ if (DPV->getType() == DPValue::LocationType::Declare) {
+ std::optional<BasicBlock::iterator> InsertPt;
+ if (auto *I = dyn_cast<Instruction>(Storage))
+ InsertPt = I->getInsertionPointAfterDef();
+ else if (isa<Argument>(Storage))
+ InsertPt = F->getEntryBlock().begin();
+ if (InsertPt) {
+ DPV->removeFromParent();
+ (*InsertPt)->getParent()->insertDPValueBefore(DPV, *InsertPt);
+ }
+ }
+}
+
static void doRematerializations(
Function &F, SuspendCrossingInfo &Checker,
const std::function<bool(Instruction &)> &MaterializableCallback) {
@@ -3087,10 +3184,15 @@ void coro::buildCoroutineFrame(
for (auto &Iter : FrameData.Spills) {
auto *V = Iter.first;
SmallVector<DbgValueInst *, 16> DVIs;
- findDbgValues(DVIs, V);
+ SmallVector<DPValue *, 16> DPVs;
+ findDbgValues(DVIs, V, &DPVs);
for (DbgValueInst *DVI : DVIs)
if (Checker.isDefinitionAcrossSuspend(*V, DVI))
FrameData.Spills[V].push_back(DVI);
+ // Add the instructions which carry debug info that is in the frame.
+ for (DPValue *DPV : DPVs)
+ if (Checker.isDefinitionAcrossSuspend(*V, DPV->Marker->MarkedInstr))
+ FrameData.Spills[V].push_back(DPV->Marker->MarkedInstr);
}
LLVM_DEBUG(dumpSpills("Spills", FrameData.Spills));
diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
index 0856c4925cc5d5..1022f7a2979f5e 100644
--- a/llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -33,6 +33,10 @@ void replaceCoroFree(CoroIdInst *CoroId, bool Elide);
void salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool IsEntryPoint);
+/// See overload.
+void salvageDebugInfo(
+ SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap, DPValue *DPV,
+ bool OptimizeFrame, bool UseEntryValue);
// Keeps data and helper functions for lowering coroutine intrinsics.
struct LowererBase {
@@ -240,10 +244,13 @@ struct LLVM_LIBRARY_VISIBILITY Shape {
return nullptr;
}
- Instruction *getInsertPtAfterFramePtr() const {
- if (auto *I = dyn_cast<Instruction>(FramePtr))
- return I->getNextNode();
- return &cast<Argument>(FramePtr)->getParent()->getEntryBlock().front();
+ BasicBlock::iterator getInsertPtAfterFramePtr() const {
+ if (auto *I = dyn_cast<Instruction>(FramePtr)) {
+ BasicBlock::iterator It = std::next(I->getIterator());
+ It.setHeadBit(true); // Copy pre-RemoveDIs behaviour.
+ return It;
+ }
+ return cast<Argument>(FramePtr)->getParent()->getEntryBlock().begin();
}
/// Allocate memory according to the rules of the active lowering.
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 244580f503d5b4..504aa912ac2668 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -726,11 +726,14 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape,
/// Returns all DbgVariableIntrinsic in F.
static SmallVector<DbgVariableIntrinsic *, 8>
-collectDbgVariableIntrinsics(Function &F) {
+collectDbgVariableIntrinsics(Function &F, SmallVector<DPValue *> &DPValues) {
SmallVector<DbgVariableIntrinsic *, 8> Intrinsics;
- for (auto &I : instructions(F))
+ for (auto &I : instructions(F)) {
+ for (DPValue &DPV : I.getDbgValueRange())
+ DPValues.push_back(&DPV);
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
Intrinsics.push_back(DVI);
+ }
return Intrinsics;
}
@@ -739,8 +742,9 @@ void CoroCloner::replaceSwiftErrorOps() {
}
void CoroCloner::salvageDebugInfo() {
+ SmallVector<DPValue *> DPValues;
SmallVector<DbgVariableIntrinsic *, 8> Worklist =
- collectDbgVariableIntrinsics(*NewF);
+ collectDbgVariableIntrinsics(*NewF, DPValues);
SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
// Only 64-bit ABIs have a register we can refer to with the entry value.
@@ -749,6 +753,9 @@ void CoroCloner::salvageDebugInfo() {
for (DbgVariableIntrinsic *DVI : Worklist)
coro::salvageDebugInfo(ArgToAllocaMap, DVI, Shape.OptimizeFrame,
UseEntryValue);
+ for (DPValue *DPV : DPValues)
+ coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame,
+ UseEntryValue);
// Remove all salvaged dbg.declare intrinsics that became
// either unreachable or stale due to the CoroSplit transformation.
@@ -757,7 +764,7 @@ void CoroCloner::salvageDebugInfo() {
return !isPotentiallyReachable(&NewF->getEntryBlock(), BB, nullptr,
&DomTree);
};
- for (DbgVariableIntrinsic *DVI : Worklist) {
+ auto RemoveOne = [&](auto *DVI) {
if (IsUnreachableBlock(DVI->getParent()))
DVI->eraseFromParent();
else if (isa_and_nonnull<AllocaInst>(DVI->getVariableLocationOp(0))) {
@@ -770,7 +777,9 @@ void CoroCloner::salvageDebugInfo() {
if (!Uses)
DVI->eraseFromParent();
}
- }
+ };
+ for_each(Worklist, RemoveOne);
+ for_each(DPValues, RemoveOne);
}
void CoroCloner::replaceEntryBlock() {
@@ -1243,7 +1252,7 @@ static void updateCoroFrame(coro::Shape &Shape, Function *ResumeFn,
Function *DestroyFn, Function *CleanupFn) {
assert(Shape.ABI == coro::ABI::Switch);
- IRBuilder<> Builder(Shape.getInsertPtAfterFramePtr());
+ IRBuilder<> Builder(&*Shape.getInsertPtAfterFramePtr());
auto *ResumeAddr = Builder.CreateStructGEP(
Shape.FrameTy, Shape.FramePtr, coro::Shape::SwitchFieldIndex::Resume,
@@ -2039,10 +2048,13 @@ splitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
// original function. The Cloner has already salvaged debug info in the new
// coroutine funclets.
SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
- for (auto *DDI : collectDbgVariableIntrinsics(F))
+ SmallVector<DPValue *> DPValues;
+ for (auto *DDI : collectDbgVariableIntrinsics(F, DPValues))
coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame,
false /*UseEntryValue*/);
-
+ for (DPValue *DPV : DPValues)
+ coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame,
+ false /*UseEntryValue*/);
return Shape;
}
diff --git a/llvm/test/Transforms/Coroutines/coro-debug-O2.ll b/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
index a668bd1a521960..3f9af30a92e343 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
@@ -1,4 +1,5 @@
; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split<reuse-storage>),function(sroa)' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split<reuse-storage>),function(sroa)' -S | FileCheck %s
; Checks whether the dbg.declare for `__promise` remains valid under O2.
diff --git a/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll b/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll
index 7f5679e4d522fd..2978f85be23854 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll
@@ -1,4 +1,5 @@
; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
; Checks whether the dbg.declare for `__coro_frame` are created.
diff --git a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
index 1b9a1bd63a2e3e..79793dc293d0d9 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
@@ -1,5 +1,6 @@
; Tests whether resume function would remain dbg.value infomation if corresponding values are not used in the frame.
; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
;
; This file is based on coro-debug-frame-variable.ll.
; CHECK: define internal fastcc void @f.resume(ptr noundef nonnull align 16 dereferenceable(80) %begin) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
diff --git a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
index 54cdde8ae4ac0c..47b2ddafcfc650 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
@@ -1,5 +1,6 @@
; Tests whether resume function would remain dbg.value infomation.
; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
;
; This file is based on coro-debug-frame-variable.ll.
; CHECK-LABEL: define void @f(
diff --git a/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll b/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
index 37b4126ce37304..19a89fefd5269d 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
@@ -1,4 +1,5 @@
; RUN: opt < %s -passes='default<O0>' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='default<O0>' -S | FileCheck %s
; Define a function 'f' that resembles the Clang frontend's output for the
; following C++ coroutine:
diff --git a/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll b/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll
index e7a271a96ead12..c943ea5ca22ec0 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll
@@ -1,6 +1,7 @@
; Test spilling a temp generates dbg.declare in resume/destroy/cleanup functions.
;
; RUN: opt < %s -passes='cgscc(coro-split)' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='cgscc(coro-split)' -S | FileCheck %s
;
; The test case simulates a coroutine method in a class.
;
diff --git a/llvm/test/Transforms/Coroutines/coro-debug.ll b/llvm/test/Transforms/Coroutines/coro-debug.ll
index 064693c80ad233..4792825f4ce080 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug.ll
@@ -1,5 +1,6 @@
; Tests that debug information is sane after coro-split
; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
source_filename = "simple-repro.c"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/Transforms/Coroutines/coro-split-dbg.ll b/llvm/test/Transforms/Coroutines/coro-split-dbg.ll
index 184d4a564ab723..7d95be308a5f49 100644
--- a/llvm/test/Transforms/Coroutines/coro-split-dbg.ll
+++ b/llvm/test/Transforms/Coroutines/coro-split-dbg.ll
@@ -1,6 +1,7 @@
; Make sure that coro-split correctly deals with debug information.
; The test here is simply that it does not result in bad IR that will crash opt.
; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -disable-output
+; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -disable-output
source_filename = "coro.c"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/Transforms/Coroutines/swift-async-dbg.ll b/llvm/test/Transforms/Coroutines/swift-async-dbg.ll
index af7d816c128616..74edf7a3f3a540 100644
--- a/llvm/test/Transforms/Coroutines/swift-async-dbg.ll
+++ b/llvm/test/Transforms/Coroutines/swift-async-dbg.ll
@@ -2,6 +2,13 @@
; RUN: opt -mtriple='x86_64' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s
; RUN: opt -mtriple='i386-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY
; RUN: opt -mtriple='armv7-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY
+
+;; Replicate those tests with non-instruction debug markers.
+; RUN: opt --try-experimental-debuginfo-iterators -mtriple='arm64-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -mtriple='x86_64' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -mtriple='i386-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY
+; RUN: opt --try-experimental-debuginfo-iterators -mtriple='armv7-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY
+
; NOENTRY-NOT: OP_llvm_entry_value
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
>From e63eb59a49699692bddc61e5ceee8c93bca541ce Mon Sep 17 00:00:00 2001
From: OCHyams <orlando.hyams at sony.com>
Date: Wed, 6 Dec 2023 10:51:45 +0000
Subject: [PATCH 2/3] coro: address feedback
---
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 48 +++++++++----------
llvm/lib/Transforms/Coroutines/CoroInternal.h | 5 +-
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 23 +++++----
3 files changed, 37 insertions(+), 39 deletions(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 3f201b5d910b45..a1ecb7a9983271 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1907,7 +1907,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
}
// This dbg.declare is for the main function entry point. It
// will be deleted in all coro-split functions.
- coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame,
+ coro::salvageDebugInfo(ArgToAllocaMap, *DDI, Shape.OptimizeFrame,
false /*UseEntryValue*/);
};
for_each(DIs, SalvageOne);
@@ -2845,7 +2845,7 @@ static void collectFrameAlloca(AllocaInst *AI, coro::Shape &Shape,
Visitor.getMayWriteBeforeCoroBegin());
}
-static std::optional<std::pair<Value *, DIExpression *>>
+static std::optional<std::pair<Value &, DIExpression &>>
salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
bool OptimizeFrame, bool UseEntryValue, Function *F,
Value *Storage, DIExpression *Expr,
@@ -2922,30 +2922,30 @@ salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);
}
- return {{Storage, Expr}};
+ return {{*Storage, *Expr}};
}
void coro::salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool UseEntryValue) {
+ DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool UseEntryValue) {
- Function *F = DVI->getFunction();
+ Function *F = DVI.getFunction();
// Follow the pointer arithmetic all the way to the incoming
// function argument and convert into a DIExpression.
bool SkipOutermostLoad = !isa<DbgValueInst>(DVI);
- Value *OriginalStorage = DVI->getVariableLocationOp(0);
+ Value *OriginalStorage = DVI.getVariableLocationOp(0);
auto SalvagedInfo = ::salvageDebugInfoImpl(
ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
- DVI->getExpression(), SkipOutermostLoad);
+ DVI.getExpression(), SkipOutermostLoad);
if (!SalvagedInfo)
return;
- Value *Storage = SalvagedInfo->first;
- DIExpression *Expr = SalvagedInfo->second;
+ Value *Storage = &SalvagedInfo->first;
+ DIExpression *Expr = &SalvagedInfo->second;
- DVI->replaceVariableLocationOp(OriginalStorage, Storage);
- DVI->setExpression(Expr);
+ DVI.replaceVariableLocationOp(OriginalStorage, Storage);
+ DVI.setExpression(Expr);
// We only hoist dbg.declare today since it doesn't make sense to hoist
// dbg.value since it does not have the same function wide guarantees that
// dbg.declare does.
@@ -2956,43 +2956,43 @@ void coro::salvageDebugInfo(
else if (isa<Argument>(Storage))
InsertPt = F->getEntryBlock().begin();
if (InsertPt)
- DVI->moveBefore(*(*InsertPt)->getParent(), *InsertPt);
+ DVI.moveBefore(*(*InsertPt)->getParent(), *InsertPt);
}
}
void coro::salvageDebugInfo(
- SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap, DPValue *DPV,
+ SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap, DPValue &DPV,
bool OptimizeFrame, bool UseEntryValue) {
- Function *F = DPV->getFunction();
+ Function *F = DPV.getFunction();
// Follow the pointer arithmetic all the way to the incoming
// function argument and convert into a DIExpression.
- bool SkipOutermostLoad = DPV->getType() == DPValue::LocationType::Declare;
- Value *OriginalStorage = DPV->getVariableLocationOp(0);
+ bool SkipOutermostLoad = DPV.getType() == DPValue::LocationType::Declare;
+ Value *OriginalStorage = DPV.getVariableLocationOp(0);
auto SalvagedInfo = ::salvageDebugInfoImpl(
ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
- DPV->getExpression(), SkipOutermostLoad);
+ DPV.getExpression(), SkipOutermostLoad);
if (!SalvagedInfo)
return;
- Value *Storage = SalvagedInfo->first;
- DIExpression *Expr = SalvagedInfo->second;
+ Value *Storage = &SalvagedInfo->first;
+ DIExpression *Expr = &SalvagedInfo->second;
- DPV->replaceVariableLocationOp(OriginalStorage, Storage);
- DPV->setExpression(Expr);
+ DPV.replaceVariableLocationOp(OriginalStorage, Storage);
+ DPV.setExpression(Expr);
// We only hoist dbg.declare today since it doesn't make sense to hoist
// dbg.value since it does not have the same function wide guarantees that
// dbg.declare does.
- if (DPV->getType() == DPValue::LocationType::Declare) {
+ if (DPV.getType() == DPValue::LocationType::Declare) {
std::optional<BasicBlock::iterator> InsertPt;
if (auto *I = dyn_cast<Instruction>(Storage))
InsertPt = I->getInsertionPointAfterDef();
else if (isa<Argument>(Storage))
InsertPt = F->getEntryBlock().begin();
if (InsertPt) {
- DPV->removeFromParent();
- (*InsertPt)->getParent()->insertDPValueBefore(DPV, *InsertPt);
+ DPV.removeFromParent();
+ (*InsertPt)->getParent()->insertDPValueBefore(&DPV, *InsertPt);
}
}
}
diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
index 1022f7a2979f5e..fb16a4090689b4 100644
--- a/llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -32,10 +32,9 @@ void replaceCoroFree(CoroIdInst *CoroId, bool Elide);
/// OptimizeFrame is false.
void salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool IsEntryPoint);
-/// See overload.
+ DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool IsEntryPoint);
void salvageDebugInfo(
- SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap, DPValue *DPV,
+ SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap, DPValue &DPV,
bool OptimizeFrame, bool UseEntryValue);
// Keeps data and helper functions for lowering coroutine intrinsics.
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 504aa912ac2668..7758b52abc2046 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -725,16 +725,17 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape,
}
/// Returns all DbgVariableIntrinsic in F.
-static SmallVector<DbgVariableIntrinsic *, 8>
-collectDbgVariableIntrinsics(Function &F, SmallVector<DPValue *> &DPValues) {
+static std::pair<SmallVector<DbgVariableIntrinsic *, 8>, SmallVector<DPValue *>>
+collectDbgVariableIntrinsics(Function &F) {
SmallVector<DbgVariableIntrinsic *, 8> Intrinsics;
+ SmallVector<DPValue *> DPValues;
for (auto &I : instructions(F)) {
for (DPValue &DPV : I.getDbgValueRange())
DPValues.push_back(&DPV);
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
Intrinsics.push_back(DVI);
}
- return Intrinsics;
+ return {Intrinsics, DPValues};
}
void CoroCloner::replaceSwiftErrorOps() {
@@ -742,19 +743,17 @@ void CoroCloner::replaceSwiftErrorOps() {
}
void CoroCloner::salvageDebugInfo() {
- SmallVector<DPValue *> DPValues;
- SmallVector<DbgVariableIntrinsic *, 8> Worklist =
- collectDbgVariableIntrinsics(*NewF, DPValues);
+ auto [Worklist, DPValues] = collectDbgVariableIntrinsics(*NewF);
SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
// Only 64-bit ABIs have a register we can refer to with the entry value.
bool UseEntryValue =
llvm::Triple(OrigF.getParent()->getTargetTriple()).isArch64Bit();
for (DbgVariableIntrinsic *DVI : Worklist)
- coro::salvageDebugInfo(ArgToAllocaMap, DVI, Shape.OptimizeFrame,
+ coro::salvageDebugInfo(ArgToAllocaMap, *DVI, Shape.OptimizeFrame,
UseEntryValue);
for (DPValue *DPV : DPValues)
- coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame,
+ coro::salvageDebugInfo(ArgToAllocaMap, *DPV, Shape.OptimizeFrame,
UseEntryValue);
// Remove all salvaged dbg.declare intrinsics that became
@@ -2048,12 +2047,12 @@ splitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
// original function. The Cloner has already salvaged debug info in the new
// coroutine funclets.
SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
- SmallVector<DPValue *> DPValues;
- for (auto *DDI : collectDbgVariableIntrinsics(F, DPValues))
- coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame,
+ auto [DbgInsts, DPValues] = collectDbgVariableIntrinsics(F);
+ for (auto *DDI : DbgInsts)
+ coro::salvageDebugInfo(ArgToAllocaMap, *DDI, Shape.OptimizeFrame,
false /*UseEntryValue*/);
for (DPValue *DPV : DPValues)
- coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame,
+ coro::salvageDebugInfo(ArgToAllocaMap, *DPV, Shape.OptimizeFrame,
false /*UseEntryValue*/);
return Shape;
}
>From 6e03e019f3e126950ea66b73e14285861c7e8c3d Mon Sep 17 00:00:00 2001
From: OCHyams <orlando.hyams at sony.com>
Date: Wed, 13 Dec 2023 11:29:36 +0000
Subject: [PATCH 3/3] format
---
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index a1ecb7a9983271..f37b4dc938d304 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1931,7 +1931,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
U->replaceUsesOfWith(Def, CurrentReload);
// Instructions are added to Def's user list if the attached
// debug records use Def. Update those now.
- for (auto &DPV: U->getDbgValueRange())
+ for (auto &DPV : U->getDbgValueRange())
DPV.replaceVariableLocationOp(Def, CurrentReload, true);
}
}
More information about the llvm-commits
mailing list