Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DebugInfo][RemoveDIs] Support finding DPValues as well as dbg.values in findDbgValues #71952

Closed
wants to merge 2 commits into from

Conversation

jmorse
Copy link
Member

@jmorse jmorse commented Nov 10, 2023

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?

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.
To illustrate upstream the situation with DIArgLists not being temporary,
or directly lookable-uppable, meaning we can't handle them in findDbgValues
yet.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 10, 2023

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

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 --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

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 51916f0c924f2ed4e970dd043a14d70b6b1d3f71 e37fe1fd8d7c73960f40298a381c2d9e72d45468 -- llvm/include/llvm/IR/DebugInfo.h llvm/lib/IR/DebugInfo.cpp llvm/lib/IR/DebugInfoMetadata.cpp llvm/lib/IR/Value.cpp llvm/lib/IR/Verifier.cpp llvm/unittests/IR/DebugInfoTest.cpp llvm/unittests/IR/ValueTest.cpp
View the diff from clang-format here.
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 5d6e8c2fd28d..96f8a7386ffb 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -43,12 +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, SmallVectorImpl<DPValue *> *DPValues = nullptr);
+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, SmallVectorImpl<DPValue *> *DPValues = nullptr);
+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 6d22b442944a..bcf5837681a3 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -66,8 +66,8 @@ TinyPtrVector<DbgDeclareInst *> llvm::FindDbgDeclareUses(Value *V) {
 }
 
 template <typename IntrinsicT>
-static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
-                              Value *V, SmallVectorImpl<DPValue *> *DPValues) {
+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())
@@ -119,8 +119,8 @@ static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
   }
 }
 
-void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
-                         Value *V, SmallVectorImpl<DPValue *> *DPValues) {
+void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V,
+                         SmallVectorImpl<DPValue *> *DPValues) {
   findDbgIntrinsics<DbgValueInst>(DbgValues, V, DPValues);
 }
 

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if @SLTozer is happy with the DIArgList situation

DIArgList *DI = cast<DIArgList>(AL);
if (auto *Replaceable = DI->getReplaceableUses()) {
for (DPValue *DPV : Replaceable->getAllDPValueUsers())
if (DPV->getType() == DPValue::LocationType::Value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a template parameter for intrinsic type but not for DPValue type. What do you think of changing the signature to:

template <typename IntrinsicT, bool AnyDPVType, DPValue::LocationType SpecificDPVType = DPValue::LocationType(-1)>
static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
                              Value *V, SmallVectorImpl<DPValue *> *DPValues)

Filter DPValeus by SpecificDPVType unless AnyDPVType == true.

wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hhhrrmmm. This is awkward because the intrinsics have an inherent hierarchy due to the instruction hierachy, i.e. DbgInfoIntrinsic -> DbgValueInst -> DbgAssignInst. We haven't yet created that for DPValues because we're focusing only on dbg.value replacement for the moment... but conceptually we'll end up replicating that hierarchy in the future. I'd prefer to wait until we implement that as it should line up with the instruction hierarchy perfectly (TM).

// 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)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth also checking DPV[x]->isKillLocation() for completeness?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@SLTozer
Copy link
Contributor

SLTozer commented Nov 16, 2023

I think this patch should wait until after #72147 lands - that patch makes DIArgList not inherit from MDNode at all, and ensures that there is always a UseMap, removing the need for (and totally conflicting with) the always-temporary hack here.

@jmorse
Copy link
Member Author

jmorse commented Nov 16, 2023

SGTM, the temporary hack was just to illustrate where the issue comes from

jmorse added a commit that referenced this pull request Nov 18, 2023
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.
@jmorse
Copy link
Member Author

jmorse commented Nov 18, 2023

Merged as 4259198

@jmorse jmorse closed this Nov 18, 2023
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
…#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.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…#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.
xgupta added a commit to xgupta/llvm-project that referenced this pull request Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants