[DebugInfo][RemoveDIs] Support cloning and remapping DPValues (#72546)

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.

Author: jmorse

llvm/include/llvm/Transforms/Utils/ValueMapper.h

@@ -15,21 +15,26 @@
 #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;
 
 using ValueToValueMapTy = ValueMap<const Value *, WeakTrackingVH>;
+using DPValueIterator = simple_ilist<DPValue>::iterator;
 
 /// This is a class that can be implemented by clients to remap types when
 /// cloning constants and instructions.
@@ -175,6 +180,8 @@ class ValueMapper {
   Constant *mapConstant(const Constant &C);
 
   void remapInstruction(Instruction &I);
+  void remapDPValue(Module *M, DPValue &V);
+  void remapDPValueRange(Module *M, iterator_range<DPValueIterator> Range);
   void remapFunction(Function &F);
   void remapGlobalObjectMetadata(GlobalObject &GO);
 
@@ -260,6 +267,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<DPValueIterator> 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

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

@@ -1257,8 +1257,11 @@ 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))

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()) {

llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp

@@ -913,9 +913,12 @@ 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);
+      RemapDPValueRange(M, I.getDbgValueRange(), VMap,
+                        RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
     }
   }
 

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

@@ -1126,6 +1126,9 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
     NewBonusInst->dropUBImplyingAttrsAndMetadata();
 
     NewBonusInst->insertInto(PredBlock, PTI->getIterator());
+    auto Range = NewBonusInst->cloneDebugInfoFrom(&BonusInst);
+    RemapDPValueRange(NewBonusInst->getModule(), Range, VMap,
+                      RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
 
     if (isa<DbgInfoIntrinsic>(BonusInst))
       continue;
@@ -3303,6 +3306,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 SrcDbgCursor = BB->begin();
     for (BasicBlock::iterator BBI = BB->begin(); &*BBI != BI; ++BBI) {
       if (PHINode *PN = dyn_cast<PHINode>(BBI)) {
         TranslateMap[PN] = PN->getIncomingValueForBlock(EdgeBB);
@@ -3337,13 +3344,26 @@ 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 (; SrcDbgCursor != BBI; ++SrcDbgCursor)
+          N->cloneDebugInfoFrom(&*SrcDbgCursor);
+        SrcDbgCursor = 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)
             AC->registerAssumption(Assume);
       }
     }
 
+    for (; &*SrcDbgCursor != BI; ++SrcDbgCursor)
+      InsertPt->cloneDebugInfoFrom(&*SrcDbgCursor);
+    InsertPt->cloneDebugInfoFrom(BI);
+
     BB->removePredecessor(EdgeBB);
     BranchInst *EdgeBI = cast<BranchInst>(EdgeBB->getTerminator());
     EdgeBI->setSuccessor(0, RealDest);
@@ -3748,6 +3768,16 @@ 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()];

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,39 @@ 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 {
+    // Either we have all non-empty NewVals, or we're permitted to ignore
+    // missing locals.
+    for (unsigned int I = 0; I < Vals.size(); ++I)
+      if (NewVals[I])
+        V.replaceVariableLocationOp(I, NewVals[I]);
+  }
+}
+
 Value *Mapper::mapBlockAddress(const BlockAddress &BA) {
   Function *F = cast<Function>(mapValue(BA.getFunction()));
 
@@ -1179,6 +1214,17 @@ 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);
 }

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 its 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]+]]
 

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 the dbg.value 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 = !DILocation(line: 9, column: 3, scope: !10)
+!20 = !DIFile(filename: "b.c", directory: "/private/tmp")
+!21 = !{i32 1, !"Debug Info Version", i32 3}
+
+

llvm/test/Transforms/SimplifyCFG/branch-fold-dbg.ll

@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck %s
+; RUN: opt -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S < %s --try-experimental-debuginfo-iterators | FileCheck %s
 
 %0 = type { ptr, ptr }