[DebugInfo][RemoveDIs] Support finding DPValues like dbg.values (#71952)

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 · Nov 18, 2023 · 4259198 · 4259198
1 parent 89b0f1e
commit 4259198
Show file tree

Hide file tree

Showing 5 changed files with 167 additions and 10 deletions.
diff --git 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
@@ -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,51 @@ 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);
+      for (DPValue *DPV : DI->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
@@ -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
@@ -234,6 +234,56 @@ 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_TRUE(DPVs[0]->isKillLocation());
+  EXPECT_EQ(DPVs[1]->getNumVariableLocationOps(), 2u);
+  EXPECT_TRUE(isa<UndefValue>(DPVs[1]->getVariableLocationOp(1)));
+  EXPECT_TRUE(DPVs[1]->isKillLocation());
+  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
@@ -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