[llvm] [BranchFolder] Fix missing debug info with tail merging (PR #94715)
Alan Zhao via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 17 11:08:28 PDT 2024
https://github.com/alanzhao1 updated https://github.com/llvm/llvm-project/pull/94715
>From 5cd3c889a2e7d87392227f6422d03db14aa0b8c3 Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Thu, 6 Jun 2024 16:52:17 -0700
Subject: [PATCH 1/3] [CodeGen] Fix missing debug info with tail merging
`BranchFolder::TryTailMergeBlocks(...)` removes unconditional branch
instructions and then recreates them. However, this process loses debug
source location information from the previous branch instruction, even
if tail merging doesn't change IR. This patch preserves the debug
information from the removed instruction and inserts them into the
recreated instruction.
Fixes #94050
---
llvm/lib/CodeGen/BranchFolding.cpp | 25 +++++++++------
llvm/lib/CodeGen/BranchFolding.h | 12 ++++---
.../X86/tail-merging-preserve-debugloc.ll | 31 +++++++++++++++++++
llvm/test/DebugInfo/COFF/local-variables.ll | 8 +++--
4 files changed, 60 insertions(+), 16 deletions(-)
create mode 100644 llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 55aa1d438b2a6..c6c48cfc320c9 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -455,12 +455,14 @@ static unsigned EstimateRuntime(MachineBasicBlock::iterator I,
// with a conditional branch to the next block, optimize by reversing the
// test and conditionally branching to SuccMBB instead.
static void FixTail(MachineBasicBlock *CurMBB, MachineBasicBlock *SuccBB,
- const TargetInstrInfo *TII) {
+ const TargetInstrInfo *TII, const DebugLoc &BranchDL) {
MachineFunction *MF = CurMBB->getParent();
MachineFunction::iterator I = std::next(MachineFunction::iterator(CurMBB));
MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
SmallVector<MachineOperand, 4> Cond;
DebugLoc dl = CurMBB->findBranchDebugLoc();
+ if (!dl)
+ dl = BranchDL;
if (I != MF->end() && !TII->analyzeBranch(*CurMBB, TBB, FBB, Cond, true)) {
MachineBasicBlock *NextBB = &*I;
if (TBB == NextBB && !Cond.empty() && !FBB) {
@@ -686,7 +688,8 @@ unsigned BranchFolder::ComputeSameTails(unsigned CurHash,
void BranchFolder::RemoveBlocksWithHash(unsigned CurHash,
MachineBasicBlock *SuccBB,
- MachineBasicBlock *PredBB) {
+ MachineBasicBlock *PredBB,
+ const DebugLoc &BranchDL) {
MPIterator CurMPIter, B;
for (CurMPIter = std::prev(MergePotentials.end()),
B = MergePotentials.begin();
@@ -694,7 +697,7 @@ void BranchFolder::RemoveBlocksWithHash(unsigned CurHash,
// Put the unconditional branch back, if we need one.
MachineBasicBlock *CurMBB = CurMPIter->getBlock();
if (SuccBB && CurMBB != PredBB)
- FixTail(CurMBB, SuccBB, TII);
+ FixTail(CurMBB, SuccBB, TII, BranchDL);
if (CurMPIter == B)
break;
}
@@ -908,6 +911,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
// Walk through equivalence sets looking for actual exact matches.
while (MergePotentials.size() > 1) {
unsigned CurHash = MergePotentials.back().getHash();
+ const DebugLoc &BranchDL = MergePotentials.back().getBranchDebugLoc();
// Build SameTails, identifying the set of blocks with this hash code
// and with the maximum number of instructions in common.
@@ -918,7 +922,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
// If we didn't find any pair that has at least MinCommonTailLength
// instructions in common, remove all blocks with this hash code and retry.
if (SameTails.empty()) {
- RemoveBlocksWithHash(CurHash, SuccBB, PredBB);
+ RemoveBlocksWithHash(CurHash, SuccBB, PredBB, BranchDL);
continue;
}
@@ -965,7 +969,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
// Split a block so that one does.
if (!CreateCommonTailOnlyBlock(PredBB, SuccBB,
maxCommonTailLength, commonTailIndex)) {
- RemoveBlocksWithHash(CurHash, SuccBB, PredBB);
+ RemoveBlocksWithHash(CurHash, SuccBB, PredBB, BranchDL);
continue;
}
}
@@ -1013,7 +1017,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
if (MergePotentials.size() == TailMergeThreshold)
break;
if (!TriedMerging.count(&MBB) && MBB.succ_empty())
- MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB));
+ MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB,
+ MBB.findBranchDebugLoc()));
}
// If this is a large problem, avoid visiting the same basic blocks
@@ -1115,8 +1120,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
}
// Remove the unconditional branch at the end, if any.
+ DebugLoc dl = PBB->findBranchDebugLoc();
if (TBB && (Cond.empty() || FBB)) {
- DebugLoc dl = PBB->findBranchDebugLoc();
TII->removeBranch(*PBB);
if (!Cond.empty())
// reinsert conditional branch only, for now
@@ -1124,7 +1129,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
NewCond, dl);
}
- MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(*PBB), PBB));
+ MergePotentials.push_back(
+ MergePotentialsElt(HashEndOfMBB(*PBB), PBB, dl));
}
}
@@ -1142,7 +1148,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
PredBB = &*std::prev(I); // this may have been changed in TryTailMergeBlocks
if (MergePotentials.size() == 1 &&
MergePotentials.begin()->getBlock() != PredBB)
- FixTail(MergePotentials.begin()->getBlock(), IBB, TII);
+ FixTail(MergePotentials.begin()->getBlock(), IBB, TII,
+ MergePotentials.begin()->getBranchDebugLoc());
}
return MadeChange;
diff --git a/llvm/lib/CodeGen/BranchFolding.h b/llvm/lib/CodeGen/BranchFolding.h
index 63b2ef04b21ba..ff2bbe06c0488 100644
--- a/llvm/lib/CodeGen/BranchFolding.h
+++ b/llvm/lib/CodeGen/BranchFolding.h
@@ -50,10 +50,11 @@ class TargetRegisterInfo;
class MergePotentialsElt {
unsigned Hash;
MachineBasicBlock *Block;
+ DebugLoc BranchDebugLoc;
public:
- MergePotentialsElt(unsigned h, MachineBasicBlock *b)
- : Hash(h), Block(b) {}
+ MergePotentialsElt(unsigned h, MachineBasicBlock *b, DebugLoc bdl)
+ : Hash(h), Block(b), BranchDebugLoc(std::move(bdl)) {}
unsigned getHash() const { return Hash; }
MachineBasicBlock *getBlock() const { return Block; }
@@ -62,6 +63,8 @@ class TargetRegisterInfo;
Block = MBB;
}
+ const DebugLoc &getBranchDebugLoc() { return BranchDebugLoc; }
+
bool operator<(const MergePotentialsElt &) const;
};
@@ -162,8 +165,9 @@ class TargetRegisterInfo;
/// Remove all blocks with hash CurHash from MergePotentials, restoring
/// branches at ends of blocks as appropriate.
- void RemoveBlocksWithHash(unsigned CurHash, MachineBasicBlock* SuccBB,
- MachineBasicBlock* PredBB);
+ void RemoveBlocksWithHash(unsigned CurHash, MachineBasicBlock *SuccBB,
+ MachineBasicBlock *PredBB,
+ const DebugLoc &BranchDL);
/// None of the blocks to be tail-merged consist only of the common tail.
/// Create a block that does by splitting one.
diff --git a/llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll b/llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll
new file mode 100644
index 0000000000000..d8124d8f5960a
--- /dev/null
+++ b/llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -enable-tail-merge=1 -stop-after=branch-folder | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-grtev4-linux-gnu"
+
+define i32 @main(i1 %0) {
+entry:
+ br i1 %0, label %1, label %2
+
+1: ; preds = %entry
+ store i64 1, ptr null, align 1
+; CHECK: JMP_1 %bb.3, debug-location !3
+ br label %3, !dbg !3
+
+2: ; preds = %entry
+ store i64 0, ptr null, align 1
+ br label %3
+
+3: ; preds = %2, %1
+ ret i32 0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
+!1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !DILocation(line: 17, column: 3, scope: !4)
+!4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!5 = !DISubroutineType(types: !6)
+!6 = !{}
diff --git a/llvm/test/DebugInfo/COFF/local-variables.ll b/llvm/test/DebugInfo/COFF/local-variables.ll
index 56ece6b794b4b..820f6bd8beaa4 100644
--- a/llvm/test/DebugInfo/COFF/local-variables.ll
+++ b/llvm/test/DebugInfo/COFF/local-variables.ll
@@ -45,6 +45,8 @@
; ASM: .cv_loc 1 1 5 3 # t.cpp:5:3
; ASM: callq capture
; ASM: leaq 40(%rsp), %rcx
+; ASM: [[end_inline_1:\.Ltmp.*]]:
+; ASM: .cv_loc 0 1 11 5 # t.cpp:11:5
; ASM: jmp .LBB0_3
; ASM: [[else_start:\.Ltmp.*]]:
; ASM: .LBB0_2: # %if.else
@@ -87,7 +89,7 @@
; ASM: .long 116 # TypeIndex
; ASM: .short 0 # Flags
; ASM: .asciz "v"
-; ASM: .cv_def_range [[inline_site1]] [[else_start]], frame_ptr_rel, 44
+; ASM: .cv_def_range [[inline_site1]] [[end_inline_1]], frame_ptr_rel, 44
; ASM: .short 4430 # Record kind: S_INLINESITE_END
; ASM: .short 4429 # Record kind: S_INLINESITE
; ASM: .short 4414 # Record kind: S_LOCAL
@@ -154,7 +156,7 @@
; OBJ: ChangeLineOffset: 1
; OBJ: ChangeCodeOffset: 0x14
; OBJ: ChangeCodeOffsetAndLineOffset: {CodeOffset: 0xD, LineOffset: 1}
-; OBJ: ChangeCodeLength: 0xC
+; OBJ: ChangeCodeLength: 0xA
; OBJ: ]
; OBJ: }
; OBJ: LocalSym {
@@ -168,7 +170,7 @@
; OBJ: LocalVariableAddrRange {
; OBJ: OffsetStart: .text+0x14
; OBJ: ISectStart: 0x0
-; OBJ: Range: 0x19
+; OBJ: Range: 0x17
; OBJ: }
; OBJ: }
; OBJ: InlineSiteEnd {
>From d34b458a5c26663dfc46834d89f8563ce0d33ed5 Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Mon, 17 Jun 2024 10:56:39 -0700
Subject: [PATCH 2/3] use mir output for test
---
.../MIR/X86/branch-folder-with-label.mir | 2 +-
.../X86/tail-merging-preserve-debugloc.mir | 149 ++++++++++++++++++
.../X86/tail-merging-preserve-debugloc.ll | 31 ----
3 files changed, 150 insertions(+), 32 deletions(-)
create mode 100644 llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir
delete mode 100644 llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll
diff --git a/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir b/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir
index ce225d4567e91..235b7a1482d4c 100644
--- a/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir
+++ b/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir
@@ -1,5 +1,5 @@
# Generated with
-#
+# foobar
# clang -g -O1 -S -emit-llvm test.c
# llc -stop-before=branch-folder test.ll
#
diff --git a/llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir b/llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir
new file mode 100644
index 0000000000000..08b353a58c19e
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir
@@ -0,0 +1,149 @@
+# Generated with
+#
+# bin/llc -stop-before=branch-folder test.ll
+#
+# target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+# target triple = "x86_64-grtev4-linux-gnu"
+#
+# define i32 @main(i1 %0) {
+# entry:
+# br i1 %0, label %1, label %2
+#
+# 1: ; preds = %entry
+# store i64 1, ptr null, align 1
+# br label %3, !dbg !3
+#
+# 2: ; preds = %entry
+# store i64 0, ptr null, align 1
+# br label %3
+#
+# 3: ; preds = %2, %1
+# ret i32 0
+# }
+#
+# !llvm.dbg.cu = !{!0}
+# !llvm.module.flags = !{!2}
+#
+# !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
+# !1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
+# !2 = !{i32 2, !"Debug Info Version", i32 3}
+# !3 = !DILocation(line: 17, column: 3, scope: !4)
+# !4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+# !5 = !DISubroutineType(types: !6)
+# !6 = !{}
+#
+# RUN: llc -o - %s -mtriple=x86_64-unknown-linux-gnu --run-pass=branch-folder -enable-tail-merge | FileCheck %s
+--- |
+
+ ; ModuleID = '../llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll'
+ source_filename = "../llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll"
+ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+ target triple = "x86_64-grtev4-linux-gnu"
+
+ define i32 @main(i1 %0) {
+ entry:
+ br i1 %0, label %1, label %2
+
+ 1: ; preds = %entry
+ store i64 1, ptr null, align 1
+ br label %3, !dbg !3
+
+ 2: ; preds = %entry
+ store i64 0, ptr null, align 1
+ br label %3
+
+ 3: ; preds = %2, %1
+ ret i32 0
+ }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
+ !1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
+ !2 = !{i32 2, !"Debug Info Version", i32 3}
+ !3 = !DILocation(line: 17, column: 3, scope: !4)
+ !4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+ !5 = !DISubroutineType(types: !6)
+ !6 = !{}
+
+...
+---
+name: main
+alignment: 16
+exposesReturnsTwice: false
+legalized: false
+regBankSelected: false
+selected: false
+failedISel: false
+tracksRegLiveness: true
+hasWinCFI: false
+callsEHReturn: false
+callsUnwindInit: false
+hasEHCatchret: false
+hasEHScopes: false
+hasEHFunclets: false
+isOutlined: false
+debugInstrRef: true
+failsVerification: false
+tracksDebugUserValues: true
+registers: []
+liveins:
+ - { reg: '$edi', virtual-reg: '' }
+frameInfo:
+ isFrameAddressTaken: false
+ isReturnAddressTaken: false
+ hasStackMap: false
+ hasPatchPoint: false
+ stackSize: 0
+ offsetAdjustment: 0
+ maxAlignment: 1
+ adjustsStack: false
+ hasCalls: false
+ stackProtector: ''
+ functionContext: ''
+ maxCallFrameSize: 0
+ cvBytesOfCalleeSavedRegisters: 0
+ hasOpaqueSPAdjustment: false
+ hasVAStart: false
+ hasMustTailInVarArgFunc: false
+ hasTailCall: false
+ isCalleeSavedInfoValid: true
+ localFrameSize: 0
+ savePoint: ''
+ restorePoint: ''
+fixedStack: []
+stack: []
+entry_values: []
+callSites: []
+debugValueSubstitutions: []
+constants: []
+machineFunctionInfo:
+ amxProgModel: None
+body: |
+ bb.0.entry:
+ successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ liveins: $edi
+
+ TEST8ri renamable $dil, 1, implicit-def $eflags, implicit killed $edi
+ JCC_1 %bb.2, 4, implicit killed $eflags
+ JMP_1 %bb.1
+
+ bb.1 (%ir-block.1):
+ successors: %bb.3(0x80000000)
+
+ MOV64mi32 $noreg, 1, $noreg, 0, $noreg, 1 :: (store (s64) into `ptr null`, align 1)
+ JMP_1 %bb.3, debug-location !3
+
+ bb.2 (%ir-block.2):
+ successors: %bb.3(0x80000000)
+
+ MOV64mi32 $noreg, 1, $noreg, 0, $noreg, 0 :: (store (s64) into `ptr null`, align 1)
+
+ bb.3 (%ir-block.3):
+ $eax = MOV32r0 implicit-def dead $eflags
+ RET 0, killed $eax
+
+...
+# CHECK: [[DEBUGNUM:!.*]] = !DILocation(line: 17, column: 3, scope: !4)
+# CHECK: JMP_1 %bb.3, debug-location [[DEBUGNUM]]
diff --git a/llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll b/llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll
deleted file mode 100644
index d8124d8f5960a..0000000000000
--- a/llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll
+++ /dev/null
@@ -1,31 +0,0 @@
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -enable-tail-merge=1 -stop-after=branch-folder | FileCheck %s
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-grtev4-linux-gnu"
-
-define i32 @main(i1 %0) {
-entry:
- br i1 %0, label %1, label %2
-
-1: ; preds = %entry
- store i64 1, ptr null, align 1
-; CHECK: JMP_1 %bb.3, debug-location !3
- br label %3, !dbg !3
-
-2: ; preds = %entry
- store i64 0, ptr null, align 1
- br label %3
-
-3: ; preds = %2, %1
- ret i32 0
-}
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!2}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
-!1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
-!2 = !{i32 2, !"Debug Info Version", i32 3}
-!3 = !DILocation(line: 17, column: 3, scope: !4)
-!4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
-!5 = !DISubroutineType(types: !6)
-!6 = !{}
>From 7fafd01ff4d4190cf9950006424cfa52ab876fe5 Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Mon, 17 Jun 2024 11:08:13 -0700
Subject: [PATCH 3/3] undo unintentional file change
---
llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir b/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir
index 235b7a1482d4c..ce225d4567e91 100644
--- a/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir
+++ b/llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir
@@ -1,5 +1,5 @@
# Generated with
-# foobar
+#
# clang -g -O1 -S -emit-llvm test.c
# llc -stop-before=branch-folder test.ll
#
More information about the llvm-commits
mailing list