[DebugInfo][LoopUnroll] Preserve DebugLocs on optimized cond branches

[SLTozer]

This patch fixes a simple error where as part of loop unrolling we optimize conditional loop-exiting branches into unconditional branches when we know that they will or won't exit the loop, but does not propagate the source location of the original branch to the new one.

Found using #107279.

[llvmbot]

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-transforms

Author: Stephen Tozer (SLTozer)

Changes

This patch fixes a simple error where as part of loop unrolling we optimize conditional loop-exiting branches into unconditional branches when we know that they will or won't exit the loop, but does not propagate the source location of the original branch to the new one.

---

Full diff: https://github.com/llvm/llvm-project/pull/110276.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/LoopUnroll.cpp (+2-1)
• (added) llvm/test/Transforms/LoopUnroll/preserve-branch-debuglocs.ll (+52)

diff --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
index b90addcef69e6e..4cbbac07915fbe 100644
--- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -883,7 +883,8 @@ llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
     DeadSucc->removePredecessor(Src, /* KeepOneInputPHIs */ true);
 
     // Replace the conditional branch with an unconditional one.
-    BranchInst::Create(Dest, Term->getIterator());
+    auto *BI = BranchInst::Create(Dest, Term->getIterator());
+    BI->setDebugLoc(Term->getDebugLoc());
     Term->eraseFromParent();
 
     DTUpdates.emplace_back(DominatorTree::Delete, Src, DeadSucc);
diff --git a/llvm/test/Transforms/LoopUnroll/preserve-branch-debuglocs.ll b/llvm/test/Transforms/LoopUnroll/preserve-branch-debuglocs.ll
new file mode 100644
index 00000000000000..623fb68c784fe7
--- /dev/null
+++ b/llvm/test/Transforms/LoopUnroll/preserve-branch-debuglocs.ll
@@ -0,0 +1,52 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --version 5
+; RUN: opt < %s -passes=loop-unroll -S | FileCheck %s
+; RUN: opt < %s -passes=loop-unroll-full -S | FileCheck %s
+
+;; Loop Unrolling should preserve the DebugLocs of conditional branches that get
+;; optimized into unconditional branches.
+
+define i32 @_ZeqRK4int3S1_() {
+; CHECK-LABEL: define i32 @_ZeqRK4int3S1_() {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br label %[[FOR_BODY:.*]]
+; CHECK:       [[FOR_COND:.*]]:
+; CHECK-NEXT:    br label %[[CLEANUP:.*]], !dbg [[DBG3:![0-9]+]]
+; CHECK:       [[FOR_BODY]]:
+; CHECK-NEXT:    br i1 false, label %[[FOR_COND]], label %[[CLEANUP]]
+; CHECK:       [[CLEANUP]]:
+; CHECK-NEXT:    ret i32 0
+;
+entry:
+  br label %for.body
+
+for.cond:                                         ; preds = %for.body
+  br i1 false, label %cleanup, label %for.body, !dbg !3
+
+for.body:                                         ; preds = %for.cond, %entry
+  br i1 false, label %for.cond, label %cleanup
+
+cleanup:                                          ; preds = %for.body, %for.cond
+  ret i32 0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 20.0.0git", isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug)
+!1 = !DIFile(filename: "test.cpp", directory: "/tmp")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !DILocation(line: 304, column: 2, scope: !4)
+!4 = distinct !DILexicalBlock(scope: !5, file: !1, line: 304, column: 2)
+!5 = distinct !DISubprogram(name: "operator==", linkageName: "_ZeqRK4int3S1_", scope: !1, file: !1, line: 302, type: !6, scopeLine: 303, spFlags: DISPFlagDefinition, unit: !0)
+!6 = distinct !DISubroutineType(types: !7)
+!7 = !{}
+;.
+; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug)
+; CHECK: [[META1]] = !DIFile(filename: "test.cpp", directory: {{.*}})
+; CHECK: [[META2:![0-9]+]] = !{i32 2, !"Debug Info Version", i32 3}
+; CHECK: [[DBG3]] = !DILocation(line: 304, column: 2, scope: [[META4:![0-9]+]])
+; CHECK: [[META4]] = distinct !DILexicalBlock(scope: [[META5:![0-9]+]], file: [[META1]], line: 304, column: 2)
+; CHECK: [[META5]] = distinct !DISubprogram(name: "operator==", linkageName: "_ZeqRK4int3S1_", scope: [[META1]], file: [[META1]], line: 302, type: [[META6:![0-9]+]], scopeLine: 303, spFlags: DISPFlagDefinition, unit: [[META0]])
+; CHECK: [[META6]] = distinct !DISubroutineType(types: [[META7:![0-9]+]])
+; CHECK: [[META7]] = !{}
+;.

[dwblaikie]

感謝補丁的提交！保留優化條件分支中的 DebugLocs 對於調試信息的完整性至關重要。我認為這個修復將極大改善循環展開過程中調試信息的跟蹤。

不過，我有一個小建議：在無條件分支上保留原始條件分支的 DebugLocs 是正確的，但也許我們可以考慮在生成這些優化時增加更多調試日誌或註釋，以便開發者更直觀地理解這一優化過程。如果有可能，是否可以在文檔中也補充一些關於此修復對調試工具鏈影響的描述？

總體來說，這個補丁在保持源代碼可追蹤性方面非常有幫助，期待看到它的更多應用！

Thanks for contributing your feedback on this patch! Though the LLVM community uses English in shared spaces like this (there's some regional/language-specific parts of the LLVM discourse, though!) - could you use English here in the future?

Here's Google Translate's attempt at translating the above:

Thanks for submitting the patch! Preserving DebugLocs in optimization condition branches is critical for the integrity of debugging information. I think this fix will greatly improve the tracing of debug information during loop unrolling.

However, I have a small suggestion: it is correct to keep the DebugLocs of the original conditional branch on the unconditional branch, but maybe we can consider adding more debug logs or comments when generating these optimizations so that developers can understand this optimization more intuitively process. If possible, could you also add some description in the documentation about the impact of this fix on the debugging toolchain?

Overall, this patch has been very helpful in maintaining source code traceability, and I look forward to seeing more applications of it!

[jmorse]

LGTM

[dwblaikie]

(did this get merged? The PR just shows "closed" - be good to mention which commit merged this, if it was merged, or why it wasn't if it wasn't)

[SLTozer]

Merged in 92e0fb0.