Skip to content

Commit

Permalink
[DebugInfo][RemoveDIs][NFC] Split findDbgDeclares into two functions (#…
Browse files Browse the repository at this point in the history
…77478)

This patch follows on from comments on
#73498, implementing the
proposed split of findDbgDeclares into two separate functions for
DbgDeclareInsts and DPVDeclares, which return containers rather than
taking containers by reference.
  • Loading branch information
SLTozer committed Jan 15, 2024
1 parent c6dfb62 commit 3041198
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 48 deletions.
5 changes: 3 additions & 2 deletions llvm/include/llvm/IR/DebugInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ class Module;

/// Finds dbg.declare intrinsics declaring local variables as living in the
/// memory that 'V' points to.
void findDbgDeclares(SmallVectorImpl<DbgDeclareInst *> &DbgUsers, Value *V,
SmallVectorImpl<DPValue *> *DPValues = nullptr);
TinyPtrVector<DbgDeclareInst *> findDbgDeclares(Value *V);
/// As above, for DPVDeclares.
TinyPtrVector<DPValue *> findDPVDeclares(Value *V);

/// Finds the llvm.dbg.value intrinsics describing a value.
void findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
Expand Down
42 changes: 36 additions & 6 deletions llvm/lib/IR/DebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,42 @@ using namespace llvm;
using namespace llvm::at;
using namespace llvm::dwarf;

TinyPtrVector<DbgDeclareInst *> llvm::findDbgDeclares(Value *V) {
// This function is hot. Check whether the value has any metadata to avoid a
// DenseMap lookup.
if (!V->isUsedByMetadata())
return {};
auto *L = LocalAsMetadata::getIfExists(V);
if (!L)
return {};
auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L);
if (!MDV)
return {};

TinyPtrVector<DbgDeclareInst *> Declares;
for (User *U : MDV->users())
if (auto *DDI = dyn_cast<DbgDeclareInst>(U))
Declares.push_back(DDI);

return Declares;
}
TinyPtrVector<DPValue *> llvm::findDPVDeclares(Value *V) {
// This function is hot. Check whether the value has any metadata to avoid a
// DenseMap lookup.
if (!V->isUsedByMetadata())
return {};
auto *L = LocalAsMetadata::getIfExists(V);
if (!L)
return {};

TinyPtrVector<DPValue *> Declares;
for (DPValue *DPV : L->getAllDPValueUsers())
if (DPV->getType() == DPValue::LocationType::Declare)
Declares.push_back(DPV);

return Declares;
}

template <typename IntrinsicT,
DPValue::LocationType Type = DPValue::LocationType::Any>
static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result, Value *V,
Expand Down Expand Up @@ -97,12 +133,6 @@ static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result, Value *V,
}
}

void llvm::findDbgDeclares(SmallVectorImpl<DbgDeclareInst *> &DbgUsers,
Value *V, SmallVectorImpl<DPValue *> *DPValues) {
findDbgIntrinsics<DbgDeclareInst, DPValue::LocationType::Declare>(DbgUsers, V,
DPValues);
}

void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
Value *V, SmallVectorImpl<DPValue *> *DPValues) {
findDbgIntrinsics<DbgValueInst, DPValue::LocationType::Value>(DbgValues, V,
Expand Down
24 changes: 9 additions & 15 deletions llvm/lib/Transforms/Coroutines/CoroFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -963,18 +963,15 @@ static void cacheDIVar(FrameDataInfo &FrameData,
if (DIVarCache.contains(V))
continue;

SmallVector<DbgDeclareInst *, 1> DDIs;
SmallVector<DPValue *, 1> DPVs;
findDbgDeclares(DDIs, V, &DPVs);
auto CacheIt = [&DIVarCache, V](auto &Container) {
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);
CacheIt(findDbgDeclares(V));
CacheIt(findDPVDeclares(V));
}
}

Expand Down Expand Up @@ -1125,9 +1122,8 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape,
assert(PromiseAlloca &&
"Coroutine with switch ABI should own Promise alloca");

SmallVector<DbgDeclareInst *, 1> DIs;
SmallVector<DPValue *, 1> DPVs;
findDbgDeclares(DIs, PromiseAlloca, &DPVs);
TinyPtrVector<DbgDeclareInst *> DIs = findDbgDeclares(PromiseAlloca);
TinyPtrVector<DPValue *> DPVs = findDPVDeclares(PromiseAlloca);

DILocalVariable *PromiseDIVariable = nullptr;
DILocation *DILoc = nullptr;
Expand Down Expand Up @@ -1865,9 +1861,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
FrameTy->getElementType(FrameData.getFieldIndex(E.first)), GEP,
SpillAlignment, E.first->getName() + Twine(".reload"));

SmallVector<DbgDeclareInst *, 1> DIs;
SmallVector<DPValue *, 1> DPVs;
findDbgDeclares(DIs, Def, &DPVs);
TinyPtrVector<DbgDeclareInst *> DIs = findDbgDeclares(Def);
TinyPtrVector<DPValue *> DPVs = findDPVDeclares(Def);
// 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 @@ -1881,9 +1876,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
CurDef = LdInst->getPointerOperand();
if (!isa<AllocaInst, LoadInst>(CurDef))
break;
DIs.clear();
DPVs.clear();
findDbgDeclares(DIs, CurDef, &DPVs);
DIs = findDbgDeclares(CurDef);
DPVs = findDPVDeclares(CurDef);
}
}

Expand Down
21 changes: 6 additions & 15 deletions llvm/lib/Transforms/Scalar/SROA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5021,9 +5021,6 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {

// Remove any existing intrinsics on the new alloca describing
// the variable fragment.
SmallVector<DbgDeclareInst *, 1> FragDbgDeclares;
SmallVector<DPValue *, 1> FragDPVs;
findDbgDeclares(FragDbgDeclares, Fragment.Alloca, &FragDPVs);
auto RemoveOne = [DbgVariable](auto *OldDII) {
auto SameVariableFragment = [](const auto *LHS, const auto *RHS) {
return LHS->getVariable() == RHS->getVariable() &&
Expand All @@ -5033,20 +5030,17 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
if (SameVariableFragment(OldDII, DbgVariable))
OldDII->eraseFromParent();
};
for_each(FragDbgDeclares, RemoveOne);
for_each(FragDPVs, RemoveOne);
for_each(findDbgDeclares(Fragment.Alloca), RemoveOne);
for_each(findDPVDeclares(Fragment.Alloca), RemoveOne);

insertNewDbgInst(DIB, DbgVariable, Fragment.Alloca, FragmentExpr, &AI);
}
};

// Migrate debug information from the old alloca to the new alloca(s)
// and the individual partitions.
SmallVector<DbgDeclareInst *, 1> DbgDeclares;
SmallVector<DPValue *, 1> DPValues;
findDbgDeclares(DbgDeclares, &AI, &DPValues);
for_each(DbgDeclares, MigrateOne);
for_each(DPValues, MigrateOne);
for_each(findDbgDeclares(&AI), MigrateOne);
for_each(findDPVDeclares(&AI), MigrateOne);
for_each(at::getAssignmentMarkers(&AI), MigrateOne);

return Changed;
Expand Down Expand Up @@ -5169,12 +5163,9 @@ bool SROA::deleteDeadInstructions(
// not be able to find it.
if (AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
DeletedAllocas.insert(AI);
SmallVector<DbgDeclareInst *, 1> DbgDeclares;
SmallVector<DPValue *, 1> DPValues;
findDbgDeclares(DbgDeclares, AI, &DPValues);
for (DbgDeclareInst *OldDII : DbgDeclares)
for (DbgDeclareInst *OldDII : findDbgDeclares(AI))
OldDII->eraseFromParent();
for (DPValue *OldDII : DPValues)
for (DPValue *OldDII : findDPVDeclares(AI))
OldDII->eraseFromParent();
}

Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/Transforms/Utils/Local.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2130,9 +2130,8 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
DIBuilder &Builder, uint8_t DIExprFlags,
int Offset) {
SmallVector<DbgDeclareInst *, 1> DbgDeclares;
SmallVector<DPValue *, 1> DPValues;
findDbgDeclares(DbgDeclares, Address, &DPValues);
TinyPtrVector<DbgDeclareInst *> DbgDeclares = findDbgDeclares(Address);
TinyPtrVector<DPValue *> DPVDeclares = findDPVDeclares(Address);

auto ReplaceOne = [&](auto *DII) {
assert(DII->getVariable() && "Missing variable");
Expand All @@ -2143,9 +2142,9 @@ bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
};

for_each(DbgDeclares, ReplaceOne);
for_each(DPValues, ReplaceOne);
for_each(DPVDeclares, ReplaceOne);

return !DbgDeclares.empty() || !DPValues.empty();
return !DbgDeclares.empty() || !DPVDeclares.empty();
}

static void updateOneDbgValueForAlloca(const DebugLoc &Loc,
Expand Down
7 changes: 2 additions & 5 deletions llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,6 @@ void MemoryOpRemark::visitVariable(const Value *V,
bool FoundDI = false;
// Try to get an llvm.dbg.declare, which has a DILocalVariable giving us the
// real debug info name and size of the variable.
SmallVector<DbgDeclareInst *, 1> DbgDeclares;
SmallVector<DPValue *, 1> DPValues;
findDbgDeclares(DbgDeclares, const_cast<Value *>(V), &DPValues);
auto FindDI = [&](const auto *DVI) {
if (DILocalVariable *DILV = DVI->getVariable()) {
std::optional<uint64_t> DISize = getSizeInBytes(DILV->getSizeInBits());
Expand All @@ -334,8 +331,8 @@ void MemoryOpRemark::visitVariable(const Value *V,
}
}
};
for_each(DbgDeclares, FindDI);
for_each(DPValues, FindDI);
for_each(findDbgDeclares(const_cast<Value *>(V)), FindDI);
for_each(findDPVDeclares(const_cast<Value *>(V)), FindDI);

if (FoundDI) {
assert(!Result.empty());
Expand Down

0 comments on commit 3041198

Please sign in to comment.