[llvm] [DebugInfo][RemoveDIs] Support finding DPValues as well as dbg.values in findDbgValues (PR #71952)
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 10 07:55:51 PST 2023
https://github.com/jmorse created https://github.com/llvm/llvm-project/pull/71952
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?
>From 47b21d0b40ac78d59d20a7e3a63ef8b9d86ef7d1 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Wed, 7 Jun 2023 12:33:43 +0100
Subject: [PATCH 1/2] [DebugInfo][RemoveDIs] Support finding DPValues as well
as dbg.values
This patch extends findDbgValue 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.
Unit tests to check this behaves in the same way as dbg.values.
---
llvm/include/llvm/IR/DebugInfo.h | 7 ++-
llvm/lib/IR/DebugInfo.cpp | 38 ++++++++++++---
llvm/lib/IR/Value.cpp | 8 ++-
llvm/unittests/IR/DebugInfoTest.cpp | 48 ++++++++++++++++++
llvm/unittests/IR/ValueTest.cpp | 76 +++++++++++++++++++++++++++++
5 files changed, 167 insertions(+), 10 deletions(-)
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/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/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
>From e37fe1fd8d7c73960f40298a381c2d9e72d45468 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Fri, 10 Nov 2023 15:47:09 +0000
Subject: [PATCH 2/2] Temporary fixes to make DIArgLists... temporary
To illustrate upstream the situation with DIArgLists not being temporary,
or directly lookable-uppable, meaning we can't handle them in findDbgValues
yet.
---
llvm/lib/IR/DebugInfoMetadata.cpp | 2 ++
llvm/lib/IR/Verifier.cpp | 3 +++
2 files changed, 5 insertions(+)
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/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);
}
More information about the llvm-commits
mailing list