[indvars] Missing variables at Og:

https://bugs.llvm.org/show_bug.cgi?id=51735
#51077

After internal discussion with @jmorse, it was decided
to split the work between the 'indvars' and the
'loop-deletion' passes.

1) passes="loop(indvars)"
- 'indvars' transformation is fired:
  the 'rewriteLoopExitValues' will rewrite the collected
  PNs with the exit values.
- 'indvars' transformation is not fired:
  If the loop can be deleted, we preserve the induction
  variable information to be used by 'loop-deletion' if
  that pass will be executed.

2) passes="loop(indvars,loop-deletion)"
  If the loop is deleted in 'deleteDeadLoop' and there
  is a valid exit block, we use any collected values
  by 'indvars' to updated the exit values.

Added extra tests to cover the following cases:
  ...
  char Var = 1;
  for (; Index < End; ++Index)
    if (Index == 666)
      ++Var;
  ...

and
  ...
  char Var = 1;
  for (; Index < End; ++Index)
    if (Index == 666)
      Var = 555;
  ...

Modified but otherwise unused variable 'Var' in a loop
that gets deleted.

llvm/include/llvm/Support/GenericLoopInfo.h

@@ -44,6 +44,8 @@
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetOperations.h"
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/GenericDomTree.h"
 
@@ -75,6 +77,13 @@ template <class BlockT, class LoopT> class LoopBase {
   const LoopBase<BlockT, LoopT> &
   operator=(const LoopBase<BlockT, LoopT> &) = delete;
 
+  // Induction variable exit value and its debug users, preserved by the
+  // 'indvars' pass, when it detects that the loop can be deleted and the
+  // there are no PHIs to be rewritten.
+  // For now, we only preserve single induction variables.
+  Value *IndVarFinalValue = nullptr;
+  SmallVector<WeakVH> IndVarDebugUsers;
+
 public:
   /// Return the nesting level of this loop.  An outer-most loop has depth 1,
   /// for consistency with loop depth values used for basic blocks, where depth
@@ -251,6 +260,21 @@ template <class BlockT, class LoopT> class LoopBase {
                           [&](BlockT *Pred) { return contains(Pred); });
   }
 
+  /// Preserve the induction variable exit value and its debug users by the
+  /// 'indvars' pass if the loop can deleted. Those debug users will be used
+  /// by the 'loop-delete' pass.
+  void preserveDebugInductionVariableInfo(
+      Value *FinalValue, SmallVector<DbgVariableIntrinsic *> DbgUsers) {
+    IndVarFinalValue = FinalValue;
+    for (DbgVariableIntrinsic *DebugUser : DbgUsers)
+      IndVarDebugUsers.push_back(DebugUser);
+  }
+
+  Value *getDebugInductionVariableFinalValue() { return IndVarFinalValue; }
+  SmallVector<WeakVH> &getDebugInductionVariableDebugUsers() {
+    return IndVarDebugUsers;
+  }
+
   //===--------------------------------------------------------------------===//
   // APIs for simple analysis of the loop.
   //

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

@@ -482,7 +482,6 @@ void addDebugValuesToIncomingValue(BasicBlock *Successor, Value *IndVar,
                                    PHINode *PN);
 void addDebugValuesToLoopVariable(BasicBlock *Successor, Value *ExitValue,
                                   PHINode *PN);
-void addDebugValuesToLoopVariable(Loop *L, ScalarEvolution *SE, PHINode *PN);
 
 /// Set weights for \p UnrolledLoop and \p RemainderLoop based on weights for
 /// \p OrigLoop and the following distribution of \p OrigLoop iteration among \p

llvm/lib/Transforms/Utils/LoopUtils.cpp

@@ -608,6 +608,17 @@ void llvm::deleteDeadLoop(Loop *L, DominatorTree *DT, ScalarEvolution *SE,
   llvm::SmallVector<DPValue *, 4> DeadDPValues;
 
   if (ExitBlock) {
+    if (ExitBlock->phis().empty()) {
+      // As the loop is deleted, replace the debug users with the preserved
+      // induction variable final value recorded by the 'indvar' pass.
+      Value *FinalValue = L->getDebugInductionVariableFinalValue();
+      SmallVector<WeakVH> &DbgUsers = L->getDebugInductionVariableDebugUsers();
+      for (WeakVH &DebugUser : DbgUsers)
+        if (DebugUser)
+          dyn_cast<DbgVariableIntrinsic>(DebugUser)->replaceVariableLocationOp(
+              0u, FinalValue);
+    }
+
     // Given LCSSA form is satisfied, we should not have users of instructions
     // within the dead loop outside of the loop. However, LCSSA doesn't take
     // unreachable uses into account. We handle them here.
@@ -1418,6 +1429,10 @@ void llvm::addDebugValuesToIncomingValue(BasicBlock *Successor, Value *IndVar,
   SmallVector<DbgVariableIntrinsic *> DbgUsers;
   findDbgUsers(DbgUsers, IndVar);
   for (auto *DebugUser : DbgUsers) {
+    // Skip debug-users with variadic variable locations; they will not,
+    // get updated, which is fine as that is the existing behaviour.
+    if (DebugUser->hasArgList())
+      continue;
     auto *Cloned = cast<DbgVariableIntrinsic>(DebugUser->clone());
     Cloned->replaceVariableLocationOp(0u, PN);
     Cloned->insertBefore(Successor->getFirstNonPHI());
@@ -1439,20 +1454,6 @@ void llvm::addDebugValuesToLoopVariable(BasicBlock *Successor, Value *ExitValue,
   }
 }
 
-void llvm::addDebugValuesToLoopVariable(Loop *L, ScalarEvolution *SE,
-                                        PHINode *PN) {
-  if (!PN)
-    return;
-  const SCEV *PNSCEV = SE->getSCEVAtScope(PN, L->getParentLoop());
-  if (auto *Const = dyn_cast<SCEVConstant>(PNSCEV)) {
-    Value *FinalIVValue = Const->getValue();
-    SmallVector<BasicBlock *> ExitBlocks;
-    L->getExitBlocks(ExitBlocks);
-    for (BasicBlock *Exit : ExitBlocks)
-      addDebugValuesToLoopVariable(Exit, FinalIVValue, PN);
-  }
-}
-
 int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
                                 ScalarEvolution *SE,
                                 const TargetTransformInfo *TTI,
@@ -1595,12 +1596,9 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
           &*Inst->getParent()->getFirstInsertionPt() : Inst;
         RewritePhiSet.emplace_back(PN, i, ExitValue, InsertPt, HighCost);
 
-        // Add debug values if the PN is a induction variable.
-        PHINode *IndVar = L->getInductionVariable(*SE);
-        if (PN->getIncomingValue(i) == IndVar)
-          if (BasicBlock *Successor = ExitBB->getSingleSuccessor())
-            addDebugValuesToIncomingValue(Successor, PN->getIncomingValue(i),
-                                          PN);
+        // Add debug values for the candidate PHINode incoming value.
+        if (BasicBlock *Successor = ExitBB->getSingleSuccessor())
+          addDebugValuesToIncomingValue(Successor, PN->getIncomingValue(i), PN);
       }
     }
   }
@@ -1665,14 +1663,22 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
     }
   }
 
-  // If there are no PHIs to be rewritten then there are no loop live-out
-  // values, try to rewrite debug variables corresponding to the induction
-  // variable with their constant exit-values if we computed any. Otherwise
-  // debug-info will completely forget that this loop happened.
-  if (RewritePhiSet.empty()) {
-    // The loop exit value has been updated; insert the debug location
-    // for the given induction variable with its final value.
-    addDebugValuesToLoopVariable(L, SE, L->getInductionVariable(*SE));
+  // If the loop can be deleted and there are no PHIs to be rewritten (there
+  // are no loop live-out values), record debug variables corresponding to the
+  // induction variable with their constant exit-values. Those values will be
+  // inserted by the 'deletion loop' logic.
+  if (LoopCanBeDel && RewritePhiSet.empty()) {
+    if (auto *IndVar = L->getInductionVariable(*SE)) {
+      const SCEV *PNSCEV = SE->getSCEVAtScope(IndVar, L->getParentLoop());
+      if (auto *Const = dyn_cast<SCEVConstant>(PNSCEV)) {
+        Value *FinalIVValue = Const->getValue();
+        if (L->getUniqueExitBlock()) {
+          SmallVector<DbgVariableIntrinsic *> DbgUsers;
+          findDbgUsers(DbgUsers, IndVar);
+          L->preserveDebugInductionVariableInfo(FinalIVValue, DbgUsers);
+        }
+      }
+    }
   }
 
   // The insertion point instruction may have been deleted; clear it out

llvm/test/Transforms/LoopSimplify/pr51735-1.ll → llvm/test/Transforms/IndVarSimplify/pr51735-1.ll

@@ -1,8 +1,17 @@
-; RUN: opt -passes=indvars -S -o - < %s | FileCheck %s
+; RUN: opt -passes="loop(indvars)" \
+; RUN:     --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --check-prefix=CHECK %s
+; RUN: opt -passes="loop(indvars,loop-deletion)" \
+; RUN:     --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --check-prefix=CHECK %s
 
 ; Make sure that when we delete the loop, that the variable Index has
 ; the 777 value.
 
+; As this test case does fire the 'indvars' transformation, the debug values
+; are added to the 'for.end' exit block. No debug values are preserved by the
+; pass to be used by the 'loop-deletion' pass.
+
 ; CHECK: for.cond:
 ; CHECK:   call void @llvm.dbg.value(metadata i32 %Index.0, metadata ![[DBG:[0-9]+]], {{.*}}
 

llvm/test/Transforms/IndVarSimplify/pr51735-2.ll

@@ -0,0 +1,129 @@
+; RUN: opt -passes="loop(indvars)" \
+; RUN:     --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --implicit-check-not="call void @llvm.dbg" \
+; RUN:           --check-prefix=ALL-CHECK --check-prefix=PRE-CHECK %s
+; RUN: opt -passes="loop(indvars,loop-deletion)" \
+; RUN:     --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --implicit-check-not="call void @llvm.dbg" \
+; RUN:           --check-prefix=ALL-CHECK --check-prefix=POST-CHECK %s
+
+; Check what happens to a modified but otherwise unused variable in a loop
+; that gets deleted. The assignment in the loop is 'forgotten' by LLVM and
+; doesn't appear in the debugging information. This behaviour is suboptimal,
+; but we want to know if it changes
+
+; For all cases, LLDB shows
+;   Var = <no location, value may have been optimized out>
+
+;  1	__attribute__((optnone)) int nop() {
+;  2	  return 0;
+;  3	}
+;  4
+;  5	void bar() {
+;  6    int End = 777;
+;  7	  int Index = 27;
+;  8	  char Var = 1;
+;  9	  for (; Index < End; ++Index) {
+; 10      if (Index == 666) {
+; 11        ++Var;
+; 12      }
+; 13    }
+; 14	  nop();
+; 15	}
+
+; ALL-CHECK: entry:
+; ALL-CHECK:   call void @llvm.dbg.value(metadata i32 1, metadata ![[DBG:[0-9]+]], {{.*}}
+
+; Only the 'indvars' pass is executed.
+; PRE-CHECK: for.cond:
+; PRE-CHECK:   call void @llvm.dbg.value(metadata i32 %Var.0, metadata ![[DBG]], {{.*}}
+; PRE-CHECK:   call void @llvm.dbg.value(metadata !DIArgList{{.*}}
+
+; PRE-CHECK: for.body:
+; PRE-CHECK:   %cmp1 = icmp eq i32 %Index.0, 666
+; PRE-CHECK:   %inc = add nsw i32 %Var.0, 1
+; PRE-CHECK:   %spec.select = select i1 %cmp1, i32 %inc, i32 %Var.0
+; PRE-CHECK:   call void @llvm.dbg.value(metadata i32 %spec.select, metadata ![[DBG]], {{.*}}
+; PRE-CHECK:   br label %for.cond
+
+; PRE-CHECK: for.end:
+; PRE-CHECK-NOT: call void @llvm.dbg.value
+; PRE-CHECK:   ret void
+; PRE-CHECK-DAG: ![[DBG]] = !DILocalVariable(name: "Var"{{.*}})
+
+; The 'indvars' and 'loop-deletion' passes are executed.
+; POST-CHECK: for.end:
+; POST-CHECK:   call void @llvm.dbg.value(metadata i32 undef, metadata ![[DBG:[0-9]+]], {{.*}}
+; POST-CHECK:   ret void
+; POST-CHECK-DAG: ![[DBG]] = !DILocalVariable(name: "Var"{{.*}})
+
+define dso_local void @_Z3barv() local_unnamed_addr !dbg !18 {
+entry:
+  call void @llvm.dbg.value(metadata i32 1, metadata !24, metadata !DIExpression()), !dbg !22
+  br label %for.cond, !dbg !25
+
+for.cond:                                         ; preds = %for.cond, %entry
+  %Index.0 = phi i32 [ 27, %entry ], [ %inc2, %for.body ], !dbg !22
+  %Var.0 = phi i32 [ 1, %entry ], [ %spec.select, %for.body ], !dbg !22
+  call void @llvm.dbg.value(metadata i32 %Var.0, metadata !24, metadata !DIExpression()), !dbg !22
+  %cmp = icmp ult i32 %Index.0, 777, !dbg !26
+  call void @llvm.dbg.value(metadata !DIArgList(i32 poison, i32 %Index.0), metadata !24, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 666, DW_OP_eq, DW_OP_LLVM_convert, 1, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_plus, DW_OP_stack_value)), !dbg !22
+  %inc2 = add nuw nsw i32 %Index.0, 1, !dbg !29
+  br i1 %cmp, label %for.body, label %for.end, !dbg !30, !llvm.loop !31
+
+for.body:                                         ; preds = %for.cond
+  %cmp1 = icmp eq i32 %Index.0, 666, !dbg !30
+  %inc = add nsw i32 %Var.0, 1
+  %spec.select = select i1 %cmp1, i32 %inc, i32 %Var.0, !dbg !32
+  call void @llvm.dbg.value(metadata i32 %spec.select, metadata !24, metadata !DIExpression()), !dbg !22
+  br label %for.cond, !dbg !34, !llvm.loop !35
+
+for.end:                                          ; preds = %for.cond
+  ret void, !dbg !35
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test-b.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 19.0.0"}
+!10 = distinct !DISubprogram(name: "nop", linkageName: "_Z3nopi", scope: !1, file: !1, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13, !13}
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !{}
+!15 = !DILocalVariable(name: "Param", arg: 1, scope: !10, file: !1, line: 1, type: !13)
+!16 = !DILocation(line: 1, column: 38, scope: !10)
+!17 = !DILocation(line: 2, column: 3, scope: !10)
+!18 = distinct !DISubprogram(name: "bar", linkageName: "_Z3barv", scope: !1, file: !1, line: 5, type: !19, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!19 = !DISubroutineType(types: !20)
+!20 = !{null}
+!22 = !DILocation(line: 0, scope: !18)
+!24 = !DILocalVariable(name: "Var", scope: !18, file: !1, line: 8, type: !13)
+!25 = !DILocation(line: 9, column: 3, scope: !18)
+!26 = !DILocation(line: 9, column: 16, scope: !27)
+!27 = distinct !DILexicalBlock(scope: !28, file: !1, line: 9, column: 3)
+!28 = distinct !DILexicalBlock(scope: !18, file: !1, line: 9, column: 3)
+!29 = !DILocation(line: 9, column: 23, scope: !27)
+!30 = !DILocation(line: 9, column: 3, scope: !28)
+!31 = distinct !{!31, !30, !32, !33}
+!32 = !DILocation(line: 11, column: 9, scope: !28)
+!33 = !{!"llvm.loop.mustprogress"}
+!34 = !DILocation(line: 12, column: 3, scope: !18)
+!35 = !DILocation(line: 13, column: 1, scope: !18)
+!36 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 15, type: !37, scopeLine: 15, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0)
+!37 = !DISubroutineType(types: !38)
+!38 = !{!13}
+!39 = !DILocation(line: 16, column: 3, scope: !36)
+!40 = !DILocation(line: 17, column: 1, scope: !36)