[llvm] [DebugInfo][InstCombine] Do not overwrite prior DILocation for new Insts (PR #108565)
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 3 09:08:19 PDT 2024
https://github.com/SLTozer updated https://github.com/llvm/llvm-project/pull/108565
>From a92af4b385962d911fe4dc1d4a32735a616a0c3a Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Fri, 13 Sep 2024 14:47:57 +0100
Subject: [PATCH 1/2] [DebugInfo][InstCombine] Do not overwrite prior-set
DILocation for new Insts
When InstCombine replaces an old instruction with a new instruction, it
copies !dbg and !annotation metadata from old to new. For some InstCombine
patterns we set a specific DILocation on the new instruction prior to
insertion, however, which more accurately reflects the new instruction.
This more specific DILocation may be overwritten on insertion by a less
appropriate one, resulting in a less correct line mapping. This patch
changes this behaviour to only copy the DILocation from old to new if
the new instruction has no existing DILocation (which will always be the
case for a new instruction unless InstCombine has specifically set one).
---
.../InstCombine/InstructionCombining.cpp | 9 ++-
.../InstCombine/new-inst-dbgloc-overwrite.ll | 77 +++++++++++++++++++
2 files changed, 84 insertions(+), 2 deletions(-)
create mode 100644 llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 916a14e13ff2af..1ea3efb27937d2 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -5170,8 +5170,13 @@ bool InstCombinerImpl::run() {
LLVM_DEBUG(dbgs() << "IC: Old = " << *I << '\n'
<< " New = " << *Result << '\n');
- Result->copyMetadata(*I,
- {LLVMContext::MD_dbg, LLVMContext::MD_annotation});
+ // We copy the old instruction's DebugLoc to the new instruction, unless
+ // InstCombine already assigned a DebugLoc to it, in which case we
+ // should trust the more specifically selected DebugLoc.
+ if (!Result->getDebugLoc())
+ Result->setDebugLoc(I->getDebugLoc());
+ // We also copy annotation metadata to the new instruction.
+ Result->copyMetadata(*I, LLVMContext::MD_annotation);
// Everything uses the new instruction now.
I->replaceAllUsesWith(Result);
diff --git a/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll b/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
new file mode 100644
index 00000000000000..bd736cd3958973
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
@@ -0,0 +1,77 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals smart
+;; Tests that when InstCombine sets a DILocation on the new instruction when it
+;; is created, we do not try to overwrite that DILocation when we later insert
+;; the new instruction.
+;; In this test, InstCombine replaces two loads joined by a PHI into a PHI of
+;; the addresses followed by a load, and gives the new load a merge of the two
+;; incoming load DILocations. This test verifies that the new load keeps that
+;; DILocation, and doesn't have it overwritten by the DILocation of the original
+;; PHI. It will, however, receive the !annotation attached to the original PHI.
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+define void @test(ptr %xfA, ptr %xfB, i1 %cmp5) {
+; CHECK-LABEL: @test(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[CMP5:%.*]], label [[IF_ELSE:%.*]], label [[IF_THEN6:%.*]]
+; CHECK: if.then6:
+; CHECK-NEXT: br label [[IF_END11:%.*]]
+; CHECK: if.else:
+; CHECK-NEXT: br label [[IF_END11]]
+; CHECK: if.end11:
+; CHECK-NEXT: [[XFA_PN:%.*]] = phi ptr [ [[XFA:%.*]], [[IF_ELSE]] ], [ [[XFB:%.*]], [[IF_THEN6]] ]
+; CHECK-NEXT: [[XF1_SROA_8_0_IN:%.*]] = getelementptr i8, ptr [[XFA_PN]], i64 4
+; CHECK-NEXT: [[XF1_SROA_8_0:%.*]] = load float, ptr [[XF1_SROA_8_0_IN]], align 4, !dbg [[DBG3:![0-9]+]], !annotation [[META7:![0-9]+]]
+; CHECK-NEXT: [[CMP_I:%.*]] = fcmp ugt float [[XF1_SROA_8_0]], 0.000000e+00
+; CHECK-NEXT: br i1 [[CMP_I]], label [[IF_END_I:%.*]], label [[IF_THEN_I:%.*]]
+; CHECK: if.then.i:
+; CHECK-NEXT: br label [[IF_END_I]]
+; CHECK: if.end.i:
+; CHECK-NEXT: ret void
+;
+entry:
+ br i1 %cmp5, label %if.else, label %if.then6
+
+if.then6: ; preds = %entry
+ %xf1.sroa.8.0.xfB.sroa_idx = getelementptr i8, ptr %xfB, i64 4
+ %xf1.sroa.8.0.copyload = load float, ptr %xf1.sroa.8.0.xfB.sroa_idx, align 4, !dbg !3
+ br label %if.end11
+
+if.else: ; preds = %entry
+ %xf1.sroa.8.0.xfA.sroa_idx = getelementptr i8, ptr %xfA, i64 4
+ %xf1.sroa.8.0.copyload494 = load float, ptr %xf1.sroa.8.0.xfA.sroa_idx, align 4, !dbg !7
+ br label %if.end11
+
+if.end11: ; preds = %if.else, %if.then6
+ %xf1.sroa.8.0 = phi float [ %xf1.sroa.8.0.copyload494, %if.else ], [ %xf1.sroa.8.0.copyload, %if.then6 ], !dbg !8, !annotation !9
+ %cmp.i = fcmp ugt float %xf1.sroa.8.0, 0.000000e+00
+ br i1 %cmp.i, label %if.end.i, label %if.then.i
+
+if.then.i: ; preds = %if.end11
+ br label %if.end.i
+
+if.end.i: ; preds = %if.then.i, %if.end11
+ ret void
+}
+
+!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: FullDebug)
+!1 = !DIFile(filename: "test.cpp", directory: "/tmp")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !DILocation(line: 63, column: 12, scope: !4)
+!4 = distinct !DISubprogram(name: "operator=", linkageName: "_ZN11btMatrix3x3aSERKS_", scope: null, file: !1, line: 61, type: !5, scopeLine: 62, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !6)
+!5 = distinct !DISubroutineType(types: !6)
+!6 = !{}
+!7 = !DILocation(line: 63, column: 15, scope: !4)
+!8 = !DILocation(line: 64, scope: !4)
+!9 = !{!"Test String"}
+;.
+; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+; CHECK: [[META1]] = !DIFile(filename: "test.cpp", directory: {{.*}})
+; CHECK: [[DBG3]] = !DILocation(line: 63, scope: [[META4:![0-9]+]])
+; CHECK: [[META4]] = distinct !DISubprogram(name: "operator=", linkageName: "_ZN11btMatrix3x3aSERKS_", scope: null, file: [[META1]], line: 61, type: [[META5:![0-9]+]], scopeLine: 62, spFlags: DISPFlagDefinition, unit: [[META0]], retainedNodes: [[META6:![0-9]+]])
+; CHECK: [[META5]] = distinct !DISubroutineType(types: [[META6]])
+; CHECK: [[META6]] = !{}
+; CHECK: [[META7]] = !{!"Test String"}
+;.
>From 18604311478e33523bc81ca6417bbd53a7152c10 Mon Sep 17 00:00:00 2001
From: Stephen Tozer <stephen.tozer at sony.com>
Date: Thu, 3 Oct 2024 17:07:29 +0100
Subject: [PATCH 2/2] Change line numbers in test
---
.../Transforms/InstCombine/new-inst-dbgloc-overwrite.ll | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll b/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
index bd736cd3958973..2b933a2f9e0756 100644
--- a/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
+++ b/llvm/test/Transforms/InstCombine/new-inst-dbgloc-overwrite.ll
@@ -59,17 +59,17 @@ if.end.i: ; preds = %if.then.i, %if.end1
!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 20.0.0git", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
!1 = !DIFile(filename: "test.cpp", directory: "/tmp")
!2 = !{i32 2, !"Debug Info Version", i32 3}
-!3 = !DILocation(line: 63, column: 12, scope: !4)
+!3 = !DILocation(line: 13, column: 12, scope: !4)
!4 = distinct !DISubprogram(name: "operator=", linkageName: "_ZN11btMatrix3x3aSERKS_", scope: null, file: !1, line: 61, type: !5, scopeLine: 62, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !6)
!5 = distinct !DISubroutineType(types: !6)
!6 = !{}
-!7 = !DILocation(line: 63, column: 15, scope: !4)
-!8 = !DILocation(line: 64, scope: !4)
+!7 = !DILocation(line: 13, column: 15, scope: !4)
+!8 = !DILocation(line: 100, scope: !4)
!9 = !{!"Test String"}
;.
; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
; CHECK: [[META1]] = !DIFile(filename: "test.cpp", directory: {{.*}})
-; CHECK: [[DBG3]] = !DILocation(line: 63, scope: [[META4:![0-9]+]])
+; CHECK: [[DBG3]] = !DILocation(line: 13, scope: [[META4:![0-9]+]])
; CHECK: [[META4]] = distinct !DISubprogram(name: "operator=", linkageName: "_ZN11btMatrix3x3aSERKS_", scope: null, file: [[META1]], line: 61, type: [[META5:![0-9]+]], scopeLine: 62, spFlags: DISPFlagDefinition, unit: [[META0]], retainedNodes: [[META6:![0-9]+]])
; CHECK: [[META5]] = distinct !DISubroutineType(types: [[META6]])
; CHECK: [[META6]] = !{}
More information about the llvm-commits
mailing list