Skip to content

Commit 8367030

Browse files
alanzhao1alanzhao1
authored
[BranchFolder] Fix missing debug info with tail merging (#94715)
`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
1 parent 5e9f247 commit 8367030

File tree

4 files changed

+128
-16
lines changed

4 files changed

+128
-16
lines changed

‎llvm/lib/CodeGen/BranchFolding.cpp‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -455,12 +455,14 @@ static unsigned EstimateRuntime(MachineBasicBlock::iterator I,
455455
// with a conditional branch to the next block, optimize by reversing the
456456
// test and conditionally branching to SuccMBB instead.
457457
static void FixTail(MachineBasicBlock *CurMBB, MachineBasicBlock *SuccBB,
458-
const TargetInstrInfo *TII) {
458+
const TargetInstrInfo *TII, const DebugLoc &BranchDL) {
459459
MachineFunction *MF = CurMBB->getParent();
460460
MachineFunction::iterator I = std::next(MachineFunction::iterator(CurMBB));
461461
MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
462462
SmallVector<MachineOperand, 4> Cond;
463463
DebugLoc dl = CurMBB->findBranchDebugLoc();
464+
if (!dl)
465+
dl = BranchDL;
464466
if (I != MF->end() && !TII->analyzeBranch(*CurMBB, TBB, FBB, Cond, true)) {
465467
MachineBasicBlock *NextBB = &*I;
466468
if (TBB == NextBB && !Cond.empty() && !FBB) {
@@ -686,15 +688,16 @@ unsigned BranchFolder::ComputeSameTails(unsigned CurHash,
686688

687689
void BranchFolder::RemoveBlocksWithHash(unsigned CurHash,
688690
MachineBasicBlock *SuccBB,
689-
MachineBasicBlock *PredBB) {
691+
MachineBasicBlock *PredBB,
692+
const DebugLoc &BranchDL) {
690693
MPIterator CurMPIter, B;
691694
for (CurMPIter = std::prev(MergePotentials.end()),
692695
B = MergePotentials.begin();
693696
CurMPIter->getHash() == CurHash; --CurMPIter) {
694697
// Put the unconditional branch back, if we need one.
695698
MachineBasicBlock *CurMBB = CurMPIter->getBlock();
696699
if (SuccBB && CurMBB != PredBB)
697-
FixTail(CurMBB, SuccBB, TII);
700+
FixTail(CurMBB, SuccBB, TII, BranchDL);
698701
if (CurMPIter == B)
699702
break;
700703
}
@@ -908,6 +911,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
908911
// Walk through equivalence sets looking for actual exact matches.
909912
while (MergePotentials.size() > 1) {
910913
unsigned CurHash = MergePotentials.back().getHash();
914+
const DebugLoc &BranchDL = MergePotentials.back().getBranchDebugLoc();
911915

912916
// Build SameTails, identifying the set of blocks with this hash code
913917
// and with the maximum number of instructions in common.
@@ -918,7 +922,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
918922
// If we didn't find any pair that has at least MinCommonTailLength
919923
// instructions in common, remove all blocks with this hash code and retry.
920924
if (SameTails.empty()) {
921-
RemoveBlocksWithHash(CurHash, SuccBB, PredBB);
925+
RemoveBlocksWithHash(CurHash, SuccBB, PredBB, BranchDL);
922926
continue;
923927
}
924928

@@ -965,7 +969,7 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
965969
// Split a block so that one does.
966970
if (!CreateCommonTailOnlyBlock(PredBB, SuccBB,
967971
maxCommonTailLength, commonTailIndex)) {
968-
RemoveBlocksWithHash(CurHash, SuccBB, PredBB);
972+
RemoveBlocksWithHash(CurHash, SuccBB, PredBB, BranchDL);
969973
continue;
970974
}
971975
}
@@ -1013,7 +1017,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
10131017
if (MergePotentials.size() == TailMergeThreshold)
10141018
break;
10151019
if (!TriedMerging.count(&MBB) && MBB.succ_empty())
1016-
MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB));
1020+
MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(MBB), &MBB,
1021+
MBB.findBranchDebugLoc()));
10171022
}
10181023

10191024
// If this is a large problem, avoid visiting the same basic blocks
@@ -1115,16 +1120,17 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
11151120
}
11161121

11171122
// Remove the unconditional branch at the end, if any.
1123+
DebugLoc dl = PBB->findBranchDebugLoc();
11181124
if (TBB && (Cond.empty() || FBB)) {
1119-
DebugLoc dl = PBB->findBranchDebugLoc();
11201125
TII->removeBranch(*PBB);
11211126
if (!Cond.empty())
11221127
// reinsert conditional branch only, for now
11231128
TII->insertBranch(*PBB, (TBB == IBB) ? FBB : TBB, nullptr,
11241129
NewCond, dl);
11251130
}
11261131

1127-
MergePotentials.push_back(MergePotentialsElt(HashEndOfMBB(*PBB), PBB));
1132+
MergePotentials.push_back(
1133+
MergePotentialsElt(HashEndOfMBB(*PBB), PBB, dl));
11281134
}
11291135
}
11301136

@@ -1142,7 +1148,8 @@ bool BranchFolder::TailMergeBlocks(MachineFunction &MF) {
11421148
PredBB = &*std::prev(I); // this may have been changed in TryTailMergeBlocks
11431149
if (MergePotentials.size() == 1 &&
11441150
MergePotentials.begin()->getBlock() != PredBB)
1145-
FixTail(MergePotentials.begin()->getBlock(), IBB, TII);
1151+
FixTail(MergePotentials.begin()->getBlock(), IBB, TII,
1152+
MergePotentials.begin()->getBranchDebugLoc());
11461153
}
11471154

11481155
return MadeChange;

‎llvm/lib/CodeGen/BranchFolding.h‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,11 @@ class TargetRegisterInfo;
5050
class MergePotentialsElt {
5151
unsigned Hash;
5252
MachineBasicBlock *Block;
53+
DebugLoc BranchDebugLoc;
5354

5455
public:
55-
MergePotentialsElt(unsigned h, MachineBasicBlock *b)
56-
: Hash(h), Block(b) {}
56+
MergePotentialsElt(unsigned h, MachineBasicBlock *b, DebugLoc bdl)
57+
: Hash(h), Block(b), BranchDebugLoc(std::move(bdl)) {}
5758

5859
unsigned getHash() const { return Hash; }
5960
MachineBasicBlock *getBlock() const { return Block; }
@@ -62,6 +63,8 @@ class TargetRegisterInfo;
6263
Block = MBB;
6364
}
6465

66+
const DebugLoc &getBranchDebugLoc() { return BranchDebugLoc; }
67+
6568
bool operator<(const MergePotentialsElt &) const;
6669
};
6770

@@ -162,8 +165,9 @@ class TargetRegisterInfo;
162165

163166
/// Remove all blocks with hash CurHash from MergePotentials, restoring
164167
/// branches at ends of blocks as appropriate.
165-
void RemoveBlocksWithHash(unsigned CurHash, MachineBasicBlock* SuccBB,
166-
MachineBasicBlock* PredBB);
168+
void RemoveBlocksWithHash(unsigned CurHash, MachineBasicBlock *SuccBB,
169+
MachineBasicBlock *PredBB,
170+
const DebugLoc &BranchDL);
167171

168172
/// None of the blocks to be tail-merged consist only of the common tail.
169173
/// Create a block that does by splitting one.

‎llvm/test/CodeGen/MIR/X86/tail-merging-preserve-debugloc.mir‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# RUN: llc -o - %s -mtriple=x86_64-unknown-linux-gnu --run-pass=branch-folder -enable-tail-merge | FileCheck %s
2+
#
3+
# Generated with
4+
#
5+
# bin/llc -stop-before=branch-folder test.ll
6+
#
7+
# 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"
8+
# target triple = "x86_64-grtev4-linux-gnu"
9+
#
10+
# define i32 @main(i1 %0) {
11+
# entry:
12+
# br i1 %0, label %1, label %2
13+
#
14+
# 1: ; preds = %entry
15+
# store i64 1, ptr null, align 1
16+
# br label %3, !dbg !3
17+
#
18+
# 2: ; preds = %entry
19+
# store i64 0, ptr null, align 1
20+
# br label %3
21+
#
22+
# 3: ; preds = %2, %1
23+
# ret i32 0
24+
# }
25+
#
26+
# !llvm.dbg.cu = !{!0}
27+
# !llvm.module.flags = !{!2}
28+
#
29+
# !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
30+
# !1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
31+
# !2 = !{i32 2, !"Debug Info Version", i32 3}
32+
# !3 = !DILocation(line: 17, column: 3, scope: !4)
33+
# !4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
34+
# !5 = !DISubroutineType(types: !6)
35+
# !6 = !{}
36+
--- |
37+
38+
; ModuleID = '../llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll'
39+
source_filename = "../llvm/test/CodeGen/X86/tail-merging-preserve-debugloc.ll"
40+
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"
41+
target triple = "x86_64-grtev4-linux-gnu"
42+
43+
define i32 @main(i1 %0) {
44+
entry:
45+
br i1 %0, label %1, label %2
46+
47+
1: ; preds = %entry
48+
store i64 1, ptr null, align 1
49+
br label %3, !dbg !3
50+
51+
2: ; preds = %entry
52+
store i64 0, ptr null, align 1
53+
br label %3
54+
55+
3: ; preds = %2, %1
56+
ret i32 0
57+
}
58+
59+
!llvm.dbg.cu = !{!0}
60+
!llvm.module.flags = !{!2}
61+
62+
!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, nameTableKind: None)
63+
!1 = !DIFile(filename: "foo.c", directory: "/tmp", checksumkind: CSK_MD5, checksum: "2d07c91bb9d9c2fa4eee31a1aeed20e3")
64+
!2 = !{i32 2, !"Debug Info Version", i32 3}
65+
!3 = !DILocation(line: 17, column: 3, scope: !4)
66+
!4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
67+
!5 = !DISubroutineType(types: !6)
68+
!6 = !{}
69+
70+
...
71+
---
72+
name: main
73+
body: |
74+
bb.0.entry:
75+
successors: %bb.1(0x40000000), %bb.2(0x40000000)
76+
liveins: $edi
77+
78+
TEST8ri renamable $dil, 1, implicit-def $eflags, implicit killed $edi
79+
JCC_1 %bb.2, 4, implicit killed $eflags
80+
JMP_1 %bb.1
81+
82+
bb.1 (%ir-block.1):
83+
successors: %bb.3(0x80000000)
84+
85+
MOV64mi32 $noreg, 1, $noreg, 0, $noreg, 1 :: (store (s64) into `ptr null`, align 1)
86+
JMP_1 %bb.3, debug-location !3
87+
88+
bb.2 (%ir-block.2):
89+
successors: %bb.3(0x80000000)
90+
91+
MOV64mi32 $noreg, 1, $noreg, 0, $noreg, 0 :: (store (s64) into `ptr null`, align 1)
92+
93+
bb.3 (%ir-block.3):
94+
$eax = MOV32r0 implicit-def dead $eflags
95+
RET 0, killed $eax
96+
97+
...
98+
# CHECK: [[DEBUGNUM:!.*]] = !DILocation(line: 17, column: 3,
99+
# CHECK: JMP_1 %bb.3, debug-location [[DEBUGNUM]]

‎llvm/test/DebugInfo/COFF/local-variables.ll‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@
4545
; ASM: .cv_loc 1 1 5 3 # t.cpp:5:3
4646
; ASM: callq capture
4747
; ASM: leaq 40(%rsp), %rcx
48+
; ASM: [[end_inline_1:\.Ltmp.*]]:
49+
; ASM: .cv_loc 0 1 11 5 # t.cpp:11:5
4850
; ASM: jmp .LBB0_3
4951
; ASM: [[else_start:\.Ltmp.*]]:
5052
; ASM: .LBB0_2: # %if.else
@@ -87,7 +89,7 @@
8789
; ASM: .long 116 # TypeIndex
8890
; ASM: .short 0 # Flags
8991
; ASM: .asciz "v"
90-
; ASM: .cv_def_range [[inline_site1]] [[else_start]], frame_ptr_rel, 44
92+
; ASM: .cv_def_range [[inline_site1]] [[end_inline_1]], frame_ptr_rel, 44
9193
; ASM: .short 4430 # Record kind: S_INLINESITE_END
9294
; ASM: .short 4429 # Record kind: S_INLINESITE
9395
; ASM: .short 4414 # Record kind: S_LOCAL
@@ -154,7 +156,7 @@
154156
; OBJ: ChangeLineOffset: 1
155157
; OBJ: ChangeCodeOffset: 0x14
156158
; OBJ: ChangeCodeOffsetAndLineOffset: {CodeOffset: 0xD, LineOffset: 1}
157-
; OBJ: ChangeCodeLength: 0xC
159+
; OBJ: ChangeCodeLength: 0xA
158160
; OBJ: ]
159161
; OBJ: }
160162
; OBJ: LocalSym {
@@ -168,7 +170,7 @@
168170
; OBJ: LocalVariableAddrRange {
169171
; OBJ: OffsetStart: .text+0x14
170172
; OBJ: ISectStart: 0x0
171-
; OBJ: Range: 0x19
173+
; OBJ: Range: 0x17
172174
; OBJ: }
173175
; OBJ: }
174176
; OBJ: InlineSiteEnd {

0 commit comments

Comments
 (0)