[loop-deletion] Overly defensive with undef-ing dbg.values.

[CarlosAlbertoEnciso]

Summary

Explicitly inserting undef is overly defensive. Any values computed
inside the loop that are referenced by dbg.values should naturally
become undef when the loop is deleted, and all other values that
are loop invariant must be preserved.

[scott.linder]

LGTM

I am not particularly familiar with LoopDeletion, but if the premise from the comment holds (i.e. only debug intrinsics defined in terms of loop invariants will survive) then this sounds like a nice improvement!

[CarlosAlbertoEnciso]

@scott.linder Thanks for your valuable feedback.

llvm/lib/Transforms/Utils/LoopUtils.cpp

for (auto *Block : L->blocks())
         if (!DVI)
           continue;
         if (!DeadDebugSet.insert(DebugVariable(DVI)).second)
           continue;
         DeadDebugInst.push_back(DVI);
       }
 
     // After the loop has been deleted all the values defined and modified
-    // inside the loop are going to be unavailable.
-    // Since debug values in the loop have been deleted, inserting an undef
-    // dbg.value truncates the range of any dbg.value before the loop where the
-    // loop used to be. This is particularly important for constant values.
+    // inside the loop are going to be unavailable. Values computed in the
+    // loop will have been deleted, automatically causing their debug uses
+    // be be replaced with undef. Loop invariant values will still be available.
+    // Move dbg.values out the loop so that earlier location ranges are still
+    // terminated and loop invariant assignments are preserved.
     Instruction *InsertDbgValueBefore = ExitBlock->getFirstNonPHI();
     assert(InsertDbgValueBefore &&
            "There should be a non-PHI instruction in exit block, else these "
            "instructions will have no parent.");
-    for (auto *DVI : DeadDebugInst) {
-      DVI->setKillLocation();
+    for (auto *DVI : DeadDebugInst)
       DVI->moveBefore(InsertDbgValueBefore);
-    }
-  }
 
   // Remove the block from the reference counting scheme, so that we can
   // delete it freely later.
   for (auto *Block : L->blocks())
     Block->dropAllReferences();
 
   if (MSSA && VerifyMemorySSA)
     MSSA->verifyMemorySSA();

llvm/test/DebugInfo/Generic/assignment-tracking/loop-deletion/dead-loop.ll

 ;; $ clang++ -O2 -g -Xclang -fexperimental-assignment-tracking
 ;;
 ;; for.cond is a dead loop - the debug intrinsic inside will be moved to the
 ;; exit block (and made undef) and then the loop will be deleted. Ensure that
 ;; the dbg.assign intrinsic doesn't get transformed into a dbg.value by
 ;; mistake.
 
 ; CHECK: for.end:
-; CHECK-NEXT: call void @llvm.dbg.assign(metadata i32 poison,{{.+}}, metadata !DIExpression({{.+}}), metadata ![[ID:[0-9]+]], metadata ptr %Counter, metadata !DIExpression())
+; CHECK-NEXT: call void @llvm.dbg.assign(metadata i32 undef,{{.+}}, metadata !DIExpression({{.+}}), metadata ![[ID:[0-9]+]], metadata ptr %Counter, metadata !DIExpression())
 ; CHECK-NEXT: store i32 2, ptr %Counter, align 4,{{.*}}!DIAssignID ![[ID]]
 
 define dso_local void @_Z3funv() local_unnamed_addr #0 !dbg !7 {
 entry:
   %Counter = alloca i32, align 4, !DIAssignID !13
   call void @llvm.dbg.assign(metadata i1 undef, metadata !11, metadata !DIExpression(), metadata !13, metadata ptr %Counter, metadata !DIExpression()), !dbg !14
   call void @llvm.dbg.assign(metadata i32 0, metadata !11, metadata !DIExpression(), metadata !16, metadata ptr %Counter, metadata !DIExpression()), !dbg !14
   br label %for.cond, !dbg !17

llvm/test/Transforms/LoopDeletion/diundef.ll

 for.cond:                                         ; preds = %for.cond, %entry
   %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.cond ], !dbg !20
   call void @llvm.dbg.value(metadata i32 %i.0, metadata !16, metadata !DIExpression()), !dbg !17
   %inc = add nuw nsw i32 %i.0, 1, !dbg !21
   call void @llvm.dbg.value(metadata i32 %inc, metadata !16, metadata !DIExpression()), !dbg !17
   %exitcond = icmp ne i32 %inc, 3, !dbg !23
   br i1 %exitcond, label %for.cond, label %for.end, !dbg !24, !llvm.loop !25
 
-; CHECK: call void @llvm.dbg.value(metadata i32 poison, metadata !16, metadata !DIExpression()), !dbg !17
+; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata !16, metadata !DIExpression()), !dbg !17
 ; CHECK-NEXT: %call = tail call i32 {{.*}} @patatino()
 for.end:                                          ; preds = %for.cond
   %call = tail call i32 (...) @patatino() #3, !dbg !27
   %0 = load i32, ptr @a, align 4, !dbg !28
   ret i32 %0, !dbg !33
 }
 
 declare i32 @patatino(...) local_unnamed_addr

llvm/test/Transforms/LoopDeletion/over-defensive-undefing-dbg-values.ll

+; RUN: opt -S %s -passes=loop-deletion | FileCheck %s
+
+;; static int foo(int Param) __attribute__((always_inline));
+;; static int foo(int Param) { return Param * Param * 2; }
+;;
+;; static int zero() __attribute__((always_inline));
+;; static int zero() { return 0; }
+;;
+;; void test() {
+;;   int Constant = 0;
+;;   for (int Index = zero(); Index < 0; ++Index) {
+;;     foo(Index);
+;;     Constant = 5;
+;;     foo(Index + 1);
+;;   }
+;; }
+;;
+;; Check that a loop invariant value in a dbg.value inside the dead
+;; loop is preserved.
+
+; CHECK-LABEL: for.end:
+; CHECK-NEXT:    @llvm.dbg.value({{.+}} undef, metadata ![[VAR1:[0-9]+]],{{.+}}), !dbg ![[DBG1:[0-9]+]]
+; CHECK-NEXT:    @llvm.dbg.value({{.+}} 5, metadata ![[VAR2:[0-9]+]],{{.+}}), !dbg ![[DBG2:[0-9]+]]
+
+; CHECK-DAG: ![[VAR1]] = !DILocalVariable(name: "Index"
+; CHECK-DAG: ![[VAR2]] = !DILocalVariable(name: "Constant"
+
+; CHECK-DAG: ![[DBG1]] = !DILocation(line: 0
+; CHECK-DAG: ![[DBG2]] = !DILocation(line: 0
+
+define dso_local void @test() !dbg !9 {
+entry:
+  call void @llvm.dbg.value(metadata i32 0, metadata !13, metadata !DIExpression()), !dbg !15
+  call void @llvm.dbg.value(metadata i32 0, metadata !16, metadata !DIExpression()), !dbg !18
+  br label %for.end, !dbg !19
+
+for.end:                                          ; preds = %entry
+  call void @llvm.dbg.value(metadata i32 undef, metadata !16, metadata !DIExpression()), !dbg !18
+  call void @llvm.dbg.value(metadata i32 5, metadata !13, metadata !DIExpression()), !dbg !15
+  ret void, !dbg !30
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 17.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test", directory: "", checksumkind: CSK_MD5, checksum: "53f7b620ca60e4fc7af4e6e4cdac8eea")
+!2 = !{i32 2, !"CodeView", i32 1}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 2}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"uwtable", i32 2}
+!7 = !{i32 1, !"MaxTLSAlign", i32 65536}
+!8 = !{!"clang version 17.0.0"}
+!9 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 7, type: !10, scopeLine: 7, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !12)
+!10 = !DISubroutineType(types: !11)
+!11 = !{null}
+!12 = !{}
+!13 = !DILocalVariable(name: "Constant", scope: !9, file: !1, line: 8, type: !14)
+!14 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!15 = !DILocation(line: 0, scope: !9)
+!16 = !DILocalVariable(name: "Index", scope: !17, file: !1, line: 9, type: !14)
+!17 = distinct !DILexicalBlock(scope: !9, file: !1, line: 9)
+!18 = !DILocation(line: 0, scope: !17)
+!19 = !DILocation(line: 9, scope: !17)
+!21 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !22, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !0, retainedNodes: !12)
+!22 = !DISubroutineType(types: !23)
+!23 = !{!14, !14}
+!24 = !DILocation(line: 0, scope: !21, inlinedAt: !25)
+!25 = distinct !DILocation(line: 10, scope: !26)
+!26 = distinct !DILexicalBlock(scope: !27, file: !1, line: 9)
+!27 = distinct !DILexicalBlock(scope: !17, file: !1, line: 9)
+!28 = !DILocation(line: 0, scope: !21, inlinedAt: !29)
+!29 = distinct !DILocation(line: 12, scope: !26)
+!30 = !DILocation(line: 14, scope: !9)