Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RemoveDIs] Update Coroutine passes to handle DPValues #74480

Merged
merged 3 commits into from
Dec 13, 2023
Merged

[RemoveDIs] Update Coroutine passes to handle DPValues #74480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
862f553
[RemoveDIs] Update Coroutine passes to handle DPValues
OCHyams Dec 4, 2023
e63eb59
coro: address feedback
OCHyams Dec 6, 2023
6e03e01
format
OCHyams Dec 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions llvm/lib/IR/BasicBlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
190 changes: 146 additions & 44 deletions llvm/lib/Transforms/Coroutines/CoroFrame.cpp
View file
Open in desktop
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),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am out of the loop of the RemoveDIs project, who owns this memory? The BB?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BasicBlock owns it indirectly. It's tied to the lifetime of the block, while not actually being part of the Value hierarchy. The DPMarker owns the DPValue memory, and BasicBlock owns the DPMarkers. Just like instructions, eraseFromParent can be called to delete these.

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
Open in desktop
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