Skip to content

Commit

Permalink
[RemoveDIs] Update Coroutine passes to handle DPValues (#74480)
Browse files Browse the repository at this point in the history 
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.

`getInsertPtAfterFramePtr` now returns an iterator so we don't lose
debug-info-communicating bits.

---

Depends on #73500, #74090, #74091.
  • Loading branch information
@OCHyams
OCHyams authored Dec 13, 2023
1 parent e418988 commit fd8fa31
Show file tree
Hide file tree
Showing 13 changed files with 197 additions and 61 deletions.
2 changes: 2 additions & 0 deletions llvm/lib/IR/BasicBlock.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,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);
}
Expand Down
190 changes: 146 additions & 44 deletions llvm/lib/Transforms/Coroutines/CoroFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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,
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
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -2848,7 +2886,7 @@ void coro::salvageDebugInfo(
SkipOutermostLoad = false;
}
if (!Storage)
return;
return std::nullopt;

auto *StorageAsArg = dyn_cast<Argument>(Storage);
const bool IsSwiftAsyncArg =
Expand Down Expand Up @@ -2884,8 +2922,30 @@ void coro::salvageDebugInfo(
Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);
}

DVI->replaceVariableLocationOp(OriginalStorage, Storage);
DVI->setExpression(Expr);
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
// dbg.value since it does not have the same function wide guarantees that
// dbg.declare does.
Expand All @@ -2896,7 +2956,44 @@ 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,
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);
}
}
}

Expand Down Expand Up @@ -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));
Expand Down
16 changes: 11 additions & 5 deletions llvm/lib/Transforms/Coroutines/CoroInternal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ void replaceCoroFree(CoroIdInst *CoroId, bool Elide);
/// OptimizeFrame is false.
void salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool IsEntryPoint);
DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool IsEntryPoint);
void salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap, DPValue &DPV,
bool OptimizeFrame, bool UseEntryValue);

// Keeps data and helper functions for lowering coroutine intrinsics.
struct LowererBase {
Expand Down Expand Up @@ -240,10 +243,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.
Expand Down
Loading

0 comments on commit fd8fa31

Please sign in to comment.