[llvm] [DebugInfo][RemoveDIs] Support finding DPValues as well as dbg.values in findDbgValues (PR #71952)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 10 07:56:11 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-debuginfo
Author: Jeremy Morse (jmorse)
<details>
<summary>Changes</summary>
The first commit here extends findDbgValues and friends to optionally fill out a vector of DPValue pointers, containing DPValues that refer to the sought Value. This will allow us to incrementally add instrumentation to other optimisation passes one-at-a-time, while un-instrumented passes will not (yet) update DPValues.
However, we're not 100% there yet -- observe the DIArgList added in the unit test. This currently isn't discoverable by findDbgIntrinsincs (so the test fails today). I believe the reason why is that, as DIArgLists aren't Temporary by default, they don't have a use-list / UseMap maintained for them, therefore we can't lookup users backwards in the same way that we can for LocalAsMetadatas.
Hacking all DIArgLists to be temporary (see the second commit in this PR) fixes this and the test passes. CC @<!-- -->SLTozer , as you're polishing the Metadata tracking situation, is this something that'll be addressed by that work?
---
Full diff: https://github.com/llvm/llvm-project/pull/71952.diff
7 Files Affected:
- (modified) llvm/include/llvm/IR/DebugInfo.h (+5-2)
- (modified) llvm/lib/IR/DebugInfo.cpp (+31-7)
- (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+2)
- (modified) llvm/lib/IR/Value.cpp (+7-1)
- (modified) llvm/lib/IR/Verifier.cpp (+3)
- (modified) llvm/unittests/IR/DebugInfoTest.cpp (+48)
- (modified) llvm/unittests/IR/ValueTest.cpp (+76)
``````````diff
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 92beebed8ad51df..5d6e8c2fd28da61 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -34,6 +34,7 @@ namespace llvm {
class DbgDeclareInst;
class DbgValueInst;
class DbgVariableIntrinsic;
+class DPValue;
class Instruction;
class Module;
@@ -42,10 +43,12 @@ class Module;
TinyPtrVector<DbgDeclareInst *> FindDbgDeclareUses(Value *V);
/// Finds the llvm.dbg.value intrinsics describing a value.
-void findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V);
+void findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
+ Value *V, SmallVectorImpl<DPValue *> *DPValues = nullptr);
/// Finds the debug info intrinsics describing a value.
-void findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgInsts, Value *V);
+void findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgInsts,
+ Value *V, SmallVectorImpl<DPValue *> *DPValues = nullptr);
/// Find subprogram that is enclosing this scope.
DISubprogram *getDISubprogram(const MDNode *Scope);
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 390a27c4bc0c4dd..6d22b442944a792 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -25,6 +25,7 @@
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/DebugLoc.h"
+#include "llvm/IR/DebugProgramInstruction.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GVMaterializer.h"
#include "llvm/IR/Instruction.h"
@@ -65,7 +66,8 @@ TinyPtrVector<DbgDeclareInst *> llvm::FindDbgDeclareUses(Value *V) {
}
template <typename IntrinsicT>
-static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result, Value *V) {
+static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
+ Value *V, SmallVectorImpl<DPValue *> *DPValues) {
// This function is hot. Check whether the value has any metadata to avoid a
// DenseMap lookup.
if (!V->isUsedByMetadata())
@@ -78,31 +80,53 @@ static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result, Value *V) {
// V will also appear twice in a dbg.assign if its used in the both the value
// and address components.
SmallPtrSet<IntrinsicT *, 4> EncounteredIntrinsics;
+ SmallPtrSet<DPValue *, 4> EncounteredDPValues;
/// Append IntrinsicT users of MetadataAsValue(MD).
- auto AppendUsers = [&Ctx, &EncounteredIntrinsics, &Result](Metadata *MD) {
+ auto AppendUsers = [&Ctx, &EncounteredIntrinsics, &Result,
+ DPValues](Metadata *MD) {
if (auto *MDV = MetadataAsValue::getIfExists(Ctx, MD)) {
for (User *U : MDV->users())
if (IntrinsicT *DVI = dyn_cast<IntrinsicT>(U))
if (EncounteredIntrinsics.insert(DVI).second)
Result.push_back(DVI);
}
+ if (!DPValues)
+ return;
+ // Get DPValues that use this as a single value.
+ if (LocalAsMetadata *L = dyn_cast<LocalAsMetadata>(MD)) {
+ for (DPValue *DPV : L->getAllDPValueUsers()) {
+ if (DPV->getType() == DPValue::LocationType::Value)
+ DPValues->push_back(DPV);
+ }
+ }
};
if (auto *L = LocalAsMetadata::getIfExists(V)) {
AppendUsers(L);
- for (Metadata *AL : L->getAllArgListUsers())
+ for (Metadata *AL : L->getAllArgListUsers()) {
AppendUsers(AL);
+ if (!DPValues)
+ continue;
+ DIArgList *DI = cast<DIArgList>(AL);
+ if (auto *Replaceable = DI->getReplaceableUses()) {
+ for (DPValue *DPV : Replaceable->getAllDPValueUsers())
+ if (DPV->getType() == DPValue::LocationType::Value)
+ if (EncounteredDPValues.insert(DPV).second)
+ DPValues->push_back(DPV);
+ }
+ }
}
}
-void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V) {
- findDbgIntrinsics<DbgValueInst>(DbgValues, V);
+void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
+ Value *V, SmallVectorImpl<DPValue *> *DPValues) {
+ findDbgIntrinsics<DbgValueInst>(DbgValues, V, DPValues);
}
void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
- Value *V) {
- findDbgIntrinsics<DbgVariableIntrinsic>(DbgUsers, V);
+ Value *V, SmallVectorImpl<DPValue *> *DPValues) {
+ findDbgIntrinsics<DbgVariableIntrinsic>(DbgUsers, V, DPValues);
}
DISubprogram *llvm::getDISubprogram(const MDNode *Scope) {
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 48c4b62b467d10a..6393cedbf312088 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -2110,6 +2110,8 @@ DIMacroFile *DIMacroFile::getImpl(LLVMContext &Context, unsigned MIType,
DIArgList *DIArgList::getImpl(LLVMContext &Context,
ArrayRef<ValueAsMetadata *> Args,
StorageType Storage, bool ShouldCreate) {
+ if (Storage == Uniqued)
+ Storage = Temporary;
DEFINE_GETIMPL_LOOKUP(DIArgList, (Args));
DEFINE_GETIMPL_STORE_NO_OPS(DIArgList, (Args));
}
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index b485a6275b4ded1..c05f9ac5a0fe024 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -574,11 +574,17 @@ void Value::replaceUsesWithIf(Value *New,
/// with New.
static void replaceDbgUsesOutsideBlock(Value *V, Value *New, BasicBlock *BB) {
SmallVector<DbgVariableIntrinsic *> DbgUsers;
- findDbgUsers(DbgUsers, V);
+ SmallVector<DPValue *> DPUsers;
+ findDbgUsers(DbgUsers, V, &DPUsers);
for (auto *DVI : DbgUsers) {
if (DVI->getParent() != BB)
DVI->replaceVariableLocationOp(V, New);
}
+ for (auto *DPV : DPUsers) {
+ DPMarker *Marker = DPV->getMarker();
+ if (Marker->getParent() != BB)
+ DPV->replaceVariableLocationOp(V, New);
+ }
}
// Like replaceAllUsesWith except it does not handle constants or basic blocks.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index b1d1075285c2210..aa7bd2cfc3fe7bf 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -1016,6 +1016,9 @@ void Verifier::visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs) {
}
// Check these last, so we diagnose problems in operands first.
+ if (isa<DIArgList>(&MD))
+ // These remain tracked throughout compilation.
+ return;
Check(!MD.isTemporary(), "Expected no forward declarations!", &MD);
Check(MD.isResolved(), "All nodes should be resolved!", &MD);
}
diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index 034a9230bbadb59..592ec8d638d4f99 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -234,6 +234,54 @@ TEST(DbgVariableIntrinsic, EmptyMDIsKillLocation) {
EXPECT_TRUE(DbgDeclare->isKillLocation());
}
+// Duplicate of above test, but in DPValue representation.
+TEST(MetadataTest, DeleteInstUsedByDPValue) {
+ LLVMContext C;
+ std::unique_ptr<Module> M = parseIR(C, R"(
+ define i16 @f(i16 %a) !dbg !6 {
+ %b = add i16 %a, 1, !dbg !11
+ call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+ call void @llvm.dbg.value(metadata !DIArgList(i16 %a, i16 %b), metadata !9, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)), !dbg !11
+ ret i16 0, !dbg !11
+ }
+ declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+ attributes #0 = { nounwind readnone speculatable willreturn }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!5}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+ !1 = !DIFile(filename: "t.ll", directory: "/")
+ !2 = !{}
+ !5 = !{i32 2, !"Debug Info Version", i32 3}
+ !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+ !7 = !DISubroutineType(types: !2)
+ !8 = !{!9}
+ !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+ !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+ !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+ bool OldDbgValueMode = UseNewDbgInfoFormat;
+ UseNewDbgInfoFormat = true;
+ Instruction &I = *M->getFunction("f")->getEntryBlock().getFirstNonPHI();
+ M->convertToNewDbgValues();
+
+ // Find the DPValues using %b.
+ SmallVector<DbgValueInst *, 2> DVIs;
+ SmallVector<DPValue *, 2> DPVs;
+ findDbgValues(DVIs, &I, &DPVs);
+ ASSERT_EQ(DPVs.size(), 2u);
+
+ // Delete %b. The DPValue should now point to undef.
+ I.eraseFromParent();
+ EXPECT_EQ(DPVs[0]->getNumVariableLocationOps(), 1u);
+ EXPECT_TRUE(isa<UndefValue>(DPVs[0]->getVariableLocationOp(0)));
+ EXPECT_EQ(DPVs[1]->getNumVariableLocationOps(), 2u);
+ EXPECT_TRUE(isa<UndefValue>(DPVs[1]->getVariableLocationOp(1)));
+ UseNewDbgInfoFormat = OldDbgValueMode;
+}
+
TEST(DIBuiler, CreateFile) {
LLVMContext Ctx;
std::unique_ptr<Module> M(new Module("MyModule", Ctx));
diff --git a/llvm/unittests/IR/ValueTest.cpp b/llvm/unittests/IR/ValueTest.cpp
index 76a0b0c04f5f2b3..760b6b603c6a538 100644
--- a/llvm/unittests/IR/ValueTest.cpp
+++ b/llvm/unittests/IR/ValueTest.cpp
@@ -17,6 +17,8 @@
#include "gtest/gtest.h"
using namespace llvm;
+extern cl::opt<bool> UseNewDbgInfoFormat;
+
namespace {
TEST(ValueTest, UsedInBasicBlock) {
@@ -314,4 +316,78 @@ TEST(ValueTest, replaceUsesOutsideBlock) {
ASSERT_TRUE(ExitDbg->getValue(0) == cast<Value>(B));
ASSERT_TRUE(Ret->getOperand(0) == cast<Value>(B));
}
+
+TEST(ValueTest, replaceUsesOutsideBlockDPValue) {
+ // Check that Value::replaceUsesOutsideBlock(New, BB) replaces uses outside
+ // BB, including DPValues.
+ const auto *IR = R"(
+ define i32 @f() !dbg !6 {
+ entry:
+ %a = add i32 0, 1, !dbg !15
+ %b = add i32 0, 2, !dbg !15
+ %c = add i32 %a, 2, !dbg !15
+ call void @llvm.dbg.value(metadata i32 %a, metadata !9, metadata !DIExpression()), !dbg !15
+ br label %exit, !dbg !15
+
+ exit:
+ call void @llvm.dbg.value(metadata i32 %a, metadata !11, metadata !DIExpression()), !dbg !16
+ ret i32 %a, !dbg !16
+ }
+
+ declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!5}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+ !1 = !DIFile(filename: "test.ll", directory: "/")
+ !2 = !{}
+ !5 = !{i32 2, !"Debug Info Version", i32 3}
+ !6 = distinct !DISubprogram(name: "f", linkageName: "f", scope: null, file: !1, line: 1, type: !7, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !8)
+ !7 = !DISubroutineType(types: !2)
+ !8 = !{!9, !11}
+ !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+ !10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_signed)
+ !11 = !DILocalVariable(name: "2", scope: !6, file: !1, line: 2, type: !12)
+ !12 = !DIBasicType(name: "ty64", size: 64, encoding: DW_ATE_signed)
+ !15 = !DILocation(line: 1, column: 1, scope: !6)
+ !16 = !DILocation(line: 5, column: 1, scope: !6)
+ )";
+ LLVMContext Ctx;
+ SMDiagnostic Err;
+ std::unique_ptr<Module> M = parseAssemblyString(IR, Err, Ctx);
+ if (!M)
+ Err.print("ValueTest", errs());
+
+ bool OldDbgValueMode = UseNewDbgInfoFormat;
+ UseNewDbgInfoFormat = true;
+ M->convertToNewDbgValues();
+
+ auto GetNext = [](auto *I) { return &*++I->getIterator(); };
+
+ Function *F = M->getFunction("f");
+ // Entry.
+ BasicBlock *Entry = &F->front();
+ Instruction *A = &Entry->front();
+ Instruction *B = GetNext(A);
+ Instruction *C = GetNext(B);
+ Instruction *Branch = GetNext(C);
+ // Exit.
+ BasicBlock *Exit = GetNext(Entry);
+ Instruction *Ret = &Exit->front();
+
+ EXPECT_TRUE(Branch->hasDbgValues());
+ EXPECT_TRUE(Ret->hasDbgValues());
+
+ DPValue *DPV1 = &*Branch->getDbgValueRange().begin();
+ DPValue *DPV2 = &*Ret->getDbgValueRange().begin();
+
+ A->replaceUsesOutsideBlock(B, Entry);
+ // These users are in Entry so shouldn't be changed.
+ EXPECT_TRUE(DPV1->getVariableLocationOp(0) == cast<Value>(A));
+ // These users are outside Entry so should be changed.
+ EXPECT_TRUE(DPV2->getVariableLocationOp(0) == cast<Value>(B));
+ UseNewDbgInfoFormat = OldDbgValueMode;
+}
+
} // end anonymous namespace
``````````
</details>
https://github.com/llvm/llvm-project/pull/71952
More information about the llvm-commits
mailing list