[llvm] [DebugInfo][RemoveDIs] Handle DPValues at remaining dbg.value using sites (PR #73788)

Jeremy Morse via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 30 04:50:37 PST 2023


https://github.com/jmorse updated https://github.com/llvm/llvm-project/pull/73788

>From 407cfda6323a66f006faadb02ae6e329c9ddafc8 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Wed, 7 Jun 2023 12:44:31 +0100
Subject: [PATCH 1/2] [DebugInfo][RemoveDIs] Handle DPValues at rmaining
 dbg.value using sites

This patch updates the last few places in LLVM using findDbgValues that
don't also collect and handle DPValue objects. This largely involves
instcombine and mem2reg changes, and are largely mechanical, calling
existing utilities on collections of DPValues instead of just
DbgValuesInsts.

A variety of tests have had RemoveDIs RUN lines added to them to cover
these behaviours. We have some technical debt of the instcombine sinking
code for DPValues not being implemented yet, so I've left FIXME stubs
indicating that we intend to cover tests with RemoveDIs but haven't yet.
---
 .../InstCombine/InstructionCombining.cpp      | 12 +++++-
 llvm/lib/Transforms/Scalar/JumpThreading.cpp  |  2 +-
 .../Utils/PromoteMemoryToRegister.cpp         | 41 ++++++++++++++++++-
 .../Generic/mem2reg-promote-alloca-1.ll       |  1 +
 .../Generic/mem2reg-promote-alloca-2.ll       |  1 +
 .../Generic/mem2reg-promote-alloca-3.ll       |  1 +
 .../test/DebugInfo/Generic/volatile-alloca.ll |  1 +
 llvm/test/DebugInfo/X86/mem2reg_fp80.ll       |  1 +
 .../cast-set-preserve-signed-dbg-val.ll       |  1 +
 .../InstCombine/consecutive-fences.ll         |  1 +
 .../dbg-scalable-store-fixed-frag.ll          |  1 +
 .../Transforms/InstCombine/debuginfo-dce2.ll  |  1 +
 .../Transforms/InstCombine/debuginfo-skip.ll  |  3 ++
 llvm/test/Transforms/InstCombine/debuginfo.ll |  4 ++
 .../Transforms/InstCombine/debuginfo_add.ll   |  3 ++
 .../erase-dbg-values-at-dead-alloc-site.ll    |  1 +
 .../InstCombine/lower-dbg-declare.ll          |  1 +
 llvm/test/Transforms/InstCombine/pr43893.ll   |  1 +
 ...ion-introduces-unnecessary-poison-value.ll |  3 ++
 .../InstCombine/stacksave-debuginfo.ll        |  1 +
 .../InstCombine/unavailable-debug.ll          |  1 +
 21 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index e6088541349784b..3a4b7d98d53592f 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -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
index 59a8b8ad494566c..ec9228bf4a100a3 100644
--- 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
index f56742b4e86eb52..79fcf13958e3ffc 100644
--- 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.
@@ -534,6 +539,17 @@ static bool rewriteSingleStoreAlloca(
     }
   }
 
+  // Duplicate implementation for non-instr storage of debug-info in
+  // DPValue objects.
+  for (DPValue *DPV : Info.DPUsers) {
+    if (DPV->isAddressOfVariable()) {
+      ConvertDebugDeclareToDebugValue(DPV, Info.OnlyStore, DIB);
+      DPV->eraseFromParent();
+    } else if (DPV->getExpression()->startsWithDeref()) {
+      DPV->eraseFromParent();
+    }
+  }
+
   // Remove dbg.assigns linked to the alloca as these are now redundant.
   at::deleteAssignmentMarkers(AI);
 
@@ -635,6 +651,11 @@ static bool promoteSingleBlockAlloca(
         ConvertDebugDeclareToDebugValue(DII, SI, DIB);
       }
     }
+    for (DPValue *DPV : Info.DPUsers) {
+      if (DPV->isAddressOfVariable()) {
+        ConvertDebugDeclareToDebugValue(DPV, SI, DIB);
+      }
+    }
     SI->eraseFromParent();
     LBI.deleteValue(SI);
   }
@@ -647,6 +668,10 @@ static bool promoteSingleBlockAlloca(
   for (DbgVariableIntrinsic *DII : Info.DbgUsers)
     if (DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref())
       DII->eraseFromParent();
+  for (DPValue *DPV : Info.DPUsers) {
+    if (DPV->isAddressOfVariable() || DPV->getExpression()->startsWithDeref())
+      DPV->eraseFromParent();
+  }
 
   ++NumLocalPromoted;
   return true;
@@ -657,6 +682,7 @@ void PromoteMem2Reg::run() {
 
   AllocaDbgUsers.resize(Allocas.size());
   AllocaATInfo.resize(Allocas.size());
+  AllocaDPUsers.resize(Allocas.size());
 
   AllocaInfo Info;
   LargeBlockInfo LBI;
@@ -720,6 +746,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;
@@ -800,6 +828,11 @@ void PromoteMem2Reg::run() {
       if (DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref())
         DII->eraseFromParent();
   }
+  for (auto &DPUsers : AllocaDPUsers) {
+    for (auto *DPV : DPUsers)
+      if (DPV->isAddressOfVariable() || DPV->getExpression()->startsWithDeref())
+        DPV->eraseFromParent();
+  }
 
   // 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
@@ -1044,6 +1077,9 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
         for (DbgVariableIntrinsic *DII : AllocaDbgUsers[AllocaNo])
           if (DII->isAddressOfVariable())
             ConvertDebugDeclareToDebugValue(DII, APN, DIB);
+        for (DPValue *DPV : AllocaDPUsers[AllocaNo])
+          if (DPV->isAddressOfVariable())
+            ConvertDebugDeclareToDebugValue(DPV, APN, DIB);
 
         // Get the next phi node.
         ++PNI;
@@ -1101,6 +1137,9 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
       for (DbgVariableIntrinsic *DII : AllocaDbgUsers[ai->second])
         if (DII->isAddressOfVariable())
           ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+      for (DPValue *DPV : AllocaDPUsers[ai->second])
+        if (DPV->isAddressOfVariable())
+          ConvertDebugDeclareToDebugValue(DPV, SI, DIB);
       SI->eraseFromParent();
     }
   }
diff --git a/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-1.ll b/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-1.ll
index 051ea71b674f979..d471e24b2458ed1 100644
--- 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
index 073649af535213c..871fc1c68601717 100644
--- 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
index 6552001b88f90b5..5e16376a153adcf 100644
--- 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
index bb1ce817272fdfc..891906e3a30f22d 100644
--- 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
index b227a37aa25f5d8..b00b1dea93f9256 100644
--- 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
index 775a0e164f95bfd..f9a476c24246d77 100644
--- 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
index ca5e576ef01189c..ce8274811416cec 100644
--- 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
index a8a7ee4608f65e4..f1f107c4bf772f1 100644
--- 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
index 8e59f278eee923c..f4f85f396c6e1bb 100644
--- 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
index dd1146e51595c53..ce6a675559acd26 100644
--- 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
index 81fd67d5474435d..0e25f2e74b7ed5d 100644
--- 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
index 47ba3b31f8d8cf4..15080ab0e548a30 100644
--- 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
+; RemoveDIs project: this can't yet be enabled because we haven't implemented
+; DPValue sinking for instcombine-sinsk.
+; 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
index 4bf171f4506fadc..1e79d8283b6ab35 100644
--- 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
index 2d4aa27a68efda9..5c37c8e5cb61762 100644
--- 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
index 0656b02689b6656..9f42c6e41ecf213 100644
--- 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
index afafb458797d91a..a5cd42c9d026b86 100644
--- 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
+; 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
index c804db256fa2031..fa9a35f09d1871e 100644
--- 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
index 165f55841533872..7dc9ed19ea86999 100644
--- 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

>From 64a5e5482acd5f2ac890447fcd11cc8127bd8cea Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 30 Nov 2023 12:49:29 +0000
Subject: [PATCH 2/2] templatize, add "FIXME" to tests

---
 .../Utils/PromoteMemoryToRegister.cpp         | 107 +++++++++---------
 .../Transforms/InstCombine/debuginfo_add.ll   |   4 +-
 ...ion-introduces-unnecessary-poison-value.ll |   4 +-
 3 files changed, 55 insertions(+), 60 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
index 79fcf13958e3ffc..3894764e1c2d9d5 100644
--- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
+++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -530,25 +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();
-    }
-  }
-
-  // Duplicate implementation for non-instr storage of debug-info in
-  // DPValue objects.
-  for (DPValue *DPV : Info.DPUsers) {
-    if (DPV->isAddressOfVariable()) {
-      ConvertDebugDeclareToDebugValue(DPV, Info.OnlyStore, DIB);
-      DPV->eraseFromParent();
-    } else if (DPV->getExpression()->startsWithDeref()) {
-      DPV->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);
@@ -645,17 +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);
-      }
-    }
-    for (DPValue *DPV : Info.DPUsers) {
-      if (DPV->isAddressOfVariable()) {
-        ConvertDebugDeclareToDebugValue(DPV, 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);
   }
@@ -665,13 +659,13 @@ 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();
-  for (DPValue *DPV : Info.DPUsers) {
-    if (DPV->isAddressOfVariable() || DPV->getExpression()->startsWithDeref())
-      DPV->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;
@@ -823,16 +817,15 @@ 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();
-  }
-  for (auto &DPUsers : AllocaDPUsers) {
-    for (auto *DPV : DPUsers)
-      if (DPV->isAddressOfVariable() || DPV->getExpression()->startsWithDeref())
-        DPV->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
@@ -1074,12 +1067,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);
-        for (DPValue *DPV : AllocaDPUsers[AllocaNo])
-          if (DPV->isAddressOfVariable())
-            ConvertDebugDeclareToDebugValue(DPV, 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;
@@ -1134,12 +1128,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);
-      for (DPValue *DPV : AllocaDPUsers[ai->second])
-        if (DPV->isAddressOfVariable())
-          ConvertDebugDeclareToDebugValue(DPV, 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/Transforms/InstCombine/debuginfo_add.ll b/llvm/test/Transforms/InstCombine/debuginfo_add.ll
index 15080ab0e548a30..28373601cd5e96c 100644
--- a/llvm/test/Transforms/InstCombine/debuginfo_add.ll
+++ b/llvm/test/Transforms/InstCombine/debuginfo_add.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -passes=instcombine %s -o - -S | FileCheck %s
-; RemoveDIs project: this can't yet be enabled because we haven't implemented
-; DPValue sinking for instcombine-sinsk.
+; 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 {
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
index a5cd42c9d026b86..06205693bd6ac50 100644
--- 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,6 +1,6 @@
 ; RUN: opt -passes=instcombine -S -o - < %s | FileCheck %s
-; RemoveDIs project: this can't yet be enabled because we haven't implemented
-; instcombine sinking.
+; 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,



More information about the llvm-commits mailing list