[DebugInfo][RemoveDIs][NFC] Split findDbgDeclares into two functions #77478
+60
−48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Stephen Tozer (SLTozer) ChangesThis 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. Full diff: https://github.com/llvm/llvm-project/pull/77478.diff 6 Files Affected:
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 36ef77f9505bc1..e634f4bd2f5aae 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -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,
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index eab05eed428e47..78121a88f093d5 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -44,6 +44,44 @@ 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,
@@ -97,12 +135,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,
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index f37b4dc938d304..e8a23eaf2739ae 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -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));
}
}
@@ -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;
@@ -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.
@@ -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(Def);
+ DPVs = findDPVDeclares(Def);
}
}
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 656abdb0abbffa..a0291825abc1ce 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -5023,9 +5023,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() &&
@@ -5035,8 +5032,8 @@ 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);
}
@@ -5044,11 +5041,8 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
// 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;
@@ -5171,12 +5165,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();
}
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index c76cc9db16d7e7..807e66962171d8 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -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");
@@ -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,
diff --git a/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp b/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
index 47c6bcbaf26ec2..d671a9373bf026 100644
--- a/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
@@ -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());
@@ -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());
|
CarlosAlbertoEnciso
requested changes
Jan 10, 2024
| DPVs.clear(); | ||
| findDbgDeclares(DIs, CurDef, &DPVs); | ||
| DIs = findDbgDeclares(Def); | ||
| DPVs = findDPVDeclares(Def); |
There was a problem hiding this comment.
I think it should use CurDef instead of Def.
There was a problem hiding this comment.
Yeah, that's correct - typo slipped in here!
llvm/lib/IR/DebugInfo.cpp
Outdated
| return {}; | ||
|
|
||
| TinyPtrVector<DbgDeclareInst *> Declares; | ||
| for (User *U : MDV->users()) { |
There was a problem hiding this comment.
The braces in the for are not needed.
llvm/lib/IR/DebugInfo.cpp
Outdated
| return {}; | ||
|
|
||
| TinyPtrVector<DPValue *> Declares; | ||
| for (DPValue *DPV : L->getAllDPValueUsers()) { |
There was a problem hiding this comment.
The braces in the for are not needed.
jmorse
approved these changes
Jan 12, 2024
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
…lvm#77478) This patch follows on from comments on llvm#73498, implementing the proposed split of findDbgDeclares into two separate functions for DbgDeclareInsts and DPVDeclares, which return containers rather than taking containers by reference.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.