[llvm] [RemoveDIs][DebugInfo] Remove redundant DPVAssigns (PR #78574)

[Stephen Tozer via llvm-commits]

https://github.com/SLTozer created https://github.com/llvm/llvm-project/pull/78574

DPValues are already supported by most of the utilities that remove redundant debug info after certain passes; the exception to this is `removeUndefDbgAssignsFromEntryBlock`, which applies only to llvm.dbg.assigns which were previously unimplemented for DPValues. Now that DPVAssigns exist, we have to support removing redundant instances in the same way, which this patch implements.

>From d54da9ca45af6cd970acd000aa3b25abec3b7fdf Mon Sep 17 00:00:00 2001
From: Stephen Tozer <Stephen.Tozer at Sony.com>
Date: Wed, 3 Jan 2024 16:49:09 +0000
Subject: [PATCH] Update removal of redundant DPAssigns

---
 llvm/include/llvm/IR/DebugInfo.h              |  6 ++
 llvm/lib/Transforms/Utils/BasicBlockUtils.cpp | 71 +++++++++++++++++--
 2 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index e634f4bd2f5aae..e3182ab00f5ea1 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -192,6 +192,12 @@ inline AssignmentInstRange getAssignmentInsts(const DbgAssignIntrinsic *DAI) {
   return getAssignmentInsts(DAI->getAssignID());
 }
 
+inline AssignmentInstRange getAssignmentInsts(const DPValue *DPV) {
+  assert(DPV->isDbgAssign() &&
+         "Can't get assignment instructions for non-assign DPV!");
+  return getAssignmentInsts(DPV->getAssignID());
+}
+
 //
 // Utilities for enumerating llvm.dbg.assign intrinsic from an assignment ID.
 //
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index e019f76c8cacc3..ec0482ac2cdeb4 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -405,10 +405,17 @@ static bool DPValuesRemoveRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
       // If the same variable fragment is described more than once it is enough
       // to keep the last one (i.e. the first found since we for reverse
       // iteration).
-      // FIXME: add assignment tracking support (see parallel implementation
-      // below).
-      if (!R.second)
-        ToBeRemoved.push_back(&DPV);
+      if (R.second)
+        continue;
+
+      if (DPV.isDbgAssign()) {
+        // Don't delete dbg.assign intrinsics that are linked to instructions.
+        if (!at::getAssignmentInsts(&DPV).empty())
+          continue;
+        // Unlinked dbg.assign intrinsics can be treated like dbg.values.
+      }
+
+      ToBeRemoved.push_back(&DPV);
       continue;
     }
     // Sequence with consecutive dbg.value instrs ended. Clear the map to
@@ -495,14 +502,25 @@ static bool DPValuesRemoveRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
       DebugVariable Key(DPV.getVariable(), std::nullopt,
                         DPV.getDebugLoc()->getInlinedAt());
       auto VMI = VariableMap.find(Key);
+      // A dbg.assign with no linked instructions can be treated like a
+      // dbg.value (i.e. can be deleted).
+      bool IsDbgValueKind =
+          (!DPV.isDbgAssign() || at::getAssignmentInsts(&DPV).empty());
+
       // Update the map if we found a new value/expression describing the
       // variable, or if the variable wasn't mapped already.
       SmallVector<Value *, 4> Values(DPV.location_ops());
       if (VMI == VariableMap.end() || VMI->second.first != Values ||
           VMI->second.second != DPV.getExpression()) {
-        VariableMap[Key] = {Values, DPV.getExpression()};
+        if (IsDbgValueKind)
+          VariableMap[Key] = {Values, DPV.getExpression()};
+        else
+          VariableMap[Key] = {Values, nullptr};
         continue;
       }
+      // Don't delete dbg.assign intrinsics that are linked to instructions.
+      if (!IsDbgValueKind)
+        continue;
       // Found an identical mapping. Remember the instruction for later removal.
       ToBeRemoved.push_back(&DPV);
     }
@@ -514,6 +532,42 @@ static bool DPValuesRemoveRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
   return !ToBeRemoved.empty();
 }
 
+static bool DPValuesRemoveUndefDbgAssignsFromEntryBlock(BasicBlock *BB) {
+  assert(BB->isEntryBlock() && "expected entry block");
+  SmallVector<DPValue *, 8> ToBeRemoved;
+  DenseSet<DebugVariable> SeenDefForAggregate;
+  // Returns the DebugVariable for DVI with no fragment info.
+  auto GetAggregateVariable = [](const DPValue &DPV) {
+    return DebugVariable(DPV.getVariable(), std::nullopt,
+                         DPV.getDebugLoc().getInlinedAt());
+  };
+
+  // Remove undef dbg.assign intrinsics that are encountered before
+  // any non-undef intrinsics from the entry block.
+  for (auto &I : *BB) {
+    for (DPValue &DPV : I.getDbgValueRange()) {
+      if (!DPV.isDbgValue() && !DPV.isDbgAssign())
+        continue;
+      bool IsDbgValueKind =
+          (DPV.isDbgValue() || at::getAssignmentInsts(&DPV).empty());
+      DebugVariable Aggregate = GetAggregateVariable(DPV);
+      if (!SeenDefForAggregate.contains(Aggregate)) {
+        bool IsKill = DPV.isKillLocation() && IsDbgValueKind;
+        if (!IsKill) {
+          SeenDefForAggregate.insert(Aggregate);
+        } else if (DPV.isDbgAssign()) {
+          ToBeRemoved.push_back(&DPV);
+        }
+      }
+    }
+  }
+
+  for (DPValue *DPV : ToBeRemoved)
+    DPV->eraseFromParent();
+
+  return !ToBeRemoved.empty();
+}
+
 static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
   if (BB->IsNewDbgInfoFormat)
     return DPValuesRemoveRedundantDbgInstrsUsingForwardScan(BB);
@@ -578,7 +632,10 @@ static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
 /// then (only) the instruction marked with (*) can be removed.
 /// Possible improvements:
 /// - Keep track of non-overlapping fragments.
-static bool remomveUndefDbgAssignsFromEntryBlock(BasicBlock *BB) {
+static bool removeUndefDbgAssignsFromEntryBlock(BasicBlock *BB) {
+  if (BB->IsNewDbgInfoFormat)
+    return DPValuesRemoveUndefDbgAssignsFromEntryBlock(BB);
+
   assert(BB->isEntryBlock() && "expected entry block");
   SmallVector<DbgAssignIntrinsic *, 8> ToBeRemoved;
   DenseSet<DebugVariable> SeenDefForAggregate;
@@ -629,7 +686,7 @@ bool llvm::RemoveRedundantDbgInstrs(BasicBlock *BB) {
   MadeChanges |= removeRedundantDbgInstrsUsingBackwardScan(BB);
   if (BB->isEntryBlock() &&
       isAssignmentTrackingEnabled(*BB->getParent()->getParent()))
-    MadeChanges |= remomveUndefDbgAssignsFromEntryBlock(BB);
+    MadeChanges |= removeUndefDbgAssignsFromEntryBlock(BB);
   MadeChanges |= removeRedundantDbgInstrsUsingForwardScan(BB);
 
   if (MadeChanges)