[DebugInfo][RemoveDIs] Support cloning and remapping DPValues

[jmorse]

NB: this contains three commits sadly, the first is #72526 (necessary), the second is a RemoveDIs utility update (see commit message), the third is the proper topic of this review:

[DebugInfo][RemoveDIs] Support cloning and remapping DPValues

This patch adds support for CloneBasicBlock duplicating the DPValues
attached to instructions, and adds facilities to remap them into their new
context. The plumbing to achieve this is fairly straightforwards and
mechanical.

I've also added illustrative uses to LoopUnrollRuntime, SimpleLoopUnswitch
and SimplifyCFG. The former only updates for the epilogue right now so I've
added CHECK lines just for the end of an unrolled loop (further updates
coming later). SimpleLoopUnswitch had no debug-info tests so I've added a
new one. The two modified parts of SimplifyCFG are covered by the two
modified SimplifyCFG tests.

These are scenarios where we have to do extra cloning for copying of
DPValues because they're no longer instructions, and remap them too.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

NB: this contains three commits sadly, the first is #72526 (necessary), the second is a RemoveDIs utility update (see commit message), the third is the proper topic of this review:

[DebugInfo][RemoveDIs] Support cloning and remapping DPValues

This patch adds support for CloneBasicBlock duplicating the DPValues
attached to instructions, and adds facilities to remap them into their new
context. The plumbing to achieve this is fairly straightforwards and
mechanical.

I've also added illustrative uses to LoopUnrollRuntime, SimpleLoopUnswitch
and SimplifyCFG. The former only updates for the epilogue right now so I've
added CHECK lines just for the end of an unrolled loop (further updates
coming later). SimpleLoopUnswitch had no debug-info tests so I've added a
new one. The two modified parts of SimplifyCFG are covered by the two
modified SimplifyCFG tests.

These are scenarios where we have to do extra cloning for copying of
DPValues because they're no longer instructions, and remap them too.

---

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

11 Files Affected:

• (modified) llvm/include/llvm/Transforms/Utils/ValueMapper.h (+22)
• (modified) llvm/lib/IR/BasicBlock.cpp (+4-4)
• (modified) llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp (+2)
• (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+4-1)
• (modified) llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp (+5)
• (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+38-13)
• (modified) llvm/lib/Transforms/Utils/ValueMapper.cpp (+43)
• (modified) llvm/test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll (+13)
• (added) llvm/test/Transforms/SimpleLoopUnswitch/debuginfo.ll (+108)
• (modified) llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll (+21-13)
• (added) llvm/test/Transforms/SimplifyCFG/jump-threading-debuginfo.ll (+95)

diff --git a/llvm/include/llvm/Transforms/Utils/ValueMapper.h b/llvm/include/llvm/Transforms/Utils/ValueMapper.h
index e80951d50d56e87..82f0f99ee14ca9c 100644
--- a/llvm/include/llvm/Transforms/Utils/ValueMapper.h
+++ b/llvm/include/llvm/Transforms/Utils/ValueMapper.h
@@ -15,17 +15,21 @@
 #define LLVM_TRANSFORMS_UTILS_VALUEMAPPER_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/simple_ilist.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/IR/ValueMap.h"
 
 namespace llvm {
 
 class Constant;
+class DIBuilder;
+class DPValue;
 class Function;
 class GlobalVariable;
 class Instruction;
 class MDNode;
 class Metadata;
+class Module;
 class Type;
 class Value;
 
@@ -175,6 +179,8 @@ class ValueMapper {
   Constant *mapConstant(const Constant &C);
 
   void remapInstruction(Instruction &I);
+  void remapDPValue(Module *M, DPValue &V);
+  void remapDPValueRange(Module *M, iterator_range<simple_ilist<DPValue>::iterator> Range);
   void remapFunction(Function &F);
   void remapGlobalObjectMetadata(GlobalObject &GO);
 
@@ -260,6 +266,22 @@ inline void RemapInstruction(Instruction *I, ValueToValueMapTy &VM,
   ValueMapper(VM, Flags, TypeMapper, Materializer).remapInstruction(*I);
 }
 
+/// Remap the Values used in the DPValue \a V using the value map \a VM.
+inline void RemapDPValue(Module *M, DPValue *V, ValueToValueMapTy &VM,
+                  RemapFlags Flags = RF_None,
+                  ValueMapTypeRemapper *TypeMapper = nullptr,
+                  ValueMaterializer *Materializer = nullptr) {
+  ValueMapper(VM, Flags, TypeMapper, Materializer).remapDPValue(M, *V);
+}
+
+/// Remap the Values used in the DPValue \a V using the value map \a VM.
+inline void RemapDPValueRange(Module *M, iterator_range<simple_ilist<DPValue>::iterator> Range, ValueToValueMapTy &VM,
+                  RemapFlags Flags = RF_None,
+                  ValueMapTypeRemapper *TypeMapper = nullptr,
+                  ValueMaterializer *Materializer = nullptr) {
+  ValueMapper(VM, Flags, TypeMapper, Materializer).remapDPValueRange(M, Range);
+}
+
 /// Remap the operands, metadata, arguments, and instructions of a function.
 ///
 /// Calls \a MapValue() on prefix data, prologue data, and personality
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index a41bcff58e529af..ed00f3f247e3a26 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -39,8 +39,8 @@ cl::opt<bool>
 DPMarker *BasicBlock::createMarker(Instruction *I) {
   assert(IsNewDbgInfoFormat &&
          "Tried to create a marker in a non new debug-info block!");
-  assert(I->DbgMarker == nullptr &&
-         "Tried to create marker for instuction that already has one!");
+  if (I->DbgMarker)
+    return I->DbgMarker;
   DPMarker *Marker = new DPMarker();
   Marker->MarkedInstr = I;
   I->DbgMarker = Marker;
@@ -918,8 +918,8 @@ void BasicBlock::spliceDebugInfo(BasicBlock::iterator Dest, BasicBlock *Src,
   // move their markers onto Last. They remain in the Src block. No action
   // needed.
   if (!ReadFromHead) {
-    DPMarker *OntoLast = Src->getMarker(Last);
-    DPMarker *FromFirst = Src->getMarker(First);
+    DPMarker *OntoLast = Src->createMarker(Last);
+    DPMarker *FromFirst = Src->createMarker(First);
     OntoLast->absorbDebugValues(*FromFirst,
                                 true); // Always insert at head of it.
   }
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index e9e6ead9ccb7419..72465d0a1db097f 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -1260,8 +1260,10 @@ static BasicBlock *buildClonedLoopBlocks(
   // everything available. Also, we have inserted new instructions which may
   // include assume intrinsics, so we update the assumption cache while
   // processing this.
+  Module *M = ClonedPH->getParent()->getParent();
   for (auto *ClonedBB : NewBlocks)
     for (Instruction &I : *ClonedBB) {
+      RemapDPValueRange(M, I.getDbgValueRange(), VMap, RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
       RemapInstruction(&I, VMap,
                        RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
       if (auto *II = dyn_cast<AssumeInst>(&I))
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 94c6abcf7b4e1ac..9ff4f01a9809e5d 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -59,7 +59,10 @@ BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
     Instruction *NewInst = I.clone();
     if (I.hasName())
       NewInst->setName(I.getName() + NameSuffix);
-    NewInst->insertInto(NewBB, NewBB->end());
+
+    NewInst->insertBefore(*NewBB, NewBB->end());
+    NewInst->cloneDebugInfoFrom(&I);
+
     VMap[&I] = NewInst; // Add instruction map to value.
 
     if (isa<CallInst>(I) && !I.isDebugOrPseudoInst()) {
diff --git a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
index 1c8850048f6ab19..bed9dc6d22e5f51 100644
--- a/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
@@ -913,9 +913,14 @@ bool llvm::UnrollRuntimeLoopRemainder(
   // Rewrite the cloned instruction operands to use the values created when the
   // clone is created.
   for (BasicBlock *BB : NewBlocks) {
+    Module *M = BB->getModule();
     for (Instruction &I : *BB) {
       RemapInstruction(&I, VMap,
                        RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
+      for (DPValue &DPV : I.getDbgValueRange()) {
+        RemapDPValue(M, &DPV, VMap,
+                     RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
+      }
     }
   }
 
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 4dae52a8ecffdf6..fda5d6a7f92e3bd 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1101,12 +1101,13 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
   // Note that there may be multiple predecessor blocks, so we cannot move
   // bonus instructions to a predecessor block.
   for (Instruction &BonusInst : *BB) {
-    if (isa<DbgInfoIntrinsic>(BonusInst) || BonusInst.isTerminator())
+    if (BonusInst.isTerminator())
       continue;
 
     Instruction *NewBonusInst = BonusInst.clone();
 
-    if (PTI->getDebugLoc() != NewBonusInst->getDebugLoc()) {
+    if (!isa<DbgInfoIntrinsic>(BonusInst) &&
+        PTI->getDebugLoc() != NewBonusInst->getDebugLoc()) {
       // Unless the instruction has the same !dbg location as the original
       // branch, drop it. When we fold the bonus instructions we want to make
       // sure we reset their debug locations in order to avoid stepping on
@@ -1116,7 +1117,6 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
 
     RemapInstruction(NewBonusInst, VMap,
                      RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
-    VMap[&BonusInst] = NewBonusInst;
 
     // If we speculated an instruction, we need to drop any metadata that may
     // result in undefined behavior, as the metadata might have been valid
@@ -1126,8 +1126,17 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
     NewBonusInst->dropUBImplyingAttrsAndMetadata();
 
     NewBonusInst->insertInto(PredBlock, PTI->getIterator());
+    NewBonusInst->cloneDebugInfoFrom(&BonusInst);
+
+    for (DPValue &DPV : NewBonusInst->getDbgValueRange())
+      RemapDPValue(NewBonusInst->getModule(), &DPV, VMap, RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
+
+    if (isa<DbgInfoIntrinsic>(BonusInst))
+      continue;
+
     NewBonusInst->takeName(&BonusInst);
     BonusInst.setName(NewBonusInst->getName() + ".old");
+    VMap[&BonusInst] = NewBonusInst;
 
     // Update (liveout) uses of bonus instructions,
     // now that the bonus instruction has been cloned into predecessor.
@@ -3293,6 +3302,10 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
     BasicBlock::iterator InsertPt = EdgeBB->getFirstInsertionPt();
     DenseMap<Value *, Value *> TranslateMap; // Track translated values.
     TranslateMap[Cond] = CB;
+
+    // RemoveDIs: track instructions that we optimise away while folding, so
+    // that we can copy DPValues from them later.
+    BasicBlock::iterator DbgInfoCursor = BB->begin();
     for (BasicBlock::iterator BBI = BB->begin(); &*BBI != BI; ++BBI) {
       if (PHINode *PN = dyn_cast<PHINode>(BBI)) {
         TranslateMap[PN] = PN->getIncomingValueForBlock(EdgeBB);
@@ -3327,6 +3340,15 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
           TranslateMap[&*BBI] = N;
       }
       if (N) {
+        // Copy all debug-info attached to instructions from the last we
+        // successfully clone, up to this instruction (they might have been
+        // folded away).
+        for (; DbgInfoCursor != BBI; ++DbgInfoCursor)
+          N->cloneDebugInfoFrom(&*DbgInfoCursor);
+        DbgInfoCursor = std::next(BBI);
+        // Clone debug-info on this instruction too.
+        N->cloneDebugInfoFrom(&*BBI);
+
         // Register the new instruction with the assumption cache if necessary.
         if (auto *Assume = dyn_cast<AssumeInst>(N))
           if (AC)
@@ -3334,6 +3356,10 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
       }
     }
 
+    for (; &*DbgInfoCursor != BI; ++DbgInfoCursor)
+      InsertPt->cloneDebugInfoFrom(&*DbgInfoCursor);
+    InsertPt->cloneDebugInfoFrom(BI);
+
     BB->removePredecessor(EdgeBB);
     BranchInst *EdgeBI = cast<BranchInst>(EdgeBB->getTerminator());
     EdgeBI->setSuccessor(0, RealDest);
@@ -3738,22 +3764,21 @@ static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
   ValueToValueMapTy VMap; // maps original values to cloned values
   CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(BB, PredBlock, VMap);
 
+  Module *M = BB->getModule();
+
+  if (PredBlock->IsNewDbgInfoFormat) {
+    PredBlock->getTerminator()->cloneDebugInfoFrom(BB->getTerminator());
+    for (DPValue &DPV : PredBlock->getTerminator()->getDbgValueRange()) {
+      RemapDPValue(M, &DPV, VMap, RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
+    }
+  }
+
   // Now that the Cond was cloned into the predecessor basic block,
   // or/and the two conditions together.
   Value *BICond = VMap[BI->getCondition()];
   PBI->setCondition(
       createLogicalOp(Builder, Opc, PBI->getCondition(), BICond, "or.cond"));
 
-  // Copy any debug value intrinsics into the end of PredBlock.
-  for (Instruction &I : *BB) {
-    if (isa<DbgInfoIntrinsic>(I)) {
-      Instruction *NewI = I.clone();
-      RemapInstruction(NewI, VMap,
-                       RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
-      NewI->insertBefore(PBI);
-    }
-  }
-
   ++NumFoldBranchToCommonDest;
   return true;
 }
diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp
index 3446e31cc2ef176..70dfffc5d1dce00 100644
--- a/llvm/lib/Transforms/Utils/ValueMapper.cpp
+++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp
@@ -31,6 +31,7 @@
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
@@ -145,6 +146,7 @@ class Mapper {
   Value *mapValue(const Value *V);
   void remapInstruction(Instruction *I);
   void remapFunction(Function &F);
+  void remapDPValue(DPValue &DPV);
 
   Constant *mapConstant(const Constant *C) {
     return cast_or_null<Constant>(mapValue(C));
@@ -535,6 +537,37 @@ Value *Mapper::mapValue(const Value *V) {
   return getVM()[V] = ConstantPointerNull::get(cast<PointerType>(NewTy));
 }
 
+void Mapper::remapDPValue(DPValue &V) {
+  // Remap variables and DILocations.
+  auto *MappedVar = mapMetadata(V.getVariable());
+  auto *MappedDILoc = mapMetadata(V.getDebugLoc());
+  V.setVariable(cast<DILocalVariable>(MappedVar));
+  V.setDebugLoc(DebugLoc(cast<DILocation>(MappedDILoc)));
+
+  // Find Value operands and remap those.
+  SmallVector<Value *, 4> Vals, NewVals;
+  for (Value *Val: V.location_ops())
+    Vals.push_back(Val);
+  for (Value *Val : Vals)
+    NewVals.push_back(mapValue(Val));
+
+  // If there are no changes to the Value operands, finished.
+  if (Vals == NewVals)
+    return;
+
+  bool IgnoreMissingLocals = Flags & RF_IgnoreMissingLocals;
+
+  // Otherwise, do some replacement.
+  if (!IgnoreMissingLocals &&
+      llvm::any_of(NewVals, [&](Value *V) { return V == nullptr;})) {
+    V.setKillLocation();
+  } else {
+    for (unsigned int I = 0; I < Vals.size(); ++I)
+      if (NewVals[I] || !IgnoreMissingLocals)
+        V.replaceVariableLocationOp(I, NewVals[I]);
+  }
+}
+
 Value *Mapper::mapBlockAddress(const BlockAddress &BA) {
   Function *F = cast<Function>(mapValue(BA.getFunction()));
 
@@ -1179,6 +1212,16 @@ void ValueMapper::remapInstruction(Instruction &I) {
   FlushingMapper(pImpl)->remapInstruction(&I);
 }
 
+void ValueMapper::remapDPValue(Module *M, DPValue &V) {
+  FlushingMapper(pImpl)->remapDPValue(V);
+}
+
+void ValueMapper::remapDPValueRange(Module *M, iterator_range<DPValue::self_iterator> Range) {
+  for (DPValue &DPV : Range) {
+    remapDPValue(M, DPV);
+  }
+}
+
 void ValueMapper::remapFunction(Function &F) {
   FlushingMapper(pImpl)->remapFunction(F);
 }
diff --git a/llvm/test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll b/llvm/test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll
index bd5d68c5b9af37a..362237466118512 100644
--- a/llvm/test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll
+++ b/llvm/test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll
@@ -1,4 +1,5 @@
 ; RUN: opt -passes=loop-unroll -unroll-runtime -unroll-runtime-epilog -S %s | FileCheck %s
+; RUN: opt -passes=loop-unroll -unroll-runtime -unroll-runtime-epilog -S %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; Test that epilogue is tagged with the same debug information as original loop body rather than original loop exit.
 
@@ -11,6 +12,18 @@
 ; CHECK:   br i1 %lcmp.mod, label %for.body.i.epil.preheader, label %lee1.exit.loopexit, !dbg ![[LOOP_LOC]]
 ; CHECK: for.body.i.epil.preheader:
 ; CHECK:   br label %for.body.i.epil, !dbg ![[LOOP_LOC]]
+; CHECK: for.body.i.epil:
+;; Ensure that when we clone the div/add/add and it's following dbg.values,
+;; those dbg.values are remapped to the duplicated adds, not the originals.
+; CHECK:      %div.i.epil = sdiv i32 %t.08.i.epil, 2,
+; CHECK-NEXT: %add.i.epil = add i32 %t.08.i.epil, %a,
+; CHECK-NEXT: %add1.i.epil = add i32 %add.i.epil, %div.i.epil,
+; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %add1.i.epil,
+; CHECK-NEXT: %inc.i.epil = add nuw i32 %i.09.i.epil, 1, !dbg !36
+; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %inc.i.epil,
+; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %inc.i.epil,
+; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %add1.i.epil,
+
 ; CHECK: lee1.exit.loopexit:
 ; CHECK:   br label %lee1.exit, !dbg ![[EXIT_LOC:[0-9]+]]
 
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/debuginfo.ll b/llvm/test/Transforms/SimpleLoopUnswitch/debuginfo.ll
new file mode 100644
index 000000000000000..e821fd48a92fc90
--- /dev/null
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/debuginfo.ll
@@ -0,0 +1,108 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),verify<loops>' -S < %s | FileCheck %s
+; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),verify<loops>' -S < %s --try-experimental-debuginfo-iterators | FileCheck %s
+;
+;; Check that when we duplicate the load in the loop header, we also duplicate
+;; the corresponding dbg.value.
+;; FIXME: the hoisted load dominates the duplicated dbg.value, however as it's
+;; not subsequently used in the loop, so it doesn't get remapped into the
+;; debug user and we get a undef/poison dbg.value. This is suboptimal, but it's
+;; important that it gets duplicated nonetheless.
+
+declare void @clobber()
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+define i32 @partial_unswitch_true_successor(ptr %ptr, i32 %N) {
+; CHECK-LABEL: @partial_unswitch_true_successor(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[PTR:%.*]], align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i32 [[TMP0]], 100
+; CHECK-NEXT:    br i1 [[TMP1]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
+; CHECK:       entry.split.us:
+; CHECK-NEXT:    br label [[LOOP_HEADER_US:%.*]]
+; CHECK:       loop.header.us:
+; CHECK-NEXT:    [[IV_US:%.*]] = phi i32 [ 0, [[ENTRY_SPLIT_US]] ], [ [[IV_NEXT_US:%.*]], [[LOOP_LATCH_US:%.*]] ]
+; CHECK-NEXT:    call void @llvm.dbg.value(metadata i32 poison, metadata [[META3:![0-9]+]], metadata !DIExpression()), !dbg [[DBG8:![0-9]+]]
+; CHECK-NEXT:    br label [[NOCLOBBER_US:%.*]]
+; CHECK:       noclobber.us:
+; CHECK-NEXT:    br label [[LOOP_LATCH_US]]
+; CHECK:       loop.latch.us:
+; CHECK-NEXT:    [[C_US:%.*]] = icmp ult i32 [[IV_US]], [[N:%.*]]
+; CHECK-NEXT:    [[IV_NEXT_US]] = add i32 [[IV_US]], 1
+; CHECK-NEXT:    br i1 [[C_US]], label [[LOOP_HEADER_US]], label [[EXIT_SPLIT_US:%.*]]
+; CHECK:       exit.split.us:
+; CHECK-NEXT:    br label [[EXIT:%.*]]
+; CHECK:       entry.split:
+; CHECK-NEXT:    br label [[LOOP_HEADER:%.*]]
+; CHECK:       loop.header:
+; CHECK-NEXT:    [[IV:%.*]] = phi i32 [ 0, [[ENTRY_SPLIT]] ], [ [[IV_NEXT:%.*]], [[LOOP_LATCH:%.*]] ]
+; CHECK-NEXT:    [[LV:%.*]] = load i32, ptr [[PTR]], align 4
+; CHECK-NEXT:    call void @llvm.dbg.value(metadata i32 [[LV]], metadata [[META3]], metadata !DIExpression()), !dbg [[DBG8]]
+; CHECK-NEXT:    [[SC:%.*]] = icmp eq i32 [[LV]], 100
+; CHECK-NEXT:    br i1 [[SC]], label [[NOCLOBBER:%.*]], label [[CLOBBER:%.*]]
+; CHECK:       noclobber:
+; CHECK-NEXT:    br label [[LOOP_LATCH]]
+; CHECK:       clobber:
+; CHECK-NEXT:    call void @clobber()
+; CHECK-NEXT:    br label [[LOOP_LATCH]]
+; CHECK:       loop.latch:
+; CHECK-NEXT:    [[C:%.*]] = icmp ult i32 [[IV]], [[N]]
+; CHECK-NEXT:    [[IV_NEXT]] = add i32 [[IV]], 1
+; CHECK-NEXT:    br i1 [[C]], label [[LOOP_HEADER]], label [[EXIT_SPLIT:%.*]], !llvm.loop [[LOOP9:![0-9]+]]
+; CHECK:       exit.split:
+; CHECK-NEXT:    br label [[EXIT]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i32 10
+;
+entry:
+  br label %loop.header
+
+loop.header:
+  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop.latch ]
+  %lv = load i32, ptr %ptr
+  call void @llvm.dbg.value(metadata i32 %lv, metadata !6, metadata !DIExpression()), !dbg !7
+  %sc = icmp eq i32 %lv, 100
+  br i1 %sc, label %noclobber, label %clobber
+
+noclobber:
+  br label %loop.latch
+
+clobber:
+  call void @clobber()
+  br label %loop.latch
+
+loop.latch:
+  %c = icmp ult i32 %iv, %N
+  %iv.next = add i32 %iv, 1
+  br i1 %c, label %loop.header, label %exit
+
+exit:
+  ret i32 10
+}
+
+!llvm.module.flags = !{!21}
+!llvm.dbg.cu = !{!2}
+
+!0 = distinct !DISubprogram(name: "foo", line: 2, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !2, file: !20, scope: !1, type: !3)
+!1 = !DIFile(filename: "b.c", directory: "/private/tmp")
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang", isOptimized: true, emissionKind: FullDebug, file: !20)
+!3 = !DISubroutineType(types: !4)
+!4 = !{!5}
+!5 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!6 = !DILocalVariable(name: "i", line: 2, arg: 1, scope: !0, file: !1, type: !5)
+!7 = !DILocation(line: 2, column: 13, scope: !0)
+!9 = !DILocalVariable(name: "k", line: 3, scope: !10, file: !1, type: !5)
+!10 = distinct !DILexicalBlock(line: 2, column: 16, file: !20, scope: !0)
+!11 = !DILocation(line: 3, column: 12, scope: !10)
+!12 = !DILocation(line: 4, column: 3, scope: !10)
+!13 = !DILocation(line: 5, column: 5, scope: !14)
+!14 = distinct !DILexicalBlock(line: 4, column: 10, file: !20, scope: !10)
+!15 = !DILocation(line: 6, column: 3, scope: !14)
+!16 = !DILocation(line: 7, column: 5, scope: !17)
+!17 = distinct !DILexicalBlock(line: 6, column: 10, file: !20, scope: !10)
+!18 = !DILocation(line: 8, column: 3, scope: !17)
+!19 =...
[truncated]

[jmorse]

(Looking at this with the fresh eyes of having clicked the "file PR" button, I wonder whether any of these tests actually want/need live Values instead of constants, so that the remapping is more exercised).

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[OCHyams]

Hasn't eraseFromParent been called on the DbgInfoCursor instructions?

[jmorse]

It's iterating over the source block where things don't get erased, I think it's only the clones that can be simplified away in the blocks above that get deleted. This is a legitimate confusion though, perhaps renaming it to SrcDbgCursor makes it clearer?

[OCHyams]

If IgnoreMissingLocals is false then we know none of the NewVals are nullptr because of the outer if check. So we just need the non-null check, without the || !IgnoreMissingLocals (if !IgnoreMissingLocals and value-is-null then we'd have hit the outer if block). Maybe !IgnoreMissingLocals should be an assert after the non-null check?

[jmorse]

Good catch; I've just removed the check and added a comment, asserting something that's guaranteed three lines above seems like overkill.

[OCHyams]

nit: it's -> its

[OCHyams]

important that it gets duplicated nonetheless.

"it" being "the behaviour"?

[jmorse]

(Submitting comments via the review window to cut down on emails?)

[jmorse]

It's iterating over the source block where things don't get erased, I think it's only the clones that can be simplified away in the blocks above that get deleted. This is a legitimate confusion though, perhaps renaming it to SrcDbgCursor makes it clearer?

[jmorse]

Good catch; I've just removed the check and added a comment, asserting something that's guaranteed three lines above seems like overkill.

[OCHyams]

LGTM

[jmorse]

Landed in 59fab22 -- I had to re-add the --try... flag to branch-fold-dbg.ll as one of the underlying patches had to remove it temporarily.