[llvm] Fix performance bug in buildLocationList (PR #108886)
Sriraman Tallam via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 19 12:32:02 PDT 2024
https://github.com/tmsri updated https://github.com/llvm/llvm-project/pull/108886
>From 68ff979c67883d85b6434bd1259a558ab0b5ff64 Mon Sep 17 00:00:00 2001
From: Sriraman Tallam <tmsriram at google.com>
Date: Mon, 16 Sep 2024 21:17:44 +0000
Subject: [PATCH 1/5] 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 <blah>
.loc 0 4 0 # ex2.cc:4:0
.cfi_startproc
.Ltmp0:
#DEBUG_VALUE: test:i <- 7
.loc 0 8 5 prologue_end # ex2.cc:8:5
....
.LBB_END0_0:
.cfi_endproc
.section .text._Z4testv,"ax", at 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.
---
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 13 ++++---
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 38 ++++++++++---------
.../DebugInfo/X86/basic-block-sections_1.ll | 6 +--
3 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index db7adfd3b21e5f..36e51a7e4639ad 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1737,6 +1737,12 @@ void AsmPrinter::emitFunctionBody() {
bool IsEHa = MMI->getModule()->getModuleFlag("eh-asynch");
bool CanDoExtraAnalysis = ORE->allowExtraAnalysis(DEBUG_TYPE);
+ /* Create a slot for the entry basic block section so that the section
+ order is preserved when iterating over MBBSectionRanges. */
+ if (!MF->empty()) {
+ MBBSectionRanges[MF->front().getSectionID()] = MBBSectionRange{CurrentFnBegin, nullptr};
+ }
+
for (auto &MBB : *MF) {
// Print a label for the basic block.
emitBasicBlockStart(MBB);
@@ -2000,11 +2006,8 @@ void AsmPrinter::emitFunctionBody() {
}
for (auto &Handler : Handlers)
Handler->markFunctionEnd();
-
- assert(!MBBSectionRanges.contains(MF->front().getSectionID()) &&
- "Overwrite section range");
- MBBSectionRanges[MF->front().getSectionID()] =
- MBBSectionRange{CurrentFnBegin, CurrentFnEnd};
+ // Update the end label of the entry block's section.
+ MBBSectionRanges[MF->front().getSectionID()].EndLabel = CurrentFnEnd;
// Print out jump tables referenced by the function.
emitJumpTableInfo();
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 148b620c2b62b7..d7c3fa5585695f 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -33,6 +33,7 @@
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalVariable.h"
@@ -1772,18 +1773,14 @@ bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
// span each individual section in the range from StartLabel to EndLabel.
if (Asm->MF->hasBBSections() && StartLabel == Asm->getFunctionBegin() &&
!Instr->getParent()->sameSection(&Asm->MF->front())) {
- const MCSymbol *BeginSectionLabel = StartLabel;
-
- for (const MachineBasicBlock &MBB : *Asm->MF) {
- if (MBB.isBeginSection() && &MBB != &Asm->MF->front())
- BeginSectionLabel = MBB.getSymbol();
-
- if (MBB.sameSection(Instr->getParent())) {
- DebugLoc.emplace_back(BeginSectionLabel, EndLabel, Values);
+ for (const auto &[MBBSectionId, MBBSectionRange] :
+ Asm->MBBSectionRanges) {
+ if (Instr->getParent()->getSectionID() == MBBSectionId) {
+ DebugLoc.emplace_back(MBBSectionRange.BeginLabel, EndLabel, Values);
break;
}
- if (MBB.isEndSection())
- DebugLoc.emplace_back(BeginSectionLabel, MBB.getEndSymbol(), Values);
+ DebugLoc.emplace_back(MBBSectionRange.BeginLabel,
+ MBBSectionRange.EndLabel, Values);
}
} else {
DebugLoc.emplace_back(StartLabel, EndLabel, Values);
@@ -1824,22 +1821,27 @@ bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
RangeMBB = &Asm->MF->front();
else
RangeMBB = Entries.begin()->getInstr()->getParent();
+ auto RangeIt = Asm->MBBSectionRanges.find(RangeMBB->getSectionID());
+ assert(RangeIt != Asm->MBBSectionRanges.end() &&
+ "Range MBB not found in MBBSectionRanges!");
auto *CurEntry = DebugLoc.begin();
auto *NextEntry = std::next(CurEntry);
+ auto NextRangeIt = std::next(RangeIt);
while (NextEntry != DebugLoc.end()) {
- // Get the last machine basic block of this section.
- while (!RangeMBB->isEndSection())
- RangeMBB = RangeMBB->getNextNode();
- if (!RangeMBB->getNextNode())
+ if (NextRangeIt == Asm->MBBSectionRanges.end())
return false;
// CurEntry should end the current section and NextEntry should start
// the next section and the Values must match for these two ranges to be
- // merged.
- if (CurEntry->getEndSym() != RangeMBB->getEndSymbol() ||
- NextEntry->getBeginSym() != RangeMBB->getNextNode()->getSymbol() ||
+ // merged. Don't match the section label end if it is the entry block
+ // section. This is because the end label for the Debug Loc and the
+ // Function end label could be different.
+ if ((RangeIt->second.EndLabel != Asm->getFunctionEnd()
+ && CurEntry->getEndSym() != RangeIt->second.EndLabel) ||
+ NextEntry->getBeginSym() != NextRangeIt->second.BeginLabel ||
CurEntry->getValues() != NextEntry->getValues())
return false;
- RangeMBB = RangeMBB->getNextNode();
+ RangeIt = NextRangeIt;
+ NextRangeIt = std::next(RangeIt);
CurEntry = NextEntry;
NextEntry = std::next(CurEntry);
}
diff --git a/llvm/test/DebugInfo/X86/basic-block-sections_1.ll b/llvm/test/DebugInfo/X86/basic-block-sections_1.ll
index 12b60c4dc321bd..c90d715142ec8b 100644
--- a/llvm/test/DebugInfo/X86/basic-block-sections_1.ll
+++ b/llvm/test/DebugInfo/X86/basic-block-sections_1.ll
@@ -16,10 +16,10 @@
; NO-SECTIONS: DW_AT_high_pc [DW_FORM_data4] ({{.*}})
; BB-SECTIONS: DW_AT_low_pc [DW_FORM_addr] (0x0000000000000000)
; BB-SECTIONS-NEXT: DW_AT_ranges [DW_FORM_sec_offset]
+; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi"
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi._Z3fooi.__part.1"
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi._Z3fooi.__part.2"
; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi._Z3fooi.__part.3"
-; BB-SECTIONS-NEXT: [{{.*}}) ".text.hot._Z3fooi"
; BB-SECTIONS-ASM: _Z3fooi:
; BB-SECTIONS-ASM: .Ltmp{{[0-9]+}}:
; BB-SECTIONS-ASM-NEXT: .loc 1 2 9 prologue_end
@@ -36,14 +36,14 @@
; BB-SECTIONS-ASM: .size _Z3fooi.__part.3, .LBB_END0_{{[0-9]+}}-_Z3fooi.__part.3
; BB-SECTIONS-ASM: .Lfunc_end0:
; BB-SECTIONS-ASM: .Ldebug_ranges0:
+; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_begin0
+; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_end0
; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.__part.1
; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.__part.2
; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
; BB-SECTIONS-ASM-NEXT: .quad _Z3fooi.__part.3
; BB-SECTIONS-ASM-NEXT: .quad .LBB_END0_{{[0-9]+}}
-; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_begin0
-; BB-SECTIONS-ASM-NEXT: .quad .Lfunc_end0
; BB-SECTIONS-ASM-NEXT: .quad 0
; BB-SECTIONS-ASM-NEXT: .quad 0
; BB-SECTIONS-LINE-TABLE: 0x0000000000000000 1 0 1 0 0 0 is_stmt
>From 816532bc3eff623eb7588c747024ea37c0e36465 Mon Sep 17 00:00:00 2001
From: Sriraman Tallam <tmsriram at google.com>
Date: Mon, 16 Sep 2024 21:46:23 +0000
Subject: [PATCH 2/5] Fix comment
---
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index d7c3fa5585695f..1765e9c5038523 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -1832,7 +1832,7 @@ bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
return false;
// CurEntry should end the current section and NextEntry should start
// the next section and the Values must match for these two ranges to be
- // merged. Don't match the section label end if it is the entry block
+ // merged. Do not match the section label end if it is the entry block
// section. This is because the end label for the Debug Loc and the
// Function end label could be different.
if ((RangeIt->second.EndLabel != Asm->getFunctionEnd()
>From 0af6f369afe1a68a7b00ccf960a95cf3ccb8fe59 Mon Sep 17 00:00:00 2001
From: Sriraman Tallam <tmsriram at google.com>
Date: Mon, 16 Sep 2024 22:03:12 +0000
Subject: [PATCH 3/5] clang-format
---
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 36e51a7e4639ad..5bff23e72d1737 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1737,11 +1737,10 @@ void AsmPrinter::emitFunctionBody() {
bool IsEHa = MMI->getModule()->getModuleFlag("eh-asynch");
bool CanDoExtraAnalysis = ORE->allowExtraAnalysis(DEBUG_TYPE);
- /* Create a slot for the entry basic block section so that the section
- order is preserved when iterating over MBBSectionRanges. */
- if (!MF->empty()) {
+ // Create a slot for the entry basic block section so that the section
+ // order is preserved when iterating over MBBSectionRanges.
+ if (!MF->empty())
MBBSectionRanges[MF->front().getSectionID()] = MBBSectionRange{CurrentFnBegin, nullptr};
- }
for (auto &MBB : *MF) {
// Print a label for the basic block.
>From bc3ef338e90aa8397aaa9cb22935d7959d28d76c Mon Sep 17 00:00:00 2001
From: Sriraman Tallam <tmsriram at google.com>
Date: Mon, 16 Sep 2024 22:22:30 +0000
Subject: [PATCH 4/5] clang-format
---
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 1765e9c5038523..4141605d27b09e 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -33,8 +33,8 @@
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
-#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/Module.h"
@@ -1835,8 +1835,8 @@ bool DwarfDebug::buildLocationList(SmallVectorImpl<DebugLocEntry> &DebugLoc,
// merged. Do not match the section label end if it is the entry block
// section. This is because the end label for the Debug Loc and the
// Function end label could be different.
- if ((RangeIt->second.EndLabel != Asm->getFunctionEnd()
- && CurEntry->getEndSym() != RangeIt->second.EndLabel) ||
+ if ((RangeIt->second.EndLabel != Asm->getFunctionEnd() &&
+ CurEntry->getEndSym() != RangeIt->second.EndLabel) ||
NextEntry->getBeginSym() != NextRangeIt->second.BeginLabel ||
CurEntry->getValues() != NextEntry->getValues())
return false;
>From 156ef0bbf235697c205d8e602e7ceec534d66388 Mon Sep 17 00:00:00 2001
From: Sriraman Tallam <tmsriram at google.com>
Date: Thu, 19 Sep 2024 19:31:12 +0000
Subject: [PATCH 5/5] clang-format
---
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 5bff23e72d1737..4c5afaa5132755 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1740,7 +1740,8 @@ void AsmPrinter::emitFunctionBody() {
// Create a slot for the entry basic block section so that the section
// order is preserved when iterating over MBBSectionRanges.
if (!MF->empty())
- MBBSectionRanges[MF->front().getSectionID()] = MBBSectionRange{CurrentFnBegin, nullptr};
+ MBBSectionRanges[MF->front().getSectionID()] =
+ MBBSectionRange{CurrentFnBegin, nullptr};
for (auto &MBB : *MF) {
// Print a label for the basic block.
More information about the llvm-commits
mailing list