Skip to content

Commit 53c0140

Browse files
committed
Fix performance bug in buildLocationList
In buildLocationList, with basic block sections, we iterate over every basic block twice to detect section start and end. This is sub-optimal and shows up as significantly time consuming when compiling large functions. This patch uses the set of sections already stored in MBBSectionRanges and iterates over sections rather than basic blocks. When detecting if loclists can be merged, the end label of an entry is matched with the beginning label of the next entry. For the section corresponding to the entry basic block, this is skipped. This is because the loc list uses the end label corresponding to the function whereas the MBBSectionRanges map uses the function end label. For example: .Lfunc_begin0: .file .loc 0 4 0 # ex2.cc:4:0 .cfi_startproc .Ltmp0: .loc 0 8 5 prologue_end # ex2.cc:8:5 .... .LBB_END0_0: .cfi_endproc .section .text._Z4testv,"ax",@progbits,unique,1 ... .Lfunc_end0: .size _Z4testv, .Lfunc_end0-_Z4testv The debug loc uses ".LBB_END0_0" for the end of the section whereas MBBSectionRanges uses ".Lfunc_end0". It is alright to skip this as we already check the section corresponding to the debugloc entry. Added a new test case to check that if this works correctly when the variable's value is mutated in the entry section.
1 parent 13dd2fd commit 53c0140

File tree

4 files changed

+123
-26
lines changed

4 files changed

+123
-26
lines changed

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,12 @@ void AsmPrinter::emitFunctionBody() {
17371737
bool IsEHa = MMI->getModule()->getModuleFlag("eh-asynch");
17381738

17391739
bool CanDoExtraAnalysis = ORE->allowExtraAnalysis(DEBUG_TYPE);
1740+
// Create a slot for the entry basic block section so that the section
1741+
// order is preserved when iterating over MBBSectionRanges.
1742+
if (!MF->empty())
1743+
MBBSectionRanges[MF->front().getSectionID()] =
1744+
MBBSectionRange{CurrentFnBegin, nullptr};
1745+
17401746
for (auto &MBB : *MF) {
17411747
// Print a label for the basic block.
17421748
emitBasicBlockStart(MBB);
@@ -2000,11 +2006,8 @@ void AsmPrinter::emitFunctionBody() {
20002006
}
20012007
for (auto &Handler : Handlers)
20022008
Handler->markFunctionEnd();
2003-
2004-
assert(!MBBSectionRanges.contains(MF->front().getSectionID()) &&
2005-
"Overwrite section range");
2006-
MBBSectionRanges[MF->front().getSectionID()] =
2007-
MBBSectionRange{CurrentFnBegin, CurrentFnEnd};
2009+
// Update the end label of the entry block's section.
2010+
MBBSectionRanges[MF->front().getSectionID()].EndLabel = CurrentFnEnd;
20082011

20092012
// Print out jump tables referenced by the function.
20102013
emitJumpTableInfo();

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
3535
#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
3636
#include "llvm/IR/Constants.h"
37+
#include "llvm/IR/DebugInfoMetadata.h"
3738
#include "llvm/IR/Function.h"
3839
#include "llvm/IR/GlobalVariable.h"
3940
#include "llvm/IR/Module.h"
@@ -1772,18 +1773,14 @@ bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
17721773
// span each individual section in the range from StartLabel to EndLabel.
17731774
if (Asm->MF->hasBBSections() && StartLabel == Asm->getFunctionBegin() &&
17741775
!Instr->getParent()->sameSection(&Asm->MF->front())) {
1775-
const MCSymbol *BeginSectionLabel = StartLabel;
1776-
1777-
for (const MachineBasicBlock &MBB : *Asm->MF) {
1778-
if (MBB.isBeginSection() && &MBB != &Asm->MF->front())
1779-
BeginSectionLabel = MBB.getSymbol();
1780-
1781-
if (MBB.sameSection(Instr->getParent())) {
1782-
DebugLoc.emplace_back(BeginSectionLabel, EndLabel, Values);
1776+
for (const auto &[MBBSectionId, MBBSectionRange] :
1777+
Asm->MBBSectionRanges) {
1778+
if (Instr->getParent()->getSectionID() == MBBSectionId) {
1779+
DebugLoc.emplace_back(MBBSectionRange.BeginLabel, EndLabel, Values);
17831780
break;
17841781
}
1785-
if (MBB.isEndSection())
1786-
DebugLoc.emplace_back(BeginSectionLabel, MBB.getEndSymbol(), Values);
1782+
DebugLoc.emplace_back(MBBSectionRange.BeginLabel,
1783+
MBBSectionRange.EndLabel, Values);
17871784
}
17881785
} else {
17891786
DebugLoc.emplace_back(StartLabel, EndLabel, Values);
@@ -1824,22 +1821,27 @@ bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
18241821
RangeMBB = &Asm->MF->front();
18251822
else
18261823
RangeMBB = Entries.begin()->getInstr()->getParent();
1824+
auto RangeIt = Asm->MBBSectionRanges.find(RangeMBB->getSectionID());
1825+
assert(RangeIt != Asm->MBBSectionRanges.end() &&
1826+
"Range MBB not found in MBBSectionRanges!");
18271827
auto *CurEntry = DebugLoc.begin();
18281828
auto *NextEntry = std::next(CurEntry);
1829+
auto NextRangeIt = std::next(RangeIt);
18291830
while (NextEntry != DebugLoc.end()) {
1830-
// Get the last machine basic block of this section.
1831-
while (!RangeMBB->isEndSection())
1832-
RangeMBB = RangeMBB->getNextNode();
1833-
if (!RangeMBB->getNextNode())
1831+
if (NextRangeIt == Asm->MBBSectionRanges.end())
18341832
return false;
18351833
// CurEntry should end the current section and NextEntry should start
18361834
// the next section and the Values must match for these two ranges to be
1837-
// merged.
1838-
if (CurEntry->getEndSym() != RangeMBB->getEndSymbol() ||
1839-
NextEntry->getBeginSym() != RangeMBB->getNextNode()->getSymbol() ||
1835+
// merged. Do not match the section label end if it is the entry block
1836+
// section. This is because the end label for the Debug Loc and the
1837+
// Function end label could be different.
1838+
if ((RangeIt->second.EndLabel != Asm->getFunctionEnd() &&
1839+
CurEntry->getEndSym() != RangeIt->second.EndLabel) ||
1840+
NextEntry->getBeginSym() != NextRangeIt->second.BeginLabel ||
18401841
CurEntry->getValues() != NextEntry->getValues())
18411842
return false;
1842-
RangeMBB = RangeMBB->getNextNode();
1843+
RangeIt = NextRangeIt;
1844+
NextRangeIt = std::next(RangeIt);
18431845
CurEntry = NextEntry;
18441846
NextEntry = std::next(CurEntry);
18451847
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
; RUN: llc %s -mtriple=x86_64-unknown-linux-gnu --dwarf-version=4 --basic-block-sections=none -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
2+
; RUN: llc %s -mtriple=x86_64-unknown-linux-gnu --dwarf-version=4 --basic-block-sections=all -filetype=obj -o - | llvm-dwarfdump - | FileCheck --check-prefix=SECTIONS %s
3+
; RUN: llc %s -mtriple=x86_64-unknown-linux-gnu --dwarf-version=5 --basic-block-sections=none -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
4+
; RUN: llc %s -mtriple=x86_64-unknown-linux-gnu --dwarf-version=5 --basic-block-sections=all -filetype=obj -o - | llvm-dwarfdump - | FileCheck --check-prefix=SECTIONS %s
5+
6+
; CHECK: DW_TAG_variable
7+
; CHECK-NEXT: DW_AT_location
8+
; CHECK-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_consts +7, DW_OP_stack_value
9+
; CHECK-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_consts +8, DW_OP_stack_value
10+
; CHECK-NEXT: DW_AT_name ("i")
11+
12+
; SECTIONS: DW_TAG_variable
13+
; SECTIONS-NEXT: DW_AT_location
14+
; SECTIONS-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_consts +7, DW_OP_stack_value
15+
; SECTIONS-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_consts +8, DW_OP_stack_value
16+
; SECTIONS-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_consts +8, DW_OP_stack_value
17+
; SECTIONS-NEXT: [0x{{[0-9a-f]+}}, 0x{{[0-9a-f]+}}): DW_OP_consts +8, DW_OP_stack_value
18+
; SECTIONS-NEXT: DW_AT_name ("i")
19+
20+
; Source to generate the IR below:
21+
; void f1();
22+
; extern bool b;
23+
; void test() {
24+
; // i is not a const throughout the whole scope and should
25+
; // not use DW_AT_const_value
26+
; int i = 7;
27+
; f1();
28+
; i = 8;
29+
; if (b)
30+
; f1();
31+
; }
32+
; $ clang++ -S loclist_section.cc -O2 -g -emit-llvm
33+
34+
@b = external local_unnamed_addr global i8, align 1
35+
36+
; Function Attrs: mustprogress uwtable
37+
define dso_local void @_Z4testv() local_unnamed_addr #0 !dbg !10 {
38+
entry:
39+
#dbg_value(i32 7, !14, !DIExpression(), !16)
40+
tail call void @_Z2f1v(), !dbg !17
41+
#dbg_value(i32 8, !14, !DIExpression(), !16)
42+
%0 = load i8, ptr @b, align 1, !dbg !18, !tbaa !20, !range !24, !noundef !25
43+
%loadedv = trunc nuw i8 %0 to i1, !dbg !18
44+
br i1 %loadedv, label %if.then, label %if.end, !dbg !26
45+
46+
if.then: ; preds = %entry
47+
tail call void @_Z2f1v(), !dbg !27
48+
br label %if.end, !dbg !27
49+
50+
if.end: ; preds = %if.then, %entry
51+
ret void, !dbg !28
52+
}
53+
54+
declare !dbg !29 void @_Z2f1v() local_unnamed_addr #1
55+
56+
attributes #0 = { mustprogress uwtable "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
57+
attributes #1 = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
58+
59+
!llvm.dbg.cu = !{!0}
60+
!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
61+
!llvm.ident = !{!9}
62+
63+
!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 20.0.0git ([email protected]:)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
64+
!1 = !DIFile(filename: "loclist_section.cc", directory: "Examples/debug_loc", checksumkind: CSK_MD5, checksum: "67769a94389681c8a6da481e2f358abb")
65+
!2 = !{i32 7, !"Dwarf Version", i32 5}
66+
!3 = !{i32 2, !"Debug Info Version", i32 3}
67+
!4 = !{i32 1, !"wchar_size", i32 4}
68+
!5 = !{i32 8, !"PIC Level", i32 2}
69+
!6 = !{i32 7, !"PIE Level", i32 2}
70+
!7 = !{i32 7, !"uwtable", i32 2}
71+
!8 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
72+
!9 = !{!"clang version 20.0.0git ([email protected]:.../llvm-project.git 7c3256280a78b0505ae4d43985c4d3239451a151)"}
73+
!10 = distinct !DISubprogram(name: "test", linkageName: "_Z4testv", scope: !1, file: !1, line: 3, type: !11, scopeLine: 3, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !13)
74+
!11 = !DISubroutineType(types: !12)
75+
!12 = !{null}
76+
!13 = !{!14}
77+
!14 = !DILocalVariable(name: "i", scope: !10, file: !1, line: 6, type: !15)
78+
!15 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
79+
!16 = !DILocation(line: 0, scope: !10)
80+
!17 = !DILocation(line: 7, column: 5, scope: !10)
81+
!18 = !DILocation(line: 9, column: 9, scope: !19)
82+
!19 = distinct !DILexicalBlock(scope: !10, file: !1, line: 9, column: 9)
83+
!20 = !{!21, !21, i64 0}
84+
!21 = !{!"bool", !22, i64 0}
85+
!22 = !{!"omnipotent char", !23, i64 0}
86+
!23 = !{!"Simple C++ TBAA"}
87+
!24 = !{i8 0, i8 2}
88+
!25 = !{}
89+
!26 = !DILocation(line: 9, column: 9, scope: !10)
90+
!27 = !DILocation(line: 10, column: 7, scope: !19)
91+
!28 = !DILocation(line: 11, column: 1, scope: !10)
92+
!29 = !DISubprogram(name: "f1", linkageName: "_Z2f1v", scope: !1, file: !1, line: 1, type: !11, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)

llvm/test/DebugInfo/X86/basic-block-sections_1.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
; NO-SECTIONS: DW_AT_high_pc [DW_FORM_data4] ({{.*}})
1717
; BB-SECTIONS: DW_AT_low_pc [DW_FORM_addr] (0x0000000000000000)
1818
; BB-SECTIONS-NEXT: DW_AT_ranges [DW_FORM_sec_offset]
19+
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi"
1920
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi._Z3fooi.__part.1"
2021
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi._Z3fooi.__part.2"
2122
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi._Z3fooi.__part.3"
22-
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi"
2323
; BB-SECTIONS-ASM: _Z3fooi:
2424
; BB-SECTIONS-ASM: .Ltmp{{[0-9]+}}:
2525
; BB-SECTIONS-ASM-NEXT: .loc 1 2 9 prologue_end
@@ -36,14 +36,14 @@
3636
; BB-SECTIONS-ASM: .size _Z3fooi.__part.3, .LBB_END0_{{[0-9]+}}-_Z3fooi.__part.3
3737
; BB-SECTIONS-ASM: .Lfunc_end0:
3838
; BB-SECTIONS-ASM: .Ldebug_ranges0:
39+
; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_begin0
40+
; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_end0
3941
; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.__part.1
4042
; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
4143
; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.__part.2
4244
; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
4345
; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.__part.3
4446
; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
45-
; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_begin0
46-
; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_end0
4747
; BB-SECTIONS-ASM-NEXT: .quad 0
4848
; BB-SECTIONS-ASM-NEXT: .quad 0
4949
; BB-SECTIONS-LINE-TABLE: 0x0000000000000000 1 0 1 0 0 0 is_stmt

0 commit comments

Comments
 (0)