[RemoveDIs] Handle DPValues in replaceDbgDeclare

[OCHyams]

Please review only the final commit in this stack.

[llvmbot]

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-coroutines

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

Please review only the final commit in this stack.

---

Full diff: https://github.com/llvm/llvm-project/pull/73507.diff

9 Files Affected:

• (modified) llvm/include/llvm/IR/DebugInfo.h (+2-1)
• (modified) llvm/lib/IR/DebugInfo.cpp (+16-27)
• (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+8-4)
• (modified) llvm/lib/Transforms/Scalar/SROA.cpp (+10-3)
• (modified) llvm/lib/Transforms/Utils/Local.cpp (+13-9)
• (modified) llvm/lib/Transforms/Utils/MemoryOpRemark.cpp (+3-2)
• (modified) llvm/test/DebugInfo/Generic/block-asan.ll (+1)
• (modified) llvm/test/DebugInfo/X86/asan_debug_info.ll (+4)
• (modified) llvm/test/Transforms/SafeStack/ARM/debug.ll (+2)

diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 2a581eb5f09d9f8..36ef77f9505bc10 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -40,7 +40,8 @@ class Module;
 
 /// Finds dbg.declare intrinsics declaring local variables as living in the
 /// memory that 'V' points to.
-TinyPtrVector<DbgDeclareInst *> FindDbgDeclareUses(Value *V);
+void findDbgDeclares(SmallVectorImpl<DbgDeclareInst *> &DbgUsers, Value *V,
+                     SmallVectorImpl<DPValue *> *DPValues = nullptr);
 
 /// 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 3fe940e19295b01..98271f1a2a88a72 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -44,30 +44,12 @@ using namespace llvm;
 using namespace llvm::at;
 using namespace llvm::dwarf;
 
-TinyPtrVector<DbgDeclareInst *> llvm::FindDbgDeclareUses(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);
-  }
+template <typename IntrinsicT, bool AnyType,
+          DPValue::LocationType Type = DPValue::LocationType(-1)>
+static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result, Value *V,
+                              SmallVectorImpl<DPValue *> *DPValues) {
+  static_assert(AnyType || (int)Type >= 0);
 
-  return Declares;
-}
-
-template <typename IntrinsicT>
-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())
@@ -96,7 +78,7 @@ static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
     // 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)
+        if (AnyType || DPV->getType() == Type)
           DPValues->push_back(DPV);
       }
     }
@@ -110,21 +92,28 @@ static void findDbgIntrinsics(SmallVectorImpl<IntrinsicT *> &Result,
         continue;
       DIArgList *DI = cast<DIArgList>(AL);
       for (DPValue *DPV : DI->getAllDPValueUsers())
-        if (DPV->getType() == DPValue::LocationType::Value)
+        if (AnyType || DPV->getType() == Type)
           if (EncounteredDPValues.insert(DPV).second)
             DPValues->push_back(DPV);
     }
   }
 }
 
+void llvm::findDbgDeclares(SmallVectorImpl<DbgDeclareInst *> &DbgUsers,
+                           Value *V, SmallVectorImpl<DPValue *> *DPValues) {
+  findDbgIntrinsics<DbgDeclareInst, false, DPValue::LocationType::Declare>(
+      DbgUsers, V, DPValues);
+}
+
 void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues,
                          Value *V, SmallVectorImpl<DPValue *> *DPValues) {
-  findDbgIntrinsics<DbgValueInst>(DbgValues, V, DPValues);
+  findDbgIntrinsics<DbgValueInst, false, DPValue::LocationType::Value>(
+      DbgValues, V, DPValues);
 }
 
 void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
                         Value *V, SmallVectorImpl<DPValue *> *DPValues) {
-  findDbgIntrinsics<DbgVariableIntrinsic>(DbgUsers, V, DPValues);
+  findDbgIntrinsics<DbgVariableIntrinsic, true>(DbgUsers, V, DPValues);
 }
 
 DISubprogram *llvm::getDISubprogram(const MDNode *Scope) {
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index fef1a698e146390..fbacfb39f62acbd 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -963,7 +963,8 @@ static void cacheDIVar(FrameDataInfo &FrameData,
     if (DIVarCache.contains(V))
       continue;
 
-    auto DDIs = FindDbgDeclareUses(V);
+    SmallVector<DbgDeclareInst *> DDIs;
+    findDbgDeclares(DDIs, V);
     auto *I = llvm::find_if(DDIs, [](DbgDeclareInst *DDI) {
       return DDI->getExpression()->getNumElements() == 0;
     });
@@ -1119,7 +1120,8 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape,
   assert(PromiseAlloca &&
          "Coroutine with switch ABI should own Promise alloca");
 
-  TinyPtrVector<DbgDeclareInst *> DIs = FindDbgDeclareUses(PromiseAlloca);
+  SmallVector<DbgDeclareInst *> DIs;
+  findDbgDeclares(DIs, PromiseAlloca);
   if (DIs.empty())
     return;
 
@@ -1840,7 +1842,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
               FrameTy->getElementType(FrameData.getFieldIndex(E.first)), GEP,
               SpillAlignment, E.first->getName() + Twine(".reload"));
 
-        TinyPtrVector<DbgDeclareInst *> DIs = FindDbgDeclareUses(Def);
+        SmallVector<DbgDeclareInst *> DIs;
+        findDbgDeclares(DIs, 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.
@@ -1854,7 +1857,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
             CurDef = LdInst->getPointerOperand();
             if (!isa<AllocaInst, LoadInst>(CurDef))
               break;
-            DIs = FindDbgDeclareUses(CurDef);
+            DIs.clear();
+            findDbgDeclares(DIs, CurDef);
           }
         }
 
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index f578762d2b4971c..92d5f970ea4ccab 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -4940,10 +4940,13 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
   // Migrate debug information from the old alloca to the new alloca(s)
   // and the individual partitions.
   TinyPtrVector<DbgVariableIntrinsic *> DbgVariables;
-  for (auto *DbgDeclare : FindDbgDeclareUses(&AI))
+  SmallVector<DbgDeclareInst *> DbgDeclares;
+  findDbgDeclares(DbgDeclares, &AI);
+  for (auto *DbgDeclare : DbgDeclares)
     DbgVariables.push_back(DbgDeclare);
   for (auto *DbgAssign : at::getAssignmentMarkers(&AI))
     DbgVariables.push_back(DbgAssign);
+
   for (DbgVariableIntrinsic *DbgVariable : DbgVariables) {
     auto *Expr = DbgVariable->getExpression();
     DIBuilder DIB(*AI.getModule(), /*AllowUnresolved*/ false);
@@ -4997,7 +5000,9 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
 
       // Remove any existing intrinsics on the new alloca describing
       // the variable fragment.
-      for (DbgDeclareInst *OldDII : FindDbgDeclareUses(Fragment.Alloca)) {
+      SmallVector<DbgDeclareInst *> FragDbgDeclares;
+      findDbgDeclares(FragDbgDeclares, Fragment.Alloca);
+      for (DbgDeclareInst *OldDII : FragDbgDeclares) {
         auto SameVariableFragment = [](const DbgVariableIntrinsic *LHS,
                                        const DbgVariableIntrinsic *RHS) {
           return LHS->getVariable() == RHS->getVariable() &&
@@ -5147,7 +5152,9 @@ bool SROA::deleteDeadInstructions(
     // not be able to find it.
     if (AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
       DeletedAllocas.insert(AI);
-      for (DbgDeclareInst *OldDII : FindDbgDeclareUses(AI))
+      SmallVector<DbgDeclareInst *> DbgDeclares;
+      findDbgDeclares(DbgDeclares, AI);
+      for (DbgDeclareInst *OldDII : DbgDeclares)
         OldDII->eraseFromParent();
     }
 
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index e399329a58873e7..a17b210e3c83de2 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2103,19 +2103,23 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
 bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
                              DIBuilder &Builder, uint8_t DIExprFlags,
                              int Offset) {
-  auto DbgDeclares = FindDbgDeclareUses(Address);
-  for (DbgVariableIntrinsic *DII : DbgDeclares) {
-    const DebugLoc &Loc = DII->getDebugLoc();
+  SmallVector<DbgDeclareInst *> DbgDeclares;
+  SmallVector<DPValue *> DPValues;
+  findDbgDeclares(DbgDeclares, Address, &DPValues);
+
+  auto ReplaceOne = [&](auto *DII) {
     auto *DIVar = DII->getVariable();
     auto *DIExpr = DII->getExpression();
     assert(DIVar && "Missing variable");
     DIExpr = DIExpression::prepend(DIExpr, DIExprFlags, Offset);
-    // Insert llvm.dbg.declare immediately before DII, and remove old
-    // llvm.dbg.declare.
-    Builder.insertDeclare(NewAddress, DIVar, DIExpr, Loc, DII);
-    DII->eraseFromParent();
-  }
-  return !DbgDeclares.empty();
+    DII->setExpression(DIExpr);
+    DII->replaceVariableLocationOp(Address, NewAddress);
+  };
+
+  for_each(DbgDeclares, ReplaceOne);
+  for_each(DPValues, ReplaceOne);
+
+  return !DbgDeclares.empty() || !DPValues.empty();
 }
 
 static void updateOneDbgValueForAlloca(const DebugLoc &Loc,
diff --git a/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp b/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
index 531b0a624dafab6..99c033cab64f803 100644
--- a/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
+++ b/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
@@ -321,8 +321,9 @@ 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.
-  for (const DbgVariableIntrinsic *DVI :
-       FindDbgDeclareUses(const_cast<Value *>(V))) {
+  SmallVector<DbgDeclareInst *> DbgDeclares;
+  findDbgDeclares(DbgDeclares, const_cast<Value *>(V));
+  for (const DbgVariableIntrinsic *DVI : DbgDeclares) {
     if (DILocalVariable *DILV = DVI->getVariable()) {
       std::optional<uint64_t> DISize = getSizeInBytes(DILV->getSizeInBits());
       VariableInfo Var{DILV->getName(), DISize};
diff --git a/llvm/test/DebugInfo/Generic/block-asan.ll b/llvm/test/DebugInfo/Generic/block-asan.ll
index db49289bd6eced4..ceaf3f1d5e9a3e7 100644
--- a/llvm/test/DebugInfo/Generic/block-asan.ll
+++ b/llvm/test/DebugInfo/Generic/block-asan.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -S -passes=asan %s | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -S -passes=asan %s | FileCheck %s
 
 ; The IR of this testcase is generated from the following C code:
 ; void bar (int);
diff --git a/llvm/test/DebugInfo/X86/asan_debug_info.ll b/llvm/test/DebugInfo/X86/asan_debug_info.ll
index 5e06b093c41d968..7a0dcdccffc8b71 100644
--- a/llvm/test/DebugInfo/X86/asan_debug_info.ll
+++ b/llvm/test/DebugInfo/X86/asan_debug_info.ll
@@ -2,6 +2,10 @@
 ; RUN:   llc -O0 -filetype=obj - -o - | \
 ; RUN:   llvm-dwarfdump - | FileCheck %s
 
+; RUN: opt --try-experimental-debginfo-iterators < %s -passes=asan -asan-use-after-return=never -S | \
+; RUN:   llc -O0 -filetype=obj - -o - | \
+; RUN:   llvm-dwarfdump - | FileCheck %s
+
 ; For this test case, ASan used to produce IR which resulted in the following
 ; DWARF (at -O0):
 ;
diff --git a/llvm/test/Transforms/SafeStack/ARM/debug.ll b/llvm/test/Transforms/SafeStack/ARM/debug.ll
index 9a61b78d2d9660e..4425278547d18d7 100644
--- a/llvm/test/Transforms/SafeStack/ARM/debug.ll
+++ b/llvm/test/Transforms/SafeStack/ARM/debug.ll
@@ -1,4 +1,6 @@
 ; RUN: opt -safe-stack -safestack-use-pointer-address < %s -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -safe-stack -safestack-use-pointer-address < %s -S | FileCheck %s
+
 target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
 target triple = "armv7-pc-linux-android"
 

[jmorse]

LGTM