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

[RemoveDIs][DebugInfo] Enable creation of DPVAssigns, update outstanding AT tests #79148

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

SLTozer
Copy link
Contributor

@SLTozer SLTozer commented Jan 23, 2024

This is the final patch for DPVAssign support, implementing the actual creation of DPVAssigns and allowing them to be converted along with dbg.values and dbg.declares. Numerous tests landed in previous patches will no longer be rotten after this patch lands (previously they would trivially pass due to DPVAssigns not actually being used), and a further batch of tests have been added here that require the changes in this patch before they pass.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 23, 2024

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

Changes

This is the final patch for DPVAssign support, implementing the actual creation of DPVAssigns and allowing them to be converted along with dbg.values and dbg.declares. Numerous tests landed in previous patches will no longer be rotten after this patch lands (previously they would trivially pass due to DPVAssigns not actually being used), and a further batch of tests have been added here that require the changes in this patch before they pass.


Patch is 25.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/79148.diff

22 Files Affected:

  • (modified) llvm/lib/IR/BasicBlock.cpp (-3)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+63-49)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/long-double-x87.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/nullptr-declare.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/scalable-vector.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/set-flag-only-if-modified.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/structured-bindings.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/vla.ll (+1)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/instcombine/remove-redundant-dbg.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/loop-vectorize/remove-redundant-dbg.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/optnone.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/parse-and-verify/roundtrip.ll (+3)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/remove-redundant.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/slp-vectorizer/merge-scalars.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/sroa/remove-redundant-dbg.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/sroa/user-memcpy.ll (+2)
  • (modified) llvm/test/DebugInfo/Generic/assignment-tracking/track-assignments.ll (+2)
  • (modified) llvm/test/DebugInfo/assignment-tracking/X86/coalesce-options.ll (+10)
  • (modified) llvm/test/DebugInfo/assignment-tracking/X86/coalesce-simple.ll (+2)
  • (modified) llvm/test/DebugInfo/assignment-tracking/X86/remove-undef-fragment.ll (+2)
  • (modified) llvm/test/DebugInfo/assignment-tracking/X86/untagged-store-frag.ll (+2)
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 03b74b0480f0713..15b7e50fe6eca11 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -74,9 +74,6 @@ void BasicBlock::convertToNewDbgValues() {
   for (Instruction &I : make_early_inc_range(InstList)) {
     assert(!I.DbgMarker && "DbgMarker already set on old-format instrs?");
     if (DbgVariableIntrinsic *DVI = dyn_cast<DbgVariableIntrinsic>(&I)) {
-      if (isa<DbgAssignIntrinsic>(DVI))
-        continue;
-
       // Convert this dbg.value to a DPValue.
       DPValue *Value = new DPValue(DVI);
       DPVals.push_back(Value);
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 2e64d0db57b259a..c160b151b2de5cc 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1816,8 +1816,12 @@ void at::RAUW(DIAssignID *Old, DIAssignID *New) {
 
 void at::deleteAll(Function *F) {
   SmallVector<DbgAssignIntrinsic *, 12> ToDelete;
+  SmallVector<DPValue *, 12> DPToDelete;
   for (BasicBlock &BB : *F) {
     for (Instruction &I : BB) {
+      for (auto &DPV : I.getDbgValueRange())
+        if (DPV.isDbgAssign())
+          DPToDelete.push_back(&DPV);
       if (auto *DAI = dyn_cast<DbgAssignIntrinsic>(&I))
         ToDelete.push_back(DAI);
       else
@@ -1826,6 +1830,8 @@ void at::deleteAll(Function *F) {
   }
   for (auto *DAI : ToDelete)
     DAI->eraseFromParent();
+  for (auto *DPV : DPToDelete)
+    DPV->eraseFromParent();
 }
 
 /// Get the FragmentInfo for the variable if it exists, otherwise return a
@@ -2056,9 +2062,9 @@ std::optional<AssignmentInfo> at::getAssignmentInfo(const DataLayout &DL,
 }
 
 /// Returns nullptr if the assignment shouldn't be attributed to this variable.
-static CallInst *emitDbgAssign(AssignmentInfo Info, Value *Val, Value *Dest,
-                               Instruction &StoreLikeInst,
-                               const VarRecord &VarRec, DIBuilder &DIB) {
+static void emitDbgAssign(AssignmentInfo Info, Value *Val, Value *Dest,
+                          Instruction &StoreLikeInst, const VarRecord &VarRec,
+                          DIBuilder &DIB) {
   auto *ID = StoreLikeInst.getMetadata(LLVMContext::MD_DIAssignID);
   assert(ID && "Store instruction must have DIAssignID metadata");
   (void)ID;
@@ -2082,7 +2088,7 @@ static CallInst *emitDbgAssign(AssignmentInfo Info, Value *Val, Value *Dest,
 
     // Discard stores to bits outside this variable.
     if (FragStartBit >= FragEndBit)
-      return nullptr;
+      return;
 
     StoreToWholeVariable = FragStartBit <= VarStartBit && FragEndBit >= *Size;
   }
@@ -2097,8 +2103,17 @@ static CallInst *emitDbgAssign(AssignmentInfo Info, Value *Val, Value *Dest,
   }
   DIExpression *AddrExpr =
       DIExpression::get(StoreLikeInst.getContext(), std::nullopt);
-  return DIB.insertDbgAssign(&StoreLikeInst, Val, VarRec.Var, Expr, Dest,
-                             AddrExpr, VarRec.DL);
+  if (StoreLikeInst.getParent()->IsNewDbgInfoFormat) {
+    auto *Assign = DPValue::createLinkedDPVAssign(
+        &StoreLikeInst, Val, VarRec.Var, Expr, Dest, AddrExpr, VarRec.DL);
+    (void)Assign;
+    LLVM_DEBUG(if (Assign) errs() << " > INSERT: " << *Assign << "\n");
+    return;
+  }
+  auto *Assign = DIB.insertDbgAssign(&StoreLikeInst, Val, VarRec.Var, Expr,
+                                     Dest, AddrExpr, VarRec.DL);
+  (void)Assign;
+  LLVM_DEBUG(if (Assign) errs() << " > INSERT: " << *Assign << "\n");
 }
 
 #undef DEBUG_TYPE // Silence redefinition warning (from ConstantsContext.h).
@@ -2185,12 +2200,8 @@ void at::trackAssignments(Function::iterator Start, Function::iterator End,
         I.setMetadata(LLVMContext::MD_DIAssignID, ID);
       }
 
-      for (const VarRecord &R : LocalIt->second) {
-        auto *Assign =
-            emitDbgAssign(*Info, ValueComponent, DestComponent, I, R, DIB);
-        (void)Assign;
-        LLVM_DEBUG(if (Assign) errs() << " > INSERT: " << *Assign << "\n");
-      }
+      for (const VarRecord &R : LocalIt->second)
+        emitDbgAssign(*Info, ValueComponent, DestComponent, I, R, DIB);
     }
   }
 }
@@ -2206,32 +2217,40 @@ bool AssignmentTrackingPass::runOnFunction(Function &F) {
   // storage" is limited to Allocas). We'll use this to find dbg.declares to
   // delete after running `trackAssignments`.
   DenseMap<const AllocaInst *, SmallPtrSet<DbgDeclareInst *, 2>> DbgDeclares;
+  DenseMap<const AllocaInst *, SmallPtrSet<DPValue *, 2>> DPVDeclares;
   // Create another similar map of {storage : variables} that we'll pass to
   // trackAssignments.
   StorageToVarsMap Vars;
+  auto ProcessDeclare = [&](auto *Declare, auto &DeclareList) {
+    // FIXME: trackAssignments doesn't let you specify any modifiers to the
+    // variable (e.g. fragment) or location (e.g. offset), so we have to
+    // leave dbg.declares with non-empty expressions in place.
+    if (Declare->getExpression()->getNumElements() != 0)
+      return;
+    if (!Declare->getAddress())
+      return;
+    if (AllocaInst *Alloca =
+            dyn_cast<AllocaInst>(Declare->getAddress()->stripPointerCasts())) {
+      // FIXME: Skip VLAs for now (let these variables use dbg.declares).
+      if (!Alloca->isStaticAlloca())
+        return;
+      // Similarly, skip scalable vectors (use dbg.declares instead).
+      if (auto Sz = Alloca->getAllocationSize(*DL); Sz && Sz->isScalable())
+        return;
+      DeclareList[Alloca].insert(Declare);
+      Vars[Alloca].insert(VarRecord(Declare));
+    }
+  };
   for (auto &BB : F) {
     for (auto &I : BB) {
+      for (auto &DPV : I.getDbgValueRange()) {
+        if (DPV.isDbgDeclare())
+          ProcessDeclare(&DPV, DPVDeclares);
+      }
       DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(&I);
       if (!DDI)
         continue;
-      // FIXME: trackAssignments doesn't let you specify any modifiers to the
-      // variable (e.g. fragment) or location (e.g. offset), so we have to
-      // leave dbg.declares with non-empty expressions in place.
-      if (DDI->getExpression()->getNumElements() != 0)
-        continue;
-      if (!DDI->getAddress())
-        continue;
-      if (AllocaInst *Alloca =
-              dyn_cast<AllocaInst>(DDI->getAddress()->stripPointerCasts())) {
-        // FIXME: Skip VLAs for now (let these variables use dbg.declares).
-        if (!Alloca->isStaticAlloca())
-          continue;
-        // Similarly, skip scalable vectors (use dbg.declares instead).
-        if (auto Sz = Alloca->getAllocationSize(*DL); Sz && Sz->isScalable())
-          continue;
-        DbgDeclares[Alloca].insert(DDI);
-        Vars[Alloca].insert(VarRecord(DDI));
-      }
+      ProcessDeclare(DDI, DbgDeclares);
     }
   }
 
@@ -2247,35 +2266,30 @@ bool AssignmentTrackingPass::runOnFunction(Function &F) {
   trackAssignments(F.begin(), F.end(), Vars, *DL);
 
   // Delete dbg.declares for variables now tracked with assignment tracking.
-  for (auto &P : DbgDeclares) {
-    const AllocaInst *Alloca = P.first;
-    auto Markers = at::getAssignmentMarkers(Alloca);
-    SmallVector<DPValue *> DPMarkers = at::getDPVAssignmentMarkers(Alloca);
+  auto DeleteSubsumedDeclare = [&](const auto &Markers, auto &Declares) {
     (void)Markers;
-    (void)DPMarkers;
-    for (DbgDeclareInst *DDI : P.second) {
-      // Assert that the alloca that DDI uses is now linked to a dbg.assign
+    for (auto *Declare : Declares) {
+      // Assert that the alloca that Declare uses is now linked to a dbg.assign
       // describing the same variable (i.e. check that this dbg.declare has
       // been replaced by a dbg.assign). Use DebugVariableAggregate to Discard
       // the fragment part because trackAssignments may alter the
       // fragment. e.g. if the alloca is smaller than the variable, then
       // trackAssignments will create an alloca-sized fragment for the
       // dbg.assign.
-      assert(llvm::any_of(Markers,
-                          [DDI](DbgAssignIntrinsic *DAI) {
-                            return DebugVariableAggregate(DAI) ==
-                                   DebugVariableAggregate(DDI);
-                          }) ||
-             llvm::any_of(DPMarkers, [DDI](DPValue *DPV) {
-               return DebugVariableAggregate(DPV) ==
-                      DebugVariableAggregate(DDI);
-             }));
-      // Delete DDI because the variable location is now tracked using
+      assert(llvm::any_of(Markers, [Declare](auto *Assign) {
+        return DebugVariableAggregate(Assign) ==
+               DebugVariableAggregate(Declare);
+      }));
+      // Delete Declare because the variable location is now tracked using
       // assignment tracking.
-      DDI->eraseFromParent();
+      Declare->eraseFromParent();
       Changed = true;
     }
-  }
+  };
+  for (auto &P : DbgDeclares)
+    DeleteSubsumedDeclare(at::getAssignmentMarkers(P.first), P.second);
+  for (auto &P : DPVDeclares)
+    DeleteSubsumedDeclare(at::getDPVAssignmentMarkers(P.first), P.second);
   return Changed;
 }
 
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/long-double-x87.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/long-double-x87.ll
index 5e308097fd1aebb..3149dcb6ebc31c9 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/long-double-x87.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/long-double-x87.ll
@@ -1,4 +1,5 @@
 ; RUN: opt %s -S -passes=declare-to-assign -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -S -passes=declare-to-assign -o - | FileCheck %s
 
 ;; Generated from this C++:
 ;; long double get();
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/nullptr-declare.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/nullptr-declare.ll
index 82c710767c185d0..a795cc4c2dae3a3 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/nullptr-declare.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/nullptr-declare.ll
@@ -1,4 +1,5 @@
 ; RUN: opt %s -passes=declare-to-assign -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes=declare-to-assign -S | FileCheck %s
 
 ;; Check AssignmentTrackingPass ignores a dbg.declare with an empty metadata
 ;; location operand.
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/scalable-vector.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/scalable-vector.ll
index 4abe5f475aafe3a..2b9c9bf16c9a474 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/scalable-vector.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/scalable-vector.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=declare-to-assign %s -S | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes=declare-to-assign %s -S | FileCheck %s
 
 ;; Check declare-to-assign skips scalable vectors for now. i.e. do not replace
 ;; the dbg.declare with a dbg.assign intrinsic.
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/set-flag-only-if-modified.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/set-flag-only-if-modified.ll
index 3481bfe01991492..849c763da9fc0a1 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/set-flag-only-if-modified.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/set-flag-only-if-modified.ll
@@ -1,5 +1,7 @@
 ; RUN: opt -passes=declare-to-assign -S %s -o - \
 ; RUN: | FileCheck %s --check-prefix=WITHOUT-INTRINSIC
+; RUN: opt --try-experimental-debuginfo-iterators -passes=declare-to-assign -S %s -o - \
+; RUN: | FileCheck %s --check-prefix=WITHOUT-INTRINSIC
 
 ; RUN: sed 's/;Uncomment-with-sed//g' < %s \
 ; RUN: | opt -passes=declare-to-assign -S - -o - \
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/structured-bindings.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/structured-bindings.ll
index 776026fcbc01303..892e8501ebf3572 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/structured-bindings.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/structured-bindings.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=declare-to-assign -S %s -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes=declare-to-assign -S %s -o - | FileCheck %s
 
 ;; Check assignment tracking debug info for structured bindings. FIXME only
 ;; variables at offset 0 in the backing alloca are currently tracked with the
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll
index 56b631a59200d5b..c009fdcc238cb81 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/var-not-alloca-sized.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=declare-to-assign -S %s -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -passes=declare-to-assign -S %s -o - | FileCheck %s
 
 ;; The variable doesn't fill the whole alloca which has a range of different
 ;; sized stores to it, overlapping (or not) the variable in various ways. Check
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/vla.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/vla.ll
index 72d54cba7b4b649..b4e619e0e62ee45 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/vla.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/declare-to-assign/vla.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -S %s -passes=declare-to-assign -o - | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators -S %s -passes=declare-to-assign -o - | FileCheck %s
 
 ;; Check declare-to-assign ignores VLA-backed variables (for now).
 ;; From C++ source:
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/instcombine/remove-redundant-dbg.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/instcombine/remove-redundant-dbg.ll
index 11895098179ebff..cffac06f8e5451c 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/instcombine/remove-redundant-dbg.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/instcombine/remove-redundant-dbg.ll
@@ -1,5 +1,7 @@
 ; RUN: opt -passes=sroa -S %s -o - \
 ; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
+; RUN: opt --try-experimental-debuginfo-iterators -passes=sroa -S %s -o - \
+; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
 
 ;; Check that sroa removes redundant debug intrinsics after it makes a
 ;; change. This has a significant positive impact on peak memory and compiler
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/loop-vectorize/remove-redundant-dbg.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/loop-vectorize/remove-redundant-dbg.ll
index b02203dd99ba8d1..5c897187086d29d 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/loop-vectorize/remove-redundant-dbg.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/loop-vectorize/remove-redundant-dbg.ll
@@ -1,5 +1,7 @@
 ; RUN: opt %s -passes=loop-vectorize -force-vector-width=2 -force-vector-interleave=2 -S -o - \
 ; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes=loop-vectorize -force-vector-width=2 -force-vector-interleave=2 -S -o - \
+; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
 
 ;; Check that loop-vectorize removes redundant debug intrinsics after it makes
 ;; a change. This has a significant positive impact on peak memory and compiler
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/optnone.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/optnone.ll
index 6177448e2e6aa38..502d46640713aff 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/optnone.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/optnone.ll
@@ -1,5 +1,7 @@
 ; RUN: opt -S %s -o - --passes=declare-to-assign \
 ; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
+; RUN: opt --try-experimental-debuginfo-iterators -S %s -o - --passes=declare-to-assign \
+; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
 
 ;; Assignment tracking doesn't add any value when optimisations are disabled.
 ;; Check it doesn't get applied to functions marked optnone.
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/parse-and-verify/roundtrip.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/parse-and-verify/roundtrip.ll
index 0751e9ec0d493e3..c8fc014fcadf1fa 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/parse-and-verify/roundtrip.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/parse-and-verify/roundtrip.ll
@@ -1,6 +1,9 @@
 ; RUN: opt %s -passes=verify   \
 ; RUN: | opt -passes=verify -S \
 ; RUN: | FileCheck %s
+; RUN: opt --try-experimental-debuginfo-iterators %s -passes=verify   \
+; RUN: | opt -passes=verify -S \
+; RUN: | FileCheck %s
 
 ;; Roundtrip test (text -> bitcode -> text) for DIAssignID metadata and
 ;; llvm.dbg.assign intrinsics.
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/remove-redundant.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/remove-redundant.ll
index efb20b6edee2dcd..24ec3e94ed2753f 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/remove-redundant.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/remove-redundant.ll
@@ -1,5 +1,7 @@
 ; RUN: opt -passes=redundant-dbg-inst-elim -S %s -o - \
 ; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
+; RUN: opt --try-experimental-debuginfo-iterators -passes=redundant-dbg-inst-elim -S %s -o - \
+; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
 
 ;; Hand-written. Test how RemoveRedundantDbgInstrs interacts with dbg.assign
 ;; intrinsics. FileCehck directives are inline.
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/slp-vectorizer/merge-scalars.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/slp-vectorizer/merge-scalars.ll
index d675300395acf9c..6bf3284bcc35878 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/slp-vectorizer/merge-scalars.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/slp-vectorizer/merge-scalars.ll
@@ -60,6 +60,7 @@ entry:
   %arrayidx10 = getelementptr inbounds [4 x float], ptr %quad, i64 0, i64 3, !dbg !58
   store float %add9, ptr %arrayidx10, align 4, !dbg !59, !DIAssignID !60
   call void @llvm.dbg.assign(metadata float %add9, metadata !16, metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32), metadata !60, metadata ptr %arrayidx10, metadata !DIExpression()), !dbg !23
+  call void @llvm.dbg.value(metadata float %add9, metadata !16, metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32)), !dbg !23
   %call11 = call float @_Z3extPf(ptr nonnull %arrayidx), !dbg !61
   ret void, !dbg !62
 }
@@ -69,6 +70,7 @@ declare !dbg !63 dso_local float @_Z3getv() local_unnamed_addr #2
 declare !dbg !66 dso_local float @_Z3extPf(ptr) local_unnamed_addr #2
 declare void @llvm.lifetime.end.p0i8(i64 immarg, ptr nocapture) #1
 declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata) #3
+declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4, !5, !1000}
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/remove-redundant-dbg.ll b/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/remove-redundant-dbg.ll
index 11895098179ebff..cffac06f8e5451c 100644
--- a/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/remove-redundant-dbg.ll
+++ b/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/remove-redundant-dbg.ll
@@ -1,5 +1,7 @@
 ; RUN: opt -passes=sroa -S %s -o - \
 ; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
+; RUN: opt --try-experimental-debuginfo-iterators -passes=sroa -S %s -o - \
+; RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
 
 ;; Check that sroa removes redundant debug intrinsics after it makes a
 ;; change. This has a significant positive impact on peak memory and compiler
diff --git a/llvm/test/DebugInfo/Generic/assignment-tracking/sroa/user-memcpy.ll b/llvm/test/DebugInfo/Generic/assig...
[truncated]

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.

Couple of inline comments, otherwise LGTM

Comment on lines 2250 to 2253
DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(&I);
if (!DDI)
continue;
// FIXME: trackAssignments doesn't let you specify any modifiers to the
// variable (e.g. fragment) or location (e.g. offset), so we have to
// leave dbg.declares with non-empty expressions in place.
if (DDI->getExpression()->getNumElements() != 0)
continue;
if (!DDI->getAddress())
continue;
if (AllocaInst *Alloca =
dyn_cast<AllocaInst>(DDI->getAddress()->stripPointerCasts())) {
// FIXME: Skip VLAs for now (let these variables use dbg.declares).
if (!Alloca->isStaticAlloca())
continue;
// Similarly, skip scalable vectors (use dbg.declares instead).
if (auto Sz = Alloca->getAllocationSize(*DL); Sz && Sz->isScalable())
continue;
DbgDeclares[Alloca].insert(DDI);
Vars[Alloca].insert(VarRecord(DDI));
}
ProcessDeclare(DDI, DbgDeclares);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, I think this should be simplified to:

      if (DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(&I))
        ProcessDeclare(DDI, DbgDeclares);

@@ -60,6 +60,7 @@ entry:
%arrayidx10 = getelementptr inbounds [4 x float], ptr %quad, i64 0, i64 3, !dbg !58
store float %add9, ptr %arrayidx10, align 4, !dbg !59, !DIAssignID !60
call void @llvm.dbg.assign(metadata float %add9, metadata !16, metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32), metadata !60, metadata ptr %arrayidx10, metadata !DIExpression()), !dbg !23
call void @llvm.dbg.value(metadata float %add9, metadata !16, metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32)), !dbg !23
Copy link
Contributor

Choose a reason for hiding this comment

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

where does this change come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think this is an accidental WIP/testing change that somehow got in, good catch.

@SLTozer SLTozer merged commit d3a6a90 into llvm:main Jan 23, 2024
3 of 4 checks passed
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

3 participants