[DebugInfo][RemoveDIs] Have LICM insert at iterator positions (#73671)

Because we're storing some extra debug-info information in the iterator
class, we need to insert new LICM-created stores using such iterators.
Switch LICM to storing iterators instead of pointers when it promotes
variables in loops, add a test for the desired behaviour, and enable
RemoveDIs instrumentation on a variety of other LICM tests for good
measure.

(This would appear to be the only pass in LLVM that needs to store
iterators on the heap).

llvm/include/llvm/IR/Instructions.h

@@ -317,17 +317,25 @@ class StoreInst : public Instruction {
 public:
   StoreInst(Value *Val, Value *Ptr, Instruction *InsertBefore);
   StoreInst(Value *Val, Value *Ptr, BasicBlock *InsertAtEnd);
+  StoreInst(Value *Val, Value *Ptr, BasicBlock::iterator InsertBefore);
   StoreInst(Value *Val, Value *Ptr, bool isVolatile, Instruction *InsertBefore);
   StoreInst(Value *Val, Value *Ptr, bool isVolatile, BasicBlock *InsertAtEnd);
+  StoreInst(Value *Val, Value *Ptr, bool isVolatile,
+            BasicBlock::iterator InsertBefore);
   StoreInst(Value *Val, Value *Ptr, bool isVolatile, Align Align,
             Instruction *InsertBefore = nullptr);
   StoreInst(Value *Val, Value *Ptr, bool isVolatile, Align Align,
             BasicBlock *InsertAtEnd);
+  StoreInst(Value *Val, Value *Ptr, bool isVolatile, Align Align,
+            BasicBlock::iterator InsertBefore);
   StoreInst(Value *Val, Value *Ptr, bool isVolatile, Align Align,
             AtomicOrdering Order, SyncScope::ID SSID = SyncScope::System,
             Instruction *InsertBefore = nullptr);
   StoreInst(Value *Val, Value *Ptr, bool isVolatile, Align Align,
             AtomicOrdering Order, SyncScope::ID SSID, BasicBlock *InsertAtEnd);
+  StoreInst(Value *Val, Value *Ptr, bool isVolatile, Align Align,
+            AtomicOrdering Order, SyncScope::ID SSID,
+            BasicBlock::iterator InsertBefore);
 
   // allocate space for exactly two operands
   void *operator new(size_t S) { return User::operator new(S, 2); }

llvm/include/llvm/Transforms/Utils/LoopUtils.h

@@ -215,7 +215,7 @@ void breakLoopBackedge(Loop *L, DominatorTree &DT, ScalarEvolution &SE,
 /// guaranteed to execute in the loop, but are safe to speculatively execute.
 bool promoteLoopAccessesToScalars(
     const SmallSetVector<Value *, 8> &, SmallVectorImpl<BasicBlock *> &,
-    SmallVectorImpl<Instruction *> &, SmallVectorImpl<MemoryAccess *> &,
+    SmallVectorImpl<BasicBlock::iterator> &, SmallVectorImpl<MemoryAccess *> &,
     PredIteratorCache &, LoopInfo *, DominatorTree *, AssumptionCache *AC,
     const TargetLibraryInfo *, TargetTransformInfo *, Loop *,
     MemorySSAUpdater &, ICFLoopSafetyInfo *, OptimizationRemarkEmitter *,

llvm/lib/IR/Instructions.cpp

@@ -1462,6 +1462,9 @@ StoreInst::StoreInst(Value *val, Value *addr, Instruction *InsertBefore)
 StoreInst::StoreInst(Value *val, Value *addr, BasicBlock *InsertAtEnd)
     : StoreInst(val, addr, /*isVolatile=*/false, InsertAtEnd) {}
 
+StoreInst::StoreInst(Value *val, Value *addr, BasicBlock::iterator InsertBefore)
+    : StoreInst(val, addr, /*isVolatile=*/false, InsertBefore) {}
+
 StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile,
                      Instruction *InsertBefore)
     : StoreInst(val, addr, isVolatile,
@@ -1474,6 +1477,12 @@ StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile,
                 computeLoadStoreDefaultAlign(val->getType(), InsertAtEnd),
                 InsertAtEnd) {}
 
+StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile,
+                     BasicBlock::iterator InsertBefore)
+    : StoreInst(val, addr, isVolatile,
+                computeLoadStoreDefaultAlign(val->getType(), &*InsertBefore),
+                InsertBefore) {}
+
 StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile, Align Align,
                      Instruction *InsertBefore)
     : StoreInst(val, addr, isVolatile, Align, AtomicOrdering::NotAtomic,
@@ -1484,6 +1493,11 @@ StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile, Align Align,
     : StoreInst(val, addr, isVolatile, Align, AtomicOrdering::NotAtomic,
                 SyncScope::System, InsertAtEnd) {}
 
+StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile, Align Align,
+                     BasicBlock::iterator InsertBefore)
+    : StoreInst(val, addr, isVolatile, Align, AtomicOrdering::NotAtomic,
+                SyncScope::System, InsertBefore) {}
+
 StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile, Align Align,
                      AtomicOrdering Order, SyncScope::ID SSID,
                      Instruction *InsertBefore)
@@ -1512,6 +1526,20 @@ StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile, Align Align,
   AssertOK();
 }
 
+StoreInst::StoreInst(Value *val, Value *addr, bool isVolatile, Align Align,
+                     AtomicOrdering Order, SyncScope::ID SSID,
+                     BasicBlock::iterator InsertBefore)
+    : Instruction(Type::getVoidTy(val->getContext()), Store,
+                  OperandTraits<StoreInst>::op_begin(this),
+                  OperandTraits<StoreInst>::operands(this)) {
+  Op<0>() = val;
+  Op<1>() = addr;
+  setVolatile(isVolatile);
+  setAlignment(Align);
+  setAtomic(Order, SSID);
+  insertBefore(*InsertBefore->getParent(), InsertBefore);
+  AssertOK();
+}
 
 //===----------------------------------------------------------------------===//
 //                       AtomicCmpXchgInst Implementation

llvm/lib/Transforms/Scalar/LICM.cpp

@@ -481,12 +481,12 @@ bool LoopInvariantCodeMotion::runOnLoop(Loop *L, AAResults *AA, LoopInfo *LI,
     });
 
     if (!HasCatchSwitch) {
-      SmallVector<Instruction *, 8> InsertPts;
+      SmallVector<BasicBlock::iterator, 8> InsertPts;
       SmallVector<MemoryAccess *, 8> MSSAInsertPts;
       InsertPts.reserve(ExitBlocks.size());
       MSSAInsertPts.reserve(ExitBlocks.size());
       for (BasicBlock *ExitBlock : ExitBlocks) {
-        InsertPts.push_back(&*ExitBlock->getFirstInsertionPt());
+        InsertPts.push_back(ExitBlock->getFirstInsertionPt());
         MSSAInsertPts.push_back(nullptr);
       }
 
@@ -1794,7 +1794,7 @@ namespace {
 class LoopPromoter : public LoadAndStorePromoter {
   Value *SomePtr; // Designated pointer to store to.
   SmallVectorImpl<BasicBlock *> &LoopExitBlocks;
-  SmallVectorImpl<Instruction *> &LoopInsertPts;
+  SmallVectorImpl<BasicBlock::iterator> &LoopInsertPts;
   SmallVectorImpl<MemoryAccess *> &MSSAInsertPts;
   PredIteratorCache &PredCache;
   MemorySSAUpdater &MSSAU;
@@ -1828,7 +1828,7 @@ class LoopPromoter : public LoadAndStorePromoter {
 public:
   LoopPromoter(Value *SP, ArrayRef<const Instruction *> Insts, SSAUpdater &S,
                SmallVectorImpl<BasicBlock *> &LEB,
-               SmallVectorImpl<Instruction *> &LIP,
+               SmallVectorImpl<BasicBlock::iterator> &LIP,
                SmallVectorImpl<MemoryAccess *> &MSSAIP, PredIteratorCache &PIC,
                MemorySSAUpdater &MSSAU, LoopInfo &li, DebugLoc dl,
                Align Alignment, bool UnorderedAtomic, const AAMDNodes &AATags,
@@ -1851,7 +1851,7 @@ class LoopPromoter : public LoadAndStorePromoter {
       Value *LiveInValue = SSA.GetValueInMiddleOfBlock(ExitBlock);
       LiveInValue = maybeInsertLCSSAPHI(LiveInValue, ExitBlock);
       Value *Ptr = maybeInsertLCSSAPHI(SomePtr, ExitBlock);
-      Instruction *InsertPos = LoopInsertPts[i];
+      BasicBlock::iterator InsertPos = LoopInsertPts[i];
       StoreInst *NewSI = new StoreInst(LiveInValue, Ptr, InsertPos);
       if (UnorderedAtomic)
         NewSI->setOrdering(AtomicOrdering::Unordered);
@@ -1949,7 +1949,7 @@ bool isThreadLocalObject(const Value *Object, const Loop *L, DominatorTree *DT,
 bool llvm::promoteLoopAccessesToScalars(
     const SmallSetVector<Value *, 8> &PointerMustAliases,
     SmallVectorImpl<BasicBlock *> &ExitBlocks,
-    SmallVectorImpl<Instruction *> &InsertPts,
+    SmallVectorImpl<BasicBlock::iterator> &InsertPts,
     SmallVectorImpl<MemoryAccess *> &MSSAInsertPts, PredIteratorCache &PIC,
     LoopInfo *LI, DominatorTree *DT, AssumptionCache *AC,
     const TargetLibraryInfo *TLI, TargetTransformInfo *TTI, Loop *CurLoop,

llvm/test/DebugInfo/X86/licm-undef-dbg-value.ll

@@ -1,4 +1,5 @@
 ; RUN: opt -passes=licm %s -S | FileCheck %s
+; RUN: opt -passes=licm %s -S --try-experimental-debuginfo-iterators | FileCheck %s
 
 ; CHECK: for.body:
 ; CHECK-NEXT: llvm.dbg.value(metadata i8 poison

llvm/test/Transforms/LICM/dbg-value-sink.ll

@@ -0,0 +1,79 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-attributes
+; RUN: opt < %s -passes='loop-mssa(licm)' -S | FileCheck %s
+; RUN: opt < %s -passes='loop-mssa(licm)' -S --try-experimental-debuginfo-iterators | FileCheck %s
+; RUN: opt -aa-pipeline=tbaa,basic-aa -passes='require<aa>,require<targetir>,require<scalar-evolution>,require<opt-remark-emit>,loop-mssa(licm)' -S %s | FileCheck %s
+; RUN: opt -aa-pipeline=tbaa,basic-aa -passes='require<aa>,require<targetir>,require<scalar-evolution>,require<opt-remark-emit>,loop-mssa(licm)' -S %s --try-experimental-debuginfo-iterators| FileCheck %s
+;
+; Test that when we sink the store into the "Out" block, it lands in front of
+; the dbg.value that we've left there.
+;
+target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
+
+@X = global i32 7   ; <ptr> [#uses=4]
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+define void @test1(i32 %i) {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:  Entry:
+; CHECK-NEXT:    [[X_PROMOTED:%.*]] = load i32, ptr @X, align 4
+; CHECK-NEXT:    br label [[LOOP:%.*]], !dbg [[DBG5:![0-9]+]]
+; CHECK:       Loop:
+; CHECK-NEXT:    [[X21:%.*]] = phi i32 [ [[X_PROMOTED]], [[ENTRY:%.*]] ], [ [[X2:%.*]], [[LOOP]] ], !dbg [[DBG5]]
+; CHECK-NEXT:    [[J:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[X2]] = add i32 [[X21]], 1, !dbg [[DBG5]]
+; CHECK-NEXT:    [[NEXT]] = add i32 [[J]], 1, !dbg [[DBG5]]
+; CHECK-NEXT:    [[COND:%.*]] = icmp eq i32 [[NEXT]], 0, !dbg [[DBG5]]
+; CHECK-NEXT:    br i1 [[COND]], label [[OUT:%.*]], label [[LOOP]], !dbg [[DBG5]]
+; CHECK:       Out:
+; CHECK-NEXT:    [[X2_LCSSA:%.*]] = phi i32 [ [[X2]], [[LOOP]] ]
+; CHECK-NEXT:    store i32 [[X2_LCSSA]], ptr @X, align 4, !dbg [[DBG5]]
+; CHECK-NEXT:    tail call void @llvm.dbg.value(metadata i32 0, metadata [[META12:![0-9]+]], metadata !DIExpression()), !dbg [[DBG5]]
+; CHECK-NEXT:    ret void, !dbg [[DBG5]]
+;
+Entry:
+  br label %Loop, !dbg !16
+
+Loop:
+  %j = phi i32 [ 0, %Entry ], [ %Next, %Loop ]
+  %x = load i32, ptr @X, !dbg !16
+  %x2 = add i32 %x, 1, !dbg !16
+  store i32 %x2, ptr @X, !dbg !16
+  %Next = add i32 %j, 1, !dbg !16
+  %cond = icmp eq i32 %Next, 0, !dbg !16
+  br i1 %cond, label %Out, label %Loop, !dbg !16
+
+Out:
+  tail call void @llvm.dbg.value(metadata i32 0, metadata !11, metadata !DIExpression()), !dbg !16
+  ret void, !dbg !16
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!2, !3}
+!llvm.module.flags = !{!4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "dbg-value-sink.ll", directory: "/")
+!2 = !{i32 9}
+!3 = !{i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "test1", linkageName: "test1", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+!6 = !DISubroutineType(types: !7)
+!7 = !{}
+!8 = !{!9, !11, !12, !13, !14}
+!9 = !DILocalVariable(name: "1", scope: !5, file: !1, line: 2, type: !10)
+!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
+!11 = !DILocalVariable(name: "2", scope: !5, file: !1, line: 3, type: !10)
+!12 = !DILocalVariable(name: "3", scope: !5, file: !1, line: 4, type: !10)
+!13 = !DILocalVariable(name: "4", scope: !5, file: !1, line: 6, type: !10)
+!14 = !DILocalVariable(name: "5", scope: !5, file: !1, line: 7, type: !15)
+!15 = !DIBasicType(name: "ty8", size: 8, encoding: DW_ATE_unsigned)
+!16 = !DILocation(line: 1, column: 1, scope: !5)
+!17 = !DILocation(line: 2, column: 1, scope: !5)
+!18 = !DILocation(line: 3, column: 1, scope: !5)
+!19 = !DILocation(line: 4, column: 1, scope: !5)
+!20 = !DILocation(line: 5, column: 1, scope: !5)
+!21 = !DILocation(line: 6, column: 1, scope: !5)
+!22 = !DILocation(line: 7, column: 1, scope: !5)
+!23 = !DILocation(line: 8, column: 1, scope: !5)
+!24 = !DILocation(line: 9, column: 1, scope: !5)

llvm/test/Transforms/LICM/debug-value.ll

@@ -1,6 +1,8 @@
 ; RUN: opt -passes=licm < %s -S | FileCheck %s
 ; RUN: opt -aa-pipeline=basic-aa -passes='require<aa>,require<targetir>,require<scalar-evolution>,require<opt-remark-emit>,loop-mssa(licm)' < %s -S | FileCheck %s
 
+; RUN: opt -passes=licm < %s -S --try-experimental-debuginfo-iterators | FileCheck %s
+
 define void @dgefa() nounwind ssp {
 entry:
   br label %for.body

llvm/test/Transforms/LICM/hoist-debuginvariant.ll

@@ -1,5 +1,6 @@
 ; RUN: opt < %s -strip-debug -passes=licm -S | FileCheck %s
 ; RUN: opt < %s -passes=licm -verify-memoryssa -S | FileCheck %s
+; RUN: opt < %s -passes=licm -verify-memoryssa -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"