[llvm] [BOLT] Reduce the number of emitted symbols. NFCI. (PR #70175)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 25 01:01:49 PDT 2023
https://github.com/maksfb created https://github.com/llvm/llvm-project/pull/70175
We emit a symbol before an instruction for a number of reasons, e.g. for tracking LocSyms, debug line, or if the instruction has a label annotation. Currently, we may emit multiple symbols per instruction.
Reuse the same label instead of creating and emitting new ones when possible. I'm planning to refactor EH labels as well in a separate diff.
Change getLabel() to return a pointer instead of std::optional<> since an empty label should be treated identically to no label.
>From df7cf43b0bdaf0335d101577678e2fcc9f773f94 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Tue, 24 Oct 2023 23:48:07 -0700
Subject: [PATCH] [BOLT] Reduce the number of emitted symbols. NFCI.
We emit a symbol before an instruction for a number of reasons, e.g. for
tracking LocSyms, debug line, or if the instruction has a label
annotation. Currently, we may emit multiple symbols per instruction.
Reuse the same label instead of creating and emitting new ones when
possible. I'm planning to refactor EH labels as well in a separate diff.
Change getLabel() to return a pointer instead of std::optional<> since
an empty label should be treated identically to no label.
---
bolt/include/bolt/Core/MCPlusBuilder.h | 2 +-
bolt/lib/Core/BinaryContext.cpp | 4 +-
bolt/lib/Core/BinaryEmitter.cpp | 54 ++++++++++++++++----------
bolt/lib/Core/MCPlusBuilder.cpp | 4 +-
bolt/lib/Passes/BinaryPasses.cpp | 8 ++--
5 files changed, 43 insertions(+), 29 deletions(-)
diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index d136c627bc5cc10..719133230651f06 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -1181,7 +1181,7 @@ class MCPlusBuilder {
bool clearOffset(MCInst &Inst);
/// Return the label of \p Inst, if available.
- std::optional<MCSymbol *> getLabel(const MCInst &Inst) const;
+ MCSymbol *getLabel(const MCInst &Inst) const;
/// Set the label of \p Inst. This label will be emitted right before \p Inst
/// is emitted to MCStreamer.
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 8e2331db3e33032..e28d8ed33c3cd04 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -1901,8 +1901,8 @@ void BinaryContext::printInstruction(raw_ostream &OS, const MCInst &Instruction,
}
if (std::optional<uint32_t> Offset = MIB->getOffset(Instruction))
OS << " # Offset: " << *Offset;
- if (auto Label = MIB->getLabel(Instruction))
- OS << " # Label: " << **Label;
+ if (MCSymbol *Label = MIB->getLabel(Instruction))
+ OS << " # Label: " << *Label;
MIB->printAnnotations(Instruction, OS);
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index de80a99a74ed080..9b8d9d69faeaf8f 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -161,9 +161,17 @@ class BinaryEmitter {
/// \p FirstInstr indicates if \p NewLoc represents the first instruction
/// in a sequence, such as a function fragment.
///
+ /// If \p NewLoc location matches \p PrevLoc, no new line number entry will be
+ /// created and the function will return \p PrevLoc while \p InstrLabel will
+ /// be ignored. Otherwise, the caller should use \p InstrLabel to mark the
+ /// corresponding instruction by emitting \p InstrLabel before it.
+ /// If \p InstrLabel is set by the caller, its value will be used with \p
+ /// \p NewLoc. If it was nullptr on entry, it will be populated with a pointer
+ /// to a new temp symbol used with \p NewLoc.
+ ///
/// Return new current location which is either \p NewLoc or \p PrevLoc.
SMLoc emitLineInfo(const BinaryFunction &BF, SMLoc NewLoc, SMLoc PrevLoc,
- bool FirstInstr);
+ bool FirstInstr, MCSymbol *&InstrLabel);
/// Use \p FunctionEndSymbol to mark the end of the line info sequence.
/// Note that it does not automatically result in the insertion of the EOS
@@ -483,23 +491,28 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
// are relaxable, we should be safe.
}
- if (!EmitCodeOnly && opts::UpdateDebugSections && BF.getDWARFUnit()) {
- LastLocSeen = emitLineInfo(BF, Instr.getLoc(), LastLocSeen, FirstInstr);
- FirstInstr = false;
- }
+ if (!EmitCodeOnly) {
+ // A symbol to be emitted before the instruction to mark its location.
+ MCSymbol *InstrLabel = BC.MIB->getLabel(Instr);
- // Prepare to tag this location with a label if we need to keep track of
- // the location of calls/returns for BOLT address translation maps
- if (!EmitCodeOnly && BF.requiresAddressTranslation() &&
- BC.MIB->getOffset(Instr)) {
- const uint32_t Offset = *BC.MIB->getOffset(Instr);
- MCSymbol *LocSym = BC.Ctx->createTempSymbol();
- Streamer.emitLabel(LocSym);
- BB->getLocSyms().emplace_back(Offset, LocSym);
- }
+ if (opts::UpdateDebugSections && BF.getDWARFUnit()) {
+ LastLocSeen = emitLineInfo(BF, Instr.getLoc(), LastLocSeen,
+ FirstInstr, InstrLabel);
+ FirstInstr = false;
+ }
- if (auto Label = BC.MIB->getLabel(Instr))
- Streamer.emitLabel(*Label);
+ // Prepare to tag this location with a label if we need to keep track of
+ // the location of calls/returns for BOLT address translation maps
+ if (BF.requiresAddressTranslation() && BC.MIB->getOffset(Instr)) {
+ const uint32_t Offset = *BC.MIB->getOffset(Instr);
+ if (!InstrLabel)
+ InstrLabel = BC.Ctx->createTempSymbol();
+ BB->getLocSyms().emplace_back(Offset, InstrLabel);
+ }
+
+ if (InstrLabel)
+ Streamer.emitLabel(InstrLabel);
+ }
Streamer.emitInstruction(Instr, *BC.STI);
LastIsPrefix = BC.MIB->isPrefix(Instr);
@@ -661,7 +674,8 @@ void BinaryEmitter::emitConstantIslands(BinaryFunction &BF, bool EmitColdPart,
}
SMLoc BinaryEmitter::emitLineInfo(const BinaryFunction &BF, SMLoc NewLoc,
- SMLoc PrevLoc, bool FirstInstr) {
+ SMLoc PrevLoc, bool FirstInstr,
+ MCSymbol *&InstrLabel) {
DWARFUnit *FunctionCU = BF.getDWARFUnit();
const DWARFDebugLine::LineTable *FunctionLineTable = BF.getDWARFLineTable();
assert(FunctionCU && "cannot emit line info for function without CU");
@@ -711,12 +725,12 @@ SMLoc BinaryEmitter::emitLineInfo(const BinaryFunction &BF, SMLoc NewLoc,
const MCDwarfLoc &DwarfLoc = BC.Ctx->getCurrentDwarfLoc();
BC.Ctx->clearDwarfLocSeen();
- MCSymbol *LineSym = BC.Ctx->createTempSymbol();
- Streamer.emitLabel(LineSym);
+ if (!InstrLabel)
+ InstrLabel = BC.Ctx->createTempSymbol();
BC.getDwarfLineTable(FunctionUnitIndex)
.getMCLineSections()
- .addLineEntry(MCDwarfLineEntry(LineSym, DwarfLoc),
+ .addLineEntry(MCDwarfLineEntry(InstrLabel, DwarfLoc),
Streamer.getCurrentSectionOnly());
return NewLoc;
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 036fcf8b3e27fd9..677d43585f0ff5e 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -268,10 +268,10 @@ bool MCPlusBuilder::clearOffset(MCInst &Inst) {
return true;
}
-std::optional<MCSymbol *> MCPlusBuilder::getLabel(const MCInst &Inst) const {
+MCSymbol *MCPlusBuilder::getLabel(const MCInst &Inst) const {
if (auto Label = tryGetAnnotationAs<MCSymbol *>(Inst, MCAnnotation::kLabel))
return *Label;
- return std::nullopt;
+ return nullptr;
}
bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label,
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index e50fa9dea602bc7..8db22de37b33eab 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -610,8 +610,8 @@ void LowerAnnotations::runOnFunctions(BinaryContext &BC) {
if (BF.requiresAddressTranslation() && BC.MIB->getOffset(*II))
PreservedOffsetAnnotations.emplace_back(&(*II),
*BC.MIB->getOffset(*II));
- if (auto Label = BC.MIB->getLabel(*II))
- PreservedLabelAnnotations.emplace_back(&*II, *Label);
+ if (MCSymbol *Label = BC.MIB->getLabel(*II))
+ PreservedLabelAnnotations.emplace_back(&*II, Label);
BC.MIB->stripAnnotations(*II);
}
}
@@ -620,8 +620,8 @@ void LowerAnnotations::runOnFunctions(BinaryContext &BC) {
for (BinaryFunction *BF : BC.getInjectedBinaryFunctions())
for (BinaryBasicBlock &BB : *BF)
for (MCInst &Instruction : BB) {
- if (auto Label = BC.MIB->getLabel(Instruction))
- PreservedLabelAnnotations.emplace_back(&Instruction, *Label);
+ if (MCSymbol *Label = BC.MIB->getLabel(Instruction))
+ PreservedLabelAnnotations.emplace_back(&Instruction, Label);
BC.MIB->stripAnnotations(Instruction);
}
More information about the llvm-commits
mailing list