llvm · jmorse · Nov 30, 2023 · Jun 7, 2023 · Nov 30, 2023 · Nov 30, 2023
@@ -2649,9 +2649,10 @@ Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) {
   // If we are removing an alloca with a dbg.declare, insert dbg.value calls
   // before each store.
   SmallVector<DbgVariableIntrinsic *, 8> DVIs;
+  SmallVector<DPValue *, 8> DPVs;
   std::unique_ptr<DIBuilder> DIB;
   if (isa<AllocaInst>(MI)) {
-    findDbgUsers(DVIs, &MI);
+    findDbgUsers(DVIs, &MI, &DPVs);
     DIB.reset(new DIBuilder(*MI.getModule(), /*AllowUnresolved=*/false));
   }
 
@@ -2691,6 +2692,9 @@ Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) {
         for (auto *DVI : DVIs)
           if (DVI->isAddressOfVariable())
             ConvertDebugDeclareToDebugValue(DVI, SI, *DIB);
+        for (auto *DPV : DPVs)
+          if (DPV->isAddressOfVariable())
+            ConvertDebugDeclareToDebugValue(DPV, SI, *DIB);
       } else {
         // Casts, GEP, or anything else: we're about to delete this instruction,
         // so it can not have any valid uses.
@@ -2729,9 +2733,15 @@ Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) {
     // If there is a dead store to `%a` in @trivially_inlinable_no_op, the
     // "arg0" dbg.value may be stale after the call. However, failing to remove
     // the DW_OP_deref dbg.value causes large gaps in location coverage.
+    //
+    // FIXME: the Assignment Tracking project has now likely made this
+    // redundant (and it's sometimes harmful).
     for (auto *DVI : DVIs)
       if (DVI->isAddressOfVariable() || DVI->getExpression()->startsWithDeref())
         DVI->eraseFromParent();
+    for (auto *DPV : DPVs)
+      if (DPV->isAddressOfVariable() || DPV->getExpression()->startsWithDeref())
+        DPV->eraseFromParent();
 
     return eraseInstFromFunction(MI);
   }

diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -1991,7 +1991,7 @@ void JumpThreadingPass::updateSSA(
     });
 
     // If there are no uses outside the block, we're done with this instruction.
-    if (UsesToRename.empty() && DbgValues.empty())
+    if (UsesToRename.empty() && DbgValues.empty() && DPValues.empty())
       continue;
     LLVM_DEBUG(dbgs() << "JT: Renaming non-local uses of: " << I << "\n");
 

diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -31,6 +31,7 @@
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DebugInfo.h"
+#include "llvm/IR/DebugProgramInstruction.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/InstrTypes.h"
@@ -172,6 +173,7 @@ class AssignmentTrackingInfo {
 
 struct AllocaInfo {
   using DbgUserVec = SmallVector<DbgVariableIntrinsic *, 1>;
+  using DPUserVec = SmallVector<DPValue *, 1>;
 
   SmallVector<BasicBlock *, 32> DefiningBlocks;
   SmallVector<BasicBlock *, 32> UsingBlocks;
@@ -182,6 +184,7 @@ struct AllocaInfo {
 
   /// Debug users of the alloca - does not include dbg.assign intrinsics.
   DbgUserVec DbgUsers;
+  DPUserVec DPUsers;
   /// Helper to update assignment tracking debug info.
   AssignmentTrackingInfo AssignmentTracking;
 
@@ -192,6 +195,7 @@ struct AllocaInfo {
     OnlyBlock = nullptr;
     OnlyUsedInOneBlock = true;
     DbgUsers.clear();
+    DPUsers.clear();
     AssignmentTracking.clear();
   }
 
@@ -225,7 +229,7 @@ struct AllocaInfo {
       }
     }
     DbgUserVec AllDbgUsers;
-    findDbgUsers(AllDbgUsers, AI);
+    findDbgUsers(AllDbgUsers, AI, &DPUsers);
     std::copy_if(AllDbgUsers.begin(), AllDbgUsers.end(),
                  std::back_inserter(DbgUsers), [](DbgVariableIntrinsic *DII) {
                    return !isa<DbgAssignIntrinsic>(DII);
@@ -329,6 +333,7 @@ struct PromoteMem2Reg {
   /// describes it, if any, so that we can convert it to a dbg.value
   /// intrinsic if the alloca gets promoted.
   SmallVector<AllocaInfo::DbgUserVec, 8> AllocaDbgUsers;
+  SmallVector<AllocaInfo::DPUserVec, 8> AllocaDPUsers;
 
   /// For each alloca, keep an instance of a helper class that gives us an easy
   /// way to update assignment tracking debug info if the alloca is promoted.
@@ -525,14 +530,18 @@ static bool rewriteSingleStoreAlloca(
 
   // Record debuginfo for the store and remove the declaration's
   // debuginfo.
-  for (DbgVariableIntrinsic *DII : Info.DbgUsers) {
-    if (DII->isAddressOfVariable()) {
-      ConvertDebugDeclareToDebugValue(DII, Info.OnlyStore, DIB);
-      DII->eraseFromParent();
-    } else if (DII->getExpression()->startsWithDeref()) {
-      DII->eraseFromParent();
+  auto ConvertDebugInfoForStore = [&](auto &Container) {
+    for (auto *DbgItem : Container) {
+      if (DbgItem->isAddressOfVariable()) {
+        ConvertDebugDeclareToDebugValue(DbgItem, Info.OnlyStore, DIB);
+        DbgItem->eraseFromParent();
+      } else if (DbgItem->getExpression()->startsWithDeref()) {
+        DbgItem->eraseFromParent();
+      }
     }
-  }
+  };
+  ConvertDebugInfoForStore(Info.DbgUsers);
+  ConvertDebugInfoForStore(Info.DPUsers);
 
   // Remove dbg.assigns linked to the alloca as these are now redundant.
   at::deleteAssignmentMarkers(AI);
@@ -629,12 +638,18 @@ static bool promoteSingleBlockAlloca(
     StoreInst *SI = cast<StoreInst>(AI->user_back());
     // Update assignment tracking info for the store we're going to delete.
     Info.AssignmentTracking.updateForDeletedStore(SI, DIB, DbgAssignsToDelete);
+
     // Record debuginfo for the store before removing it.
-    for (DbgVariableIntrinsic *DII : Info.DbgUsers) {
-      if (DII->isAddressOfVariable()) {
-        ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+    auto DbgUpdateForStore = [&](auto &Container) {
+      for (auto *DbgItem : Container) {
+        if (DbgItem->isAddressOfVariable()) {
+          ConvertDebugDeclareToDebugValue(DbgItem, SI, DIB);
+        }
       }
-    }
+    };
+    DbgUpdateForStore(Info.DbgUsers);
+    DbgUpdateForStore(Info.DPUsers);
+
     SI->eraseFromParent();
     LBI.deleteValue(SI);
   }
@@ -644,9 +659,14 @@ static bool promoteSingleBlockAlloca(
   AI->eraseFromParent();
 
   // The alloca's debuginfo can be removed as well.
-  for (DbgVariableIntrinsic *DII : Info.DbgUsers)
-    if (DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref())
-      DII->eraseFromParent();
+  auto DbgUpdateForAlloca = [&](auto &Container) {
+    for (auto *DbgItem : Container)
+      if (DbgItem->isAddressOfVariable() ||
+          DbgItem->getExpression()->startsWithDeref())
+        DbgItem->eraseFromParent();
+  };
+  DbgUpdateForAlloca(Info.DbgUsers);
+  DbgUpdateForAlloca(Info.DPUsers);
 
   ++NumLocalPromoted;
   return true;
@@ -657,6 +677,7 @@ void PromoteMem2Reg::run() {
 
   AllocaDbgUsers.resize(Allocas.size());
   AllocaATInfo.resize(Allocas.size());
+  AllocaDPUsers.resize(Allocas.size());
 
   AllocaInfo Info;
   LargeBlockInfo LBI;
@@ -720,6 +741,8 @@ void PromoteMem2Reg::run() {
       AllocaDbgUsers[AllocaNum] = Info.DbgUsers;
     if (!Info.AssignmentTracking.empty())
       AllocaATInfo[AllocaNum] = Info.AssignmentTracking;
+    if (!Info.DPUsers.empty())
+      AllocaDPUsers[AllocaNum] = Info.DPUsers;
 
     // Keep the reverse mapping of the 'Allocas' array for the rename pass.
     AllocaLookup[Allocas[AllocaNum]] = AllocaNum;
@@ -795,11 +818,16 @@ void PromoteMem2Reg::run() {
   }
 
   // Remove alloca's dbg.declare intrinsics from the function.
-  for (auto &DbgUsers : AllocaDbgUsers) {
-    for (auto *DII : DbgUsers)
-      if (DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref())
-        DII->eraseFromParent();
-  }
+  auto RemoveDbgDeclares = [&](auto &Container) {
+    for (auto &DbgUsers : Container) {
+      for (auto *DbgItem : DbgUsers)
+        if (DbgItem->isAddressOfVariable() ||
+            DbgItem->getExpression()->startsWithDeref())
+          DbgItem->eraseFromParent();
+    }
+  };
+  RemoveDbgDeclares(AllocaDbgUsers);
+  RemoveDbgDeclares(AllocaDPUsers);
 
   // Loop over all of the PHI nodes and see if there are any that we can get
   // rid of because they merge all of the same incoming values.  This can
@@ -1041,9 +1069,13 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
         // The currently active variable for this block is now the PHI.
         IncomingVals[AllocaNo] = APN;
         AllocaATInfo[AllocaNo].updateForNewPhi(APN, DIB);
-        for (DbgVariableIntrinsic *DII : AllocaDbgUsers[AllocaNo])
-          if (DII->isAddressOfVariable())
-            ConvertDebugDeclareToDebugValue(DII, APN, DIB);
+        auto ConvertDbgDeclares = [&](auto &Container) {
+          for (auto *DbgItem : Container)
+            if (DbgItem->isAddressOfVariable())
+              ConvertDebugDeclareToDebugValue(DbgItem, APN, DIB);
+        };
+        ConvertDbgDeclares(AllocaDbgUsers[AllocaNo]);
+        ConvertDbgDeclares(AllocaDPUsers[AllocaNo]);
 
         // Get the next phi node.
         ++PNI;
@@ -1098,9 +1130,13 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
       IncomingLocs[AllocaNo] = SI->getDebugLoc();
       AllocaATInfo[AllocaNo].updateForDeletedStore(SI, DIB,
                                                    &DbgAssignsToDelete);
-      for (DbgVariableIntrinsic *DII : AllocaDbgUsers[ai->second])
-        if (DII->isAddressOfVariable())
-          ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+      auto ConvertDbgDeclares = [&](auto &Container) {
+        for (auto *DbgItem : Container)
+          if (DbgItem->isAddressOfVariable())
+            ConvertDebugDeclareToDebugValue(DbgItem, SI, DIB);
+      };
+      ConvertDbgDeclares(AllocaDbgUsers[ai->second]);
+      ConvertDbgDeclares(AllocaDPUsers[ai->second]);
       SI->eraseFromParent();
     }
   }

diff --git a/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-1.ll b/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-1.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=mem2reg %s -S -o - | FileCheck %s
+; RUN: opt -passes=mem2reg %s -S -o - --try-experimental-debuginfo-iterators | FileCheck %s
 
 ;; Check that mem2reg removes dbg.value(%param.addr, DIExpression(DW_OP_deref...))
 ;; when promoting the alloca %param.addr.

diff --git a/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-2.ll b/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-2.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=mem2reg %s -S -o - | FileCheck %s
+; RUN: opt -passes=mem2reg %s -S -o - --try-experimental-debuginfo-iterators | FileCheck %s
 
 ;; Check that mem2reg removes dbg.value(%local, DIExpression(DW_OP_deref...))
 ;; that instcombine LowerDbgDeclare inserted before the call to 'esc' when

diff --git a/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-3.ll b/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-3.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=mem2reg %s -S -o - | FileCheck %s
+; RUN: opt -passes=mem2reg %s -S -o - --try-experimental-debuginfo-iterators | FileCheck %s
 
 ;; Check that mem2reg removes dbg.value(%local, DIExpression(DW_OP_deref...))
 ;; that instcombine LowerDbgDeclare inserted before the call to 'esc' when

diff --git a/llvm/test/DebugInfo/Generic/volatile-alloca.ll b/llvm/test/DebugInfo/Generic/volatile-alloca.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=mem2reg,instcombine %s -o - -S | FileCheck %s
+; RUN: opt -passes=mem2reg,instcombine %s -o - -S --try-experimental-debuginfo-iterators | FileCheck %s
 ;
 ; Test that a dbg.declare describing am alloca with volatile
 ; load/stores is not lowered into a dbg.value, since the alloca won't

diff --git a/llvm/test/DebugInfo/X86/mem2reg_fp80.ll b/llvm/test/DebugInfo/X86/mem2reg_fp80.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -passes=mem2reg -S | FileCheck %s
+; RUN: opt < %s -passes=mem2reg -S --try-experimental-debuginfo-iterators | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

diff --git a/llvm/test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll b/llvm/test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=instcombine -S < %s | FileCheck %s
+; RUN: opt -passes=instcombine -S < %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; CHECK-LABEL: define {{.*}} @test5
 define i16 @test5(i16 %A) !dbg !34 {

diff --git a/llvm/test/Transforms/InstCombine/consecutive-fences.ll b/llvm/test/Transforms/InstCombine/consecutive-fences.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=instcombine -S %s | FileCheck %s
+; RUN: opt -passes=instcombine -S %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; Make sure we collapse the fences in this case
 

diff --git a/llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll b/llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes='instcombine' -S | FileCheck %s
+; RUN: opt < %s -passes='instcombine' -S --try-experimental-debuginfo-iterators | FileCheck %s
 
 define i32 @foo(<vscale x 2 x i32> %x) {
 ; CHECK-LABEL: @foo(

diff --git a/llvm/test/Transforms/InstCombine/debuginfo-dce2.ll b/llvm/test/Transforms/InstCombine/debuginfo-dce2.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=instcombine -S %s -o - | FileCheck %s
+; RUN: opt -passes=instcombine -S %s -o - --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; In this example, the cast from ptr to ptr becomes trivially dead. We should
 ; salvage its debug info.

diff --git a/llvm/test/Transforms/InstCombine/debuginfo-skip.ll b/llvm/test/Transforms/InstCombine/debuginfo-skip.ll
@@ -1,6 +1,9 @@
 ; RUN: opt -instcombine-lower-dbg-declare=0 < %s -passes=instcombine -S | FileCheck %s
 ; RUN: opt -instcombine-lower-dbg-declare=1 < %s -passes=instcombine -S | FileCheck %s
 
+; RUN: opt -instcombine-lower-dbg-declare=0 < %s -passes=instcombine -S --try-experimental-debuginfo-iterators | FileCheck %s
+; RUN: opt -instcombine-lower-dbg-declare=1 < %s -passes=instcombine -S --try-experimental-debuginfo-iterators | FileCheck %s
+
 define i32 @foo(i32 %j) #0 !dbg !7 {
 entry:
   %j.addr = alloca i32, align 4

diff --git a/llvm/test/Transforms/InstCombine/debuginfo.ll b/llvm/test/Transforms/InstCombine/debuginfo.ll
@@ -2,6 +2,10 @@
 ; RUN:      | FileCheck %s --check-prefix=CHECK --check-prefix=NOLOWER
 ; RUN: opt < %s -passes=instcombine -instcombine-lower-dbg-declare=1 -S | FileCheck %s
 
+; RUN: opt < %s -passes=instcombine -instcombine-lower-dbg-declare=0 -S --try-experimental-debuginfo-iterators \
+; RUN:      | FileCheck %s --check-prefix=CHECK --check-prefix=NOLOWER
+; RUN: opt < %s -passes=instcombine -instcombine-lower-dbg-declare=1 -S --try-experimental-debuginfo-iterators | FileCheck %s
+
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64--linux"
 

diff --git a/llvm/test/Transforms/InstCombine/debuginfo_add.ll b/llvm/test/Transforms/InstCombine/debuginfo_add.ll
@@ -1,4 +1,7 @@
 ; RUN: opt -passes=instcombine %s -o - -S | FileCheck %s
+; FIXME RemoveDIs project: this can't yet be enabled because we haven't
+; implemented DPValue sinking for instcombine-sinks.
+; run: opt -passes=instcombine %s -o - -S --try-experimental-debuginfo-iterators | FileCheck %s
 ; typedef struct v *v_t;
 ; struct v {
 ;   unsigned long long p;

diff --git a/llvm/test/Transforms/InstCombine/erase-dbg-values-at-dead-alloc-site.ll b/llvm/test/Transforms/InstCombine/erase-dbg-values-at-dead-alloc-site.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -S -passes=instcombine %s | FileCheck %s -check-prefix=RUN-ONCE
+; RUN: opt -S -passes=instcombine %s --try-experimental-debuginfo-iterators | FileCheck %s -check-prefix=RUN-ONCE
 
 ; This example was reduced from a test case in which InstCombine ran at least
 ; twice:

diff --git a/llvm/test/Transforms/InstCombine/lower-dbg-declare.ll b/llvm/test/Transforms/InstCombine/lower-dbg-declare.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=instcombine < %s -S | FileCheck %s
+; RUN: opt -passes=instcombine < %s -S --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; This tests dbg.declare lowering for CallInst users of an alloca. The
 ; resulting dbg.value expressions should add a deref to the declare's expression.

diff --git a/llvm/test/Transforms/InstCombine/pr43893.ll b/llvm/test/Transforms/InstCombine/pr43893.ll
@@ -1,5 +1,6 @@
 ; Check for setting dbg.value as undef which depends on trivially dead instructions.
 ; RUN: opt -passes=instcombine -S -o - %s | FileCheck %s
+; RUN: opt -passes=instcombine -S -o - %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 @a = common dso_local global i8 0, align 1, !dbg !0
 @b = common dso_local global i8 0, align 1, !dbg !6

diff --git a/llvm/test/Transforms/InstCombine/sink-instruction-introduces-unnecessary-poison-value.ll b/llvm/test/Transforms/InstCombine/sink-instruction-introduces-unnecessary-poison-value.ll
@@ -1,4 +1,7 @@
 ; RUN: opt -passes=instcombine -S -o - < %s | FileCheck %s
+; FIXME RemoveDIs project: this can't yet be enabled because we haven't
+; implemented instcombine sinking.
+; run: opt -passes=instcombine -S -o - < %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; When the 'int Four = Two;' is sunk into the 'case 0:' block,
 ; the debug value for 'Three' is set incorrectly to 'poison'.

diff --git a/llvm/test/Transforms/InstCombine/stacksave-debuginfo.ll b/llvm/test/Transforms/InstCombine/stacksave-debuginfo.ll
@@ -2,6 +2,7 @@
 ; dbg.value intrinsics should not affect peephole combining of stacksave/stackrestore.
 ; PR37713
 ; RUN: opt -passes=instcombine %s -S | FileCheck %s
+; RUN: opt -passes=instcombine %s -S --try-experimental-debuginfo-iterators | FileCheck %s
 
 declare ptr @llvm.stacksave() #0
 declare void @llvm.stackrestore(ptr) #0

diff --git a/llvm/test/Transforms/InstCombine/unavailable-debug.ll b/llvm/test/Transforms/InstCombine/unavailable-debug.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; Make sure to update the debug value after dead code elimination.
 ; CHECK: %call = call signext i8 @b(i32 6), !dbg !39