[llvm] e097582 - [DebugInfo][RemoveDIs] Instrument jump-threading to update DPValues (#73127)

via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 23 09:07:15 PST 2023


Author: Jeremy Morse
Date: 2023-11-23T17:07:10Z
New Revision: e09758224b65bfa30f1d9e5082f03abeeb610964

URL: https://github.com/llvm/llvm-project/commit/e09758224b65bfa30f1d9e5082f03abeeb610964
DIFF: https://github.com/llvm/llvm-project/commit/e09758224b65bfa30f1d9e5082f03abeeb610964.diff

LOG: [DebugInfo][RemoveDIs] Instrument jump-threading to update DPValues (#73127)

This patch makes jump-threading handle non-instruction debug-info stored
in DPValues in the same way that it updates dbg.values nowadays. This
involves re-targetting their operands as with dbg.values getting moved
from one block to another, and manually cloning them when duplicating
blocks. The SSAUpdater class also grows some functions for SSA-updating
DPValues in the same way as dbg.values.

All of this is largely covered by existing debug-info tests, except for
the cloning of DPValues attached to elidable instructions and branches,
where I've added a test to thread-debug-info.ll. Where previously we
could rely on dbg.values being copied and cloned as normal instructions
are, as we need to explicitly perform that operation now I've added some
explicit testing for it.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/SSAUpdater.h
    llvm/lib/Transforms/Scalar/JumpThreading.cpp
    llvm/lib/Transforms/Utils/SSAUpdater.cpp
    llvm/test/Transforms/JumpThreading/redundant-dbg-info.ll
    llvm/test/Transforms/JumpThreading/thread-debug-info.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
index 36fbf536f6d015c..c79d436fc676bb9 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h
@@ -23,6 +23,7 @@ class BasicBlock;
 class Instruction;
 class LoadInst;
 class PHINode;
+class DPValue;
 template <typename T> class SmallVectorImpl;
 template <typename T> class SSAUpdaterTraits;
 class Type;
@@ -123,6 +124,7 @@ class SSAUpdater {
   void UpdateDebugValues(Instruction *I);
   void UpdateDebugValues(Instruction *I,
                          SmallVectorImpl<DbgValueInst *> &DbgValues);
+  void UpdateDebugValues(Instruction *I, SmallVectorImpl<DPValue *> &DbgValues);
 
   /// Rewrite a use like \c RewriteUse but handling in-block definitions.
   ///
@@ -134,6 +136,7 @@ class SSAUpdater {
 private:
   Value *GetValueAtEndOfBlockInternal(BasicBlock *BB);
   void UpdateDebugValue(Instruction *I, DbgValueInst *DbgValue);
+  void UpdateDebugValue(Instruction *I, DPValue *DbgValue);
 };
 
 /// Helper class for promoting a collection of loads and stores into SSA

diff  --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index 2d899f100f8154d..d7d503427ec3d20 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -410,6 +410,10 @@ static bool replaceFoldableUses(Instruction *Cond, Value *ToVal,
   if (Cond->getParent() == KnownAtEndOfBB)
     Changed |= replaceNonLocalUsesWith(Cond, ToVal);
   for (Instruction &I : reverse(*KnownAtEndOfBB)) {
+    // Replace any debug-info record users of Cond with ToVal.
+    for (DPValue &DPV : I.getDbgValueRange())
+      DPV.replaceVariableLocationOp(Cond, ToVal, true);
+
     // Reached the Cond whose uses we are trying to replace, so there are no
     // more uses.
     if (&I == Cond)
@@ -1961,6 +1965,7 @@ void JumpThreadingPass::updateSSA(
   SSAUpdater SSAUpdate;
   SmallVector<Use *, 16> UsesToRename;
   SmallVector<DbgValueInst *, 4> DbgValues;
+  SmallVector<DPValue *, 4> DPValues;
 
   for (Instruction &I : *BB) {
     // Scan all uses of this instruction to see if it is used outside of its
@@ -1977,10 +1982,13 @@ void JumpThreadingPass::updateSSA(
     }
 
     // Find debug values outside of the block
-    findDbgValues(DbgValues, &I);
+    findDbgValues(DbgValues, &I, &DPValues);
     llvm::erase_if(DbgValues, [&](const DbgValueInst *DbgVal) {
       return DbgVal->getParent() == BB;
     });
+    llvm::erase_if(DPValues, [&](const DPValue *DPVal) {
+      return DPVal->getParent() == BB;
+    });
 
     // If there are no uses outside the block, we're done with this instruction.
     if (UsesToRename.empty() && DbgValues.empty())
@@ -1996,9 +2004,11 @@ void JumpThreadingPass::updateSSA(
 
     while (!UsesToRename.empty())
       SSAUpdate.RewriteUse(*UsesToRename.pop_back_val());
-    if (!DbgValues.empty()) {
+    if (!DbgValues.empty() || !DPValues.empty()) {
       SSAUpdate.UpdateDebugValues(&I, DbgValues);
+      SSAUpdate.UpdateDebugValues(&I, DPValues);
       DbgValues.clear();
+      DPValues.clear();
     }
 
     LLVM_DEBUG(dbgs() << "\n");
@@ -2041,6 +2051,26 @@ JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI,
     return true;
   };
 
+  // Duplicate implementation of the above dbg.value code, using DPValues
+  // instead.
+  auto RetargetDPValueIfPossible = [&](DPValue *DPV) {
+    SmallSet<std::pair<Value *, Value *>, 16> OperandsToRemap;
+    for (auto *Op : DPV->location_ops()) {
+      Instruction *OpInst = dyn_cast<Instruction>(Op);
+      if (!OpInst)
+        continue;
+
+      auto I = ValueMapping.find(OpInst);
+      if (I != ValueMapping.end())
+        OperandsToRemap.insert({OpInst, I->second});
+    }
+
+    for (auto &[OldOp, MappedOp] : OperandsToRemap)
+      DPV->replaceVariableLocationOp(OldOp, MappedOp);
+  };
+
+  BasicBlock *RangeBB = BI->getParent();
+
   // Clone the phi nodes of the source basic block into NewBB.  The resulting
   // phi nodes are trivial since NewBB only has one predecessor, but SSAUpdater
   // might need to rewrite the operand of the cloned phi.
@@ -2059,6 +2089,12 @@ JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI,
   identifyNoAliasScopesToClone(BI, BE, NoAliasScopes);
   cloneNoAliasScopes(NoAliasScopes, ClonedScopes, "thread", Context);
 
+  auto CloneAndRemapDbgInfo = [&](Instruction *NewInst, Instruction *From) {
+    auto DPVRange = NewInst->cloneDebugInfoFrom(From);
+    for (DPValue &DPV : DPVRange)
+      RetargetDPValueIfPossible(&DPV);
+  };
+
   // Clone the non-phi instructions of the source basic block into NewBB,
   // keeping track of the mapping and using it to remap operands in the cloned
   // instructions.
@@ -2069,6 +2105,8 @@ JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI,
     ValueMapping[&*BI] = New;
     adaptNoAliasScopes(New, ClonedScopes, Context);
 
+    CloneAndRemapDbgInfo(New, &*BI);
+
     if (RetargetDbgValueIfPossible(New))
       continue;
 
@@ -2081,6 +2119,17 @@ JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI,
       }
   }
 
+  // There may be DPValues on the terminator, clone directly from marker
+  // to marker as there isn't an instruction there.
+  if (BE != RangeBB->end() && BE->hasDbgValues()) {
+    // Dump them at the end.
+    DPMarker *Marker = RangeBB->getMarker(BE);
+    DPMarker *EndMarker = NewBB->createMarker(NewBB->end());
+    auto DPVRange = EndMarker->cloneDebugInfoFrom(Marker, std::nullopt);
+    for (DPValue &DPV : DPVRange)
+      RetargetDPValueIfPossible(&DPV);
+  }
+
   return ValueMapping;
 }
 
@@ -2666,6 +2715,9 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
       if (!New->mayHaveSideEffects()) {
         New->eraseFromParent();
         New = nullptr;
+        // Clone debug-info on the elided instruction to the destination
+        // position.
+        OldPredBranch->cloneDebugInfoFrom(&*BI, std::nullopt, true);
       }
     } else {
       ValueMapping[&*BI] = New;
@@ -2673,6 +2725,8 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
     if (New) {
       // Otherwise, insert the new instruction into the block.
       New->setName(BI->getName());
+      // Clone across any debug-info attached to the old instruction.
+      New->cloneDebugInfoFrom(&*BI);
       // Update Dominance from simplified New instruction operands.
       for (unsigned i = 0, e = New->getNumOperands(); i != e; ++i)
         if (BasicBlock *SuccBB = dyn_cast<BasicBlock>(New->getOperand(i)))

diff  --git a/llvm/lib/Transforms/Utils/SSAUpdater.cpp b/llvm/lib/Transforms/Utils/SSAUpdater.cpp
index 794c0258cca27a0..fc21fb552137d70 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdater.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdater.cpp
@@ -199,12 +199,18 @@ void SSAUpdater::RewriteUse(Use &U) {
 
 void SSAUpdater::UpdateDebugValues(Instruction *I) {
   SmallVector<DbgValueInst *, 4> DbgValues;
-  llvm::findDbgValues(DbgValues, I);
+  SmallVector<DPValue *, 4> DPValues;
+  llvm::findDbgValues(DbgValues, I, &DPValues);
   for (auto &DbgValue : DbgValues) {
     if (DbgValue->getParent() == I->getParent())
       continue;
     UpdateDebugValue(I, DbgValue);
   }
+  for (auto &DPV : DPValues) {
+    if (DPV->getParent() == I->getParent())
+      continue;
+    UpdateDebugValue(I, DPV);
+  }
 }
 
 void SSAUpdater::UpdateDebugValues(Instruction *I,
@@ -214,16 +220,31 @@ void SSAUpdater::UpdateDebugValues(Instruction *I,
   }
 }
 
+void SSAUpdater::UpdateDebugValues(Instruction *I,
+                                   SmallVectorImpl<DPValue *> &DPValues) {
+  for (auto &DPV : DPValues) {
+    UpdateDebugValue(I, DPV);
+  }
+}
+
 void SSAUpdater::UpdateDebugValue(Instruction *I, DbgValueInst *DbgValue) {
   BasicBlock *UserBB = DbgValue->getParent();
   if (HasValueForBlock(UserBB)) {
     Value *NewVal = GetValueAtEndOfBlock(UserBB);
     DbgValue->replaceVariableLocationOp(I, NewVal);
-  }
-  else
+  } else
     DbgValue->setKillLocation();
 }
 
+void SSAUpdater::UpdateDebugValue(Instruction *I, DPValue *DPV) {
+  BasicBlock *UserBB = DPV->getParent();
+  if (HasValueForBlock(UserBB)) {
+    Value *NewVal = GetValueAtEndOfBlock(UserBB);
+    DPV->replaceVariableLocationOp(I, NewVal);
+  } else
+    DPV->setKillLocation();
+}
+
 void SSAUpdater::RewriteUseAfterInsertions(Use &U) {
   Instruction *User = cast<Instruction>(U.getUser());
 

diff  --git a/llvm/test/Transforms/JumpThreading/redundant-dbg-info.ll b/llvm/test/Transforms/JumpThreading/redundant-dbg-info.ll
index 7659d72babf7a42..8aaed55788c5ce6 100644
--- a/llvm/test/Transforms/JumpThreading/redundant-dbg-info.ll
+++ b/llvm/test/Transforms/JumpThreading/redundant-dbg-info.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=jump-threading -S < %s | FileCheck %s
+; RUN: opt -passes=jump-threading -S < %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 define dso_local i32 @_Z3fooi(i32 %a) !dbg !7 {
 entry:

diff  --git a/llvm/test/Transforms/JumpThreading/thread-debug-info.ll b/llvm/test/Transforms/JumpThreading/thread-debug-info.ll
index 03478292779c3ad..cc9442b09740361 100644
--- a/llvm/test/Transforms/JumpThreading/thread-debug-info.ll
+++ b/llvm/test/Transforms/JumpThreading/thread-debug-info.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -S -passes=jump-threading < %s | FileCheck %s
+; RUN: opt -S -passes=jump-threading < %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 @a = global i32 0, align 4
 ; Test that the llvm.dbg.value calls in a threaded block are correctly updated to
@@ -91,6 +92,47 @@ exit:                                             ; preds = %bb.f4, %bb.f3, %bb.
   ret void, !dbg !29
 }
 
+; Test for the cloning of dbg.values on elided instructions -- down one path
+; being threaded, the `and` in the function below is optimised away, but its
+; debug-info should still be preserved.
+; Similarly, the call to f1 gets cloned, its dbg.value should be cloned too.
+define void @test16(i1 %c, i1 %c2, i1 %c3, i1 %c4) nounwind ssp !dbg !30 {
+; CHECK-LABEL: define void @test16(i1
+entry:
+  %cmp = icmp sgt i32 undef, 1, !dbg !33
+  br i1 %c, label %land.end, label %land.rhs, !dbg !33
+
+land.rhs:
+  br i1 %c2, label %lor.lhs.false.i, label %land.end, !dbg !33
+
+lor.lhs.false.i:
+  br i1 %c3, label %land.end, label %land.end, !dbg !33
+
+; CHECK-LABEL: land.end.thr_comm:
+; CHECK-NEXT:  call void @llvm.dbg.value(metadata i32 0,
+; CHECK-NEXT:  call void @llvm.dbg.value(metadata i32 1,
+; CHECK-NEXT:  call void @f1()
+; CHECK-NEXT:  br i1 %c4,
+
+; CHECK-LABEL: land.end:
+; CHECK-NEXT:  %0 = phi i1
+; CHECK-NEXT:  call void @llvm.dbg.value(metadata i32 0,
+land.end:
+  %0 = phi i1 [ true, %entry ], [ false, %land.rhs ], [false, %lor.lhs.false.i], [false, %lor.lhs.false.i]
+  call void @llvm.dbg.value(metadata i32 0, metadata !32, metadata !DIExpression()), !dbg !33
+  %cmp12 = and i1 %cmp, %0, !dbg !33
+  %xor1 = xor i1 %cmp12, %c4, !dbg !33
+  call void @llvm.dbg.value(metadata i32 1, metadata !32, metadata !DIExpression()), !dbg !33
+  call void @f1()
+  br i1 %xor1, label %if.then, label %if.end, !dbg !33
+
+if.then:
+  ret void, !dbg !33
+
+if.end:
+  ret void, !dbg !33
+}
+
 declare void @f1()
 
 declare void @f2()
@@ -138,3 +180,7 @@ attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memo
 !27 = !DILocation(line: 13, column: 1, scope: !5)
 !28 = !DILocation(line: 14, column: 1, scope: !5)
 !29 = !DILocation(line: 15, column: 1, scope: !5)
+!30 = distinct !DISubprogram(name: "test13", linkageName: "test13", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !31)
+!31 = !{!32}
+!32 = !DILocalVariable(name: "1", scope: !30, file: !1, line: 1, type: !10)
+!33 = !DILocation(line: 1, column: 1, scope: !30)


        


More information about the llvm-commits mailing list