[Unroll][DebugInfo] Propagate loop body's debug location to epilog preheader

[zzheng]

Summary

NewExit and epilog PreHeader should has the same debug loc as the original loop
body, instead of original loop exit.

[aprantl]

I'll defer to Dehao. Can you see whether this messes with SBPGO?

[probinson]

test/Transforms/LoopUnroll/runtime-loop1.ll

16

The test is now defining BODY_LOC three times in the EPILOG run. Once should be enough. (The unmodified test incorrectly defines it twice, and you've added a third.)

[zzheng]

Removed redefinition of BODY_LOC in test case runtime-loop1.ll

[evstupac]

lib/Transforms/Utils/LoopUnrollRuntime.cpp

651–652

Am I right that this will set more DebugLoc for epilog case than for prolog? If so, why?
If "TI" is supposed to be used only once I wouldn't create it at all:

NewExit->getTerminator()->setDebugLoc(
    Header->getTerminator()->getDebugLoc());

If not - reuse it in the line 654 and give wider name (like NewExitTerminator).
EpilogPreHeader = SplitBlock(NewExit, NewExitTerminator, DT, LI);

test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll

3

Please add a comment on what test is checking.

[zzheng]

lib/Transforms/Utils/LoopUnrollRuntime.cpp

651–652

NewExit gets its DebugLoc from LatchExit. This DebugLoc is not part of the original loop, it's after the original loop (say loc A).

The conditional branch deciding if epilog will be added to NewExit, which should be part of the original loop in lexical sense.

If a later pass search for debuginfo in RPOT order, it will first find loc A at the conditional branch rather than the merge point of unrolled and epilog loop.

On the other hand, PrologPreHeader gets its debug info from original PreHeader.

[zzheng]

Addressed Evgeny's comments.

[evstupac]

lib/Transforms/Utils/LoopUnrollRuntime.cpp

651–652

Thanks for detailed explanation. Now it is clear for me. I'd add this as a comment:
NewExit gets its DebugLoc from LatchExit, which do not belongs to the Loop.
Fix this by setting Loop DebugLoc to NewExit.

[danielcdh]

The change makes sense to me. It should not affect SamplePGO as terminator instructions is not used in profile annotation.

[zzheng]

Added comments as Evgeny suggested.

[zzheng]

Ping... I'd like to commit this soon if there's no further suggestion for revision.

[zzheng]

Committed as r321465.

lib/Transforms/Utils/LoopUnrollRuntime.cpp

if (UseEpilogRemainder) {
     // If epilog remainder
     // Split PreHeader to insert a branch around loop for unrolling.
     NewPreHeader = SplitBlock(PreHeader, PreHeader->getTerminator(), DT, LI);
     NewPreHeader->setName(PreHeader->getName() + ".new");
     // Split LatchExit to create phi nodes from branch above.
     SmallVector<BasicBlock*, 4> Preds(predecessors(LatchExit));
     NewExit = SplitBlockPredecessors(LatchExit, Preds, ".unr-lcssa",
                                      DT, LI, PreserveLCSSA);
+    // NewExit gets its DebugLoc from LatchExit, which is not part of the
+    // original Loop.
+    // Fix this by setting Loop's DebugLoc to NewExit.
+    auto *NewExitTerminator = NewExit->getTerminator();
+    NewExitTerminator->setDebugLoc(Header->getTerminator()->getDebugLoc());
     // Split NewExit to insert epilog remainder loop.
-    EpilogPreHeader = SplitBlock(NewExit, NewExit->getTerminator(), DT, LI);
+    EpilogPreHeader = SplitBlock(NewExit, NewExitTerminator, DT, LI);
     EpilogPreHeader->setName(Header->getName() + ".epil.preheader");
   } else {
     // If prolog remainder
     // Split the original preheader twice to insert prolog remainder loop
     PrologPreHeader = SplitEdge(PreHeader, Header, DT, LI);
     PrologPreHeader->setName(Header->getName() + ".prol.preheader");
     PrologExit = SplitBlock(PrologPreHeader, PrologPreHeader->getTerminator(),
                             DT, LI);

test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll

+; RUN: opt -loop-unroll -unroll-runtime -unroll-runtime-epilog -S %s | FileCheck %s
+
+; Test that epilogue is tagged with the same debug information as original loop body rather than original loop exit.
+
+; CHECK: for.body.i:
+; CHECK:   br i1 {{.*}}, label %lee1.exit.loopexit.unr-lcssa.loopexit, label %for.body.i, !dbg ![[LOOP_LOC:[0-9]+]]
+; CHECK: lee1.exit.loopexit.unr-lcssa.loopexit:
+; CHECK:   br label %lee1.exit.loopexit.unr-lcssa, !dbg ![[LOOP_LOC]]
+; CHECK: lee1.exit.loopexit.unr-lcssa:
+; CHECK:   %lcmp.mod = icmp ne i32 %xtraiter, 0, !dbg ![[LOOP_LOC]]
+; 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: lee1.exit.loopexit:
+; CHECK:   br label %lee1.exit, !dbg ![[EXIT_LOC:[0-9]+]]
+
+; CHECK-DAG: ![[LOOP_LOC]] = !DILocation(line: 5, column: 3, scope: !{{.*}}, inlinedAt: !{{.*}})
+; CHECK-DAG: ![[EXIT_LOC]] = !DILocation(line: 11, column: 12, scope: !{{.*}}, inlinedAt: !{{.*}})
+
+; Function Attrs: nounwind readnone
+define i32 @goo(i32 %a, i32 %b) local_unnamed_addr #0 !dbg !8 {
+entry:
+  tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !13, metadata !15), !dbg !16
+  tail call void @llvm.dbg.value(metadata i32 %b, i64 0, metadata !14, metadata !15), !dbg !17
+  tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !18, metadata !15), !dbg !26
+  tail call void @llvm.dbg.value(metadata i32 %b, i64 0, metadata !21, metadata !15), !dbg !28
+  tail call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !22, metadata !15), !dbg !29
+  tail call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !23, metadata !15), !dbg !30
+  tail call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !23, metadata !15), !dbg !30
+  tail call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !22, metadata !15), !dbg !29
+  %cmp7.i = icmp eq i32 %b, 0, !dbg !31
+  br i1 %cmp7.i, label %lee1.exit, label %for.body.i.preheader, !dbg !33
+
+for.body.i.preheader:                             ; preds = %entry
+  br label %for.body.i, !dbg !34
+
+for.body.i:                                       ; preds = %for.body.i.preheader, %for.body.i
+  %i.09.i = phi i32 [ %inc.i, %for.body.i ], [ 0, %for.body.i.preheader ]
+  %t.08.i = phi i32 [ %add1.i, %for.body.i ], [ 0, %for.body.i.preheader ]
+  %div.i = sdiv i32 %t.08.i, 2, !dbg !34
+  %add.i = add i32 %t.08.i, %a, !dbg !35
+  %add1.i = add i32 %add.i, %div.i, !dbg !36
+  tail call void @llvm.dbg.value(metadata i32 %add1.i, i64 0, metadata !22, metadata !15), !dbg !29
+  %inc.i = add nuw i32 %i.09.i, 1, !dbg !37
+  tail call void @llvm.dbg.value(metadata i32 %inc.i, i64 0, metadata !23, metadata !15), !dbg !30
+  tail call void @llvm.dbg.value(metadata i32 %inc.i, i64 0, metadata !23, metadata !15), !dbg !30
+  tail call void @llvm.dbg.value(metadata i32 %add1.i, i64 0, metadata !22, metadata !15), !dbg !29
+  %exitcond.i = icmp eq i32 %inc.i, %b, !dbg !31
+  br i1 %exitcond.i, label %lee1.exit.loopexit, label %for.body.i, !dbg !33, !llvm.loop !38
+
+lee1.exit.loopexit:                               ; preds = %for.body.i
+  %add1.i.lcssa = phi i32 [ %add1.i, %for.body.i ]
+  br label %lee1.exit, !dbg !41
+
+lee1.exit:                                        ; preds = %lee1.exit.loopexit, %entry
+  %t.0.lcssa.i = phi i32 [ 0, %entry ], [ %add1.i.lcssa, %lee1.exit.loopexit ]
+  tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !44, metadata !15), !dbg !47
+  tail call void @llvm.dbg.value(metadata i32 %b, i64 0, metadata !45, metadata !15), !dbg !48
+  %add.i4 = add nsw i32 %b, %a, !dbg !41
+  %sub.i = sub nsw i32 %a, %b, !dbg !49
+  %mul.i = mul nsw i32 %add.i4, %sub.i, !dbg !50
+  %add = add nsw i32 %t.0.lcssa.i, %mul.i, !dbg !51
+  ret i32 %add, !dbg !52
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.value(metadata, i64, metadata, metadata) #1
+
+attributes #0 = { nounwind readnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="arm7tdmi" "target-features"="+neon,+strict-align,+vfp3,-crypto,-d16,-fp-armv8,-fp-only-sp,-fp16,-vfp4" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "Snapdragon LLVM ARM Compiler 4.0.5 (based on llvm.org 4.0+)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "t.c", directory: "/prj/llvm-arm/scratch1/zhaoshiz/bugs/debug-symbol")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 1, !"min_enum_size", i32 4}
+!7 = !{!"Snapdragon LLVM ARM Compiler 4.0.5 (based on llvm.org 4.0+)"}
+!8 = distinct !DISubprogram(name: "goo", scope: !1, file: !1, line: 23, type: !9, isLocal: false, isDefinition: true, scopeLine: 23, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !12)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !11, !11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !{!13, !14}
+!13 = !DILocalVariable(name: "a", arg: 1, scope: !8, file: !1, line: 23, type: !11)
+!14 = !DILocalVariable(name: "b", arg: 2, scope: !8, file: !1, line: 23, type: !11)
+!15 = !DIExpression()
+!16 = !DILocation(line: 23, column: 14, scope: !8)
+!17 = !DILocation(line: 23, column: 21, scope: !8)
+!18 = !DILocalVariable(name: "a", arg: 1, scope: !19, file: !1, line: 3, type: !11)
+!19 = distinct !DISubprogram(name: "lee1", scope: !1, file: !1, line: 3, type: !9, isLocal: true, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !20)
+!20 = !{!18, !21, !22, !23}
+!21 = !DILocalVariable(name: "b", arg: 2, scope: !19, file: !1, line: 3, type: !11)
+!22 = !DILocalVariable(name: "t", scope: !19, file: !1, line: 4, type: !11)
+!23 = !DILocalVariable(name: "i", scope: !24, file: !1, line: 5, type: !25)
+!24 = distinct !DILexicalBlock(scope: !19, file: !1, line: 5, column: 3)
+!25 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+!26 = !DILocation(line: 3, column: 22, scope: !19, inlinedAt: !27)
+!27 = distinct !DILocation(line: 24, column: 27, scope: !8)
+!28 = !DILocation(line: 3, column: 29, scope: !19, inlinedAt: !27)
+!29 = !DILocation(line: 4, column: 7, scope: !19, inlinedAt: !27)
+!30 = !DILocation(line: 5, column: 17, scope: !24, inlinedAt: !27)
+!31 = !DILocation(line: 5, column: 23, scope: !32, inlinedAt: !27)
+!32 = distinct !DILexicalBlock(scope: !24, file: !1, line: 5, column: 3)
+!33 = !DILocation(line: 5, column: 3, scope: !24, inlinedAt: !27)
+!34 = !DILocation(line: 6, column: 13, scope: !32, inlinedAt: !27)
+!35 = !DILocation(line: 6, column: 11, scope: !32, inlinedAt: !27)
+!36 = !DILocation(line: 6, column: 7, scope: !32, inlinedAt: !27)
+!37 = !DILocation(line: 5, column: 28, scope: !32, inlinedAt: !27)
+!38 = distinct !{!38, !39, !40}
+!39 = !DILocation(line: 5, column: 3, scope: !24)
+!40 = !DILocation(line: 6, column: 14, scope: !24)
+!41 = !DILocation(line: 11, column: 12, scope: !42, inlinedAt: !46)
+!42 = distinct !DISubprogram(name: "lee2", scope: !1, file: !1, line: 10, type: !9, isLocal: true, isDefinition: true, scopeLine: 10, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !43)
+!43 = !{!44, !45}
+!44 = !DILocalVariable(name: "a", arg: 1, scope: !42, file: !1, line: 10, type: !11)
+!45 = !DILocalVariable(name: "b", arg: 2, scope: !42, file: !1, line: 10, type: !11)
+!46 = distinct !DILocation(line: 24, column: 40, scope: !8)
+!47 = !DILocation(line: 10, column: 22, scope: !42, inlinedAt: !46)
+!48 = !DILocation(line: 10, column: 29, scope: !42, inlinedAt: !46)
+!49 = !DILocation(line: 11, column: 20, scope: !42, inlinedAt: !46)
+!50 = !DILocation(line: 11, column: 16, scope: !42, inlinedAt: !46)
+!51 = !DILocation(line: 24, column: 38, scope: !8)
+!52 = !DILocation(line: 24, column: 3, scope: !8)

test/Transforms/LoopUnroll/runtime-loop1.ll

 ; RUN: opt < %s -S -loop-unroll -unroll-runtime -unroll-count=2 -unroll-runtime-epilog=true | FileCheck %s -check-prefix=EPILOG
 ; RUN: opt < %s -S -loop-unroll -unroll-runtime -unroll-count=2 -unroll-runtime-epilog=false | FileCheck %s -check-prefix=PROLOG
 
 ; RUN: opt < %s -S -passes='require<opt-remark-emit>,unroll' -unroll-runtime -unroll-count=2 -unroll-runtime-epilog=true | FileCheck %s -check-prefix=EPILOG
 ; RUN: opt < %s -S -passes='require<opt-remark-emit>,unroll' -unroll-runtime -unroll-count=2 -unroll-runtime-epilog=false | FileCheck %s -check-prefix=PROLOG
 
 ; This tests that setting the unroll count works
 
 
 ; EPILOG: for.body.preheader:
 ; EPILOG:   br i1 %1, label %for.end.loopexit.unr-lcssa, label %for.body.preheader.new, !dbg [[PH_LOC:![0-9]+]]
 ; EPILOG: for.body:
 ; EPILOG:   br i1 %niter.ncmp.1, label %for.end.loopexit.unr-lcssa.loopexit, label %for.body, !dbg [[BODY_LOC:![0-9]+]]
 ; EPILOG-NOT: br i1 %niter.ncmp.2, label %for.end.loopexit{{.*}}, label %for.body
 ; EPILOG: for.body.epil.preheader:
-; EPILOG:   br label %for.body.epil, !dbg [[EXIT_LOC:![0-9]+]]
+; EPILOG:   br label %for.body.epil, !dbg [[BODY_LOC]]
 ; EPILOG: for.body.epil:
-; EPILOG:   br label %for.end.loopexit.epilog-lcssa, !dbg [[BODY_LOC:![0-9]+]]
+; EPILOG:   br label %for.end.loopexit.epilog-lcssa, !dbg [[BODY_LOC]]
+; EPILOG: for.end.loopexit:
+; EPILOG:   br label %for.end, !dbg [[EXIT_LOC:![0-9]+]]
 
 ; EPILOG-DAG: [[PH_LOC]] = !DILocation(line: 101, column: 1, scope: !{{.*}})
 ; EPILOG-DAG: [[BODY_LOC]] = !DILocation(line: 102, column: 1, scope: !{{.*}})
 ; EPILOG-DAG: [[EXIT_LOC]] = !DILocation(line: 103, column: 1, scope: !{{.*}})
 
 ; PROLOG: for.body.preheader:
 ; PROLOG:   br {{.*}} label %for.body.prol.preheader, label %for.body.prol.loopexit, !dbg [[PH_LOC:![0-9]+]]
 ; PROLOG: for.body.prol: