[NFC][MC][Dwarf] Add Range/Location List Entry fragment to reduce memory usage #146098
+202
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ory usage DWARF offset_pair kind RLE and LLEs encode two symbol offsets, and LLEs additionally encode a Location Description. At worst this results in two MCLEBFragments for the offsets (208 bytes each) and an MCDataFragment for the expression (208 bytes again). Add a dedicated offset pair kind list entry fragment, MCDwarfLocListOffsetPairFragment (144 bytes). To avoid additional complexity and code duplication, if either of the offsets might span linker relaxable instructions we revert to default (pre-patch) behaviour. We also use that code path for Asm printing as it's easier to shuffle comments around that way.
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-mc Author: Orlando Cazalet-Hyams (OCHyams) ChangesDWARF offset_pair kind RLE and LLEs encode two symbol offsets, and LLEs additionally encode a Location Description. At worst this results in two MCLEBFragments for the offsets (208 bytes each) and an MCDataFragment for the expression (208 bytes again). Add a dedicated offset pair kind list entry fragment, MCDwarfLocListOffsetPairFragment (144 bytes). To avoid additional complexity and code duplication, if either of the offsets might span linker relaxable instructions we revert to default (pre-patch) behaviour. We also use that code path for Asm printing as it's easier to shuffle comments around that way. This is the 2nd patch of two in which we try to reduce Compile-time-tracker max-rss link. Slight reduction to average max-rss with multiple percentage point wins for sqlite3 and tramp3d-v4. Minor reduction of instructions:u too. According to massif, a I tried various different configurations for this patch:
I'm not massively familiar with this area - I'm especially dubious of my bail-out-of-linker-relaxations code in I've not written a test yet - cooking that up now - I just thought it might be useful to get expert eyes on this in the mean time in case it's an undesirable direction. Credit to jmorse for the find and patch, I just tidied it up and shuffled some bits around. Full diff: https://github.com/llvm/llvm-project/pull/146098.diff 11 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index 6ad54fcd6d0e5..980311902354f 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -716,6 +716,11 @@ class LLVM_ABI AsmPrinter : public MachineFunctionPass {
// Dwarf Emission Helper Routines
//===------------------------------------------------------------------===//
+ MCDwarfLocListOffsetPairFragment *
+ emitDwarfLocListOffsetPairEntry(int8_t OffsetPair, const MCSymbol *Base,
+ const MCSymbol *Begin, const MCSymbol *End,
+ StringRef EnumEle);
+
/// Emit a .byte 42 directive that corresponds to an encoding. If verbose
/// assembly output is enabled, we output comments describing the encoding.
/// Desc is a string saying what the encoding is specifying (e.g. "LSDA").
diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h
index 5e009090311c4..073ae04f291d6 100644
--- a/llvm/include/llvm/MC/MCAssembler.h
+++ b/llvm/include/llvm/MC/MCAssembler.h
@@ -124,6 +124,7 @@ class MCAssembler {
bool relaxBoundaryAlign(MCBoundaryAlignFragment &BF);
bool relaxDwarfLineAddr(MCDwarfLineAddrFragment &DF);
bool relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF);
+ bool relaxDwarfLoclistEntry(MCDwarfLocListOffsetPairFragment &DF);
bool relaxCVInlineLineTable(MCCVInlineLineTableFragment &DF);
bool relaxCVDefRange(MCCVDefRangeFragment &DF);
bool relaxFill(MCFillFragment &F);
diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h
index 473dec6477df6..de599fa15f3d2 100644
--- a/llvm/include/llvm/MC/MCFragment.h
+++ b/llvm/include/llvm/MC/MCFragment.h
@@ -45,6 +45,7 @@ class MCFragment {
FT_Org,
FT_Dwarf,
FT_DwarfFrame,
+ FT_DwarfLoclistEntry,
FT_LEB,
FT_BoundaryAlign,
FT_SymbolId,
@@ -135,6 +136,7 @@ class MCEncodedFragment : public MCFragment {
case MCFragment::FT_Data:
case MCFragment::FT_Dwarf:
case MCFragment::FT_DwarfFrame:
+ case MCFragment::FT_DwarfLoclistEntry:
case MCFragment::FT_PseudoProbe:
return true;
}
@@ -197,7 +199,8 @@ class MCEncodedFragmentWithFixups : public MCEncodedFragment {
MCFragment::FragmentType Kind = F->getKind();
return Kind == MCFragment::FT_Relaxable || Kind == MCFragment::FT_Data ||
Kind == MCFragment::FT_CVDefRange || Kind == MCFragment::FT_Dwarf ||
- Kind == MCFragment::FT_DwarfFrame;
+ Kind == MCFragment::FT_DwarfFrame ||
+ Kind == MCFragment::FT_DwarfLoclistEntry;
}
};
@@ -440,6 +443,29 @@ class MCDwarfCallFrameFragment : public MCEncodedFragmentWithFixups<8, 1> {
}
};
+class MCContext;
+
+/// Represents a DWARF offset-pair kind location list or range list entry.
+/// Currently not suitable to use if either of the offsets require
+/// linker-relaxable relocations, which should be emitted as uleb fragments
+/// instead.
+/// LocationDescriptionExpr, which represents a DWARF location description,
+/// is only used for location list entries.
+class MCDwarfLocListOffsetPairFragment
+ : public MCEncodedFragmentWithFixups<16, 0> {
+public:
+ SmallVector<char, 8> LocationDescriptionExpr;
+ const MCExpr *StartOffset;
+ const MCExpr *EndOffset;
+
+ MCDwarfLocListOffsetPairFragment(MCContext &Context, const MCSymbol *Base,
+ const MCSymbol *Begin, const MCSymbol *End);
+
+ static bool classof(const MCFragment *F) {
+ return F->getKind() == MCFragment::FT_DwarfLoclistEntry;
+ }
+};
+
/// Represents a symbol table index fragment.
class MCSymbolIdFragment : public MCFragment {
const MCSymbol *Sym;
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index c987bc2426e9f..3faab83ae366c 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -200,6 +200,11 @@ class MCObjectStreamer : public MCStreamer {
void emitAbsoluteSymbolDiffAsULEB128(const MCSymbol *Hi,
const MCSymbol *Lo) override;
+ MCDwarfLocListOffsetPairFragment *
+ emitDwarfLocListOffsetPairEntry(int8_t OffsetPair, const MCSymbol *Base,
+ const MCSymbol *Begin, const MCSymbol *End,
+ StringRef EnumEle) override;
+
bool mayHaveInstructions(MCSection &Sec) const override;
/// Emits pending conditional assignments that depend on \p Symbol
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index 8f2e137ea0c84..b6df36bff0c2b 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -978,6 +978,11 @@ class LLVM_ABI MCStreamer {
virtual void emitAbsoluteSymbolDiffAsULEB128(const MCSymbol *Hi,
const MCSymbol *Lo);
+ virtual MCDwarfLocListOffsetPairFragment *
+ emitDwarfLocListOffsetPairEntry(int8_t OffsetPair, const MCSymbol *Base,
+ const MCSymbol *Begin, const MCSymbol *End,
+ StringRef EnumEle);
+
virtual MCSymbol *getDwarfLineTableSymbol(unsigned CUID);
virtual void emitCFISections(bool EH, bool Debug);
void emitCFIStartProc(bool IsSimple, SMLoc Loc = SMLoc());
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 3b96225236cd6..b802ceed30544 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -3383,6 +3383,13 @@ void AsmPrinter::emitLabelDifferenceAsULEB128(const MCSymbol *Hi,
OutStreamer->emitAbsoluteSymbolDiffAsULEB128(Hi, Lo);
}
+MCDwarfLocListOffsetPairFragment *AsmPrinter::emitDwarfLocListOffsetPairEntry(
+ int8_t OffsetPair, const MCSymbol *Base, const MCSymbol *Begin,
+ const MCSymbol *End, StringRef EnumEle) {
+ return OutStreamer->emitDwarfLocListOffsetPairEntry(OffsetPair, Base, Begin,
+ End, EnumEle);
+}
+
/// EmitLabelPlusOffset - Emit something like ".long Label+Offset"
/// where the size in bytes of the directive is specified by Size and Label
/// specifies the label. This implicitly uses .set if it is available.
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 0edfca78b0886..60959afc3f3b7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -3308,6 +3308,7 @@ static void emitRangeList(
}
for (const auto *RS : P.second) {
+ MCDwarfLocListOffsetPairFragment *LE = nullptr;
const MCSymbol *Begin = RS->Begin;
const MCSymbol *End = RS->End;
assert(Begin && "Range without a begin symbol?");
@@ -3315,12 +3316,8 @@ static void emitRangeList(
if (Base) {
if (UseDwarf5) {
// Emit offset_pair when we have a base.
- Asm->OutStreamer->AddComment(StringifyEnum(OffsetPair));
- Asm->emitInt8(OffsetPair);
- Asm->OutStreamer->AddComment(" starting offset");
- Asm->emitLabelDifferenceAsULEB128(Begin, Base);
- Asm->OutStreamer->AddComment(" ending offset");
- Asm->emitLabelDifferenceAsULEB128(End, Base);
+ LE = Asm->emitDwarfLocListOffsetPairEntry(
+ OffsetPair, Base, Begin, End, StringifyEnum(OffsetPair));
} else {
Asm->emitLabelDifference(Begin, Base, Size);
Asm->emitLabelDifference(End, Base, Size);
@@ -3336,7 +3333,7 @@ static void emitRangeList(
Asm->OutStreamer->emitSymbolValue(Begin, Size);
Asm->OutStreamer->emitSymbolValue(End, Size);
}
- EmitPayload(*RS);
+ EmitPayload(*RS, LE);
}
}
@@ -3357,8 +3354,18 @@ static void emitLocList(DwarfDebug &DD, AsmPrinter *Asm, const DebugLocStream::L
dwarf::DW_LLE_offset_pair, dwarf::DW_LLE_startx_length,
dwarf::DW_LLE_end_of_list, llvm::dwarf::LocListEncodingString,
/* ShouldUseBaseAddress */ true,
- [&](const DebugLocStream::Entry &E) {
- DD.emitDebugLocEntryLocation(E, List.CU);
+ [&](const DebugLocStream::Entry &E,
+ MCDwarfLocListOffsetPairFragment *LLE) {
+ if (LLE) {
+ // We don't need to emit the length header if we're writing
+ // to an entry fragment directly.
+ std::vector<std::string> Comments;
+ BufferByteStreamer S(LLE->LocationDescriptionExpr, Comments,
+ /*GenerateComments*/ false);
+ DD.emitDebugLocEntry(S, E, List.CU);
+ } else {
+ DD.emitDebugLocEntryLocation(E, List.CU);
+ }
});
}
@@ -3579,13 +3586,12 @@ void DwarfDebug::emitDebugARanges() {
/// Emit a single range list. We handle both DWARF v5 and earlier.
static void emitRangeList(DwarfDebug &DD, AsmPrinter *Asm,
const RangeSpanList &List) {
- emitRangeList(DD, Asm, List.Label, List.Ranges, *List.CU,
- dwarf::DW_RLE_base_addressx, dwarf::DW_RLE_offset_pair,
- dwarf::DW_RLE_startx_length, dwarf::DW_RLE_end_of_list,
- llvm::dwarf::RangeListEncodingString,
- List.CU->getCUNode()->getRangesBaseAddress() ||
- DD.getDwarfVersion() >= 5,
- [](auto) {});
+ emitRangeList(
+ DD, Asm, List.Label, List.Ranges, *List.CU, dwarf::DW_RLE_base_addressx,
+ dwarf::DW_RLE_offset_pair, dwarf::DW_RLE_startx_length,
+ dwarf::DW_RLE_end_of_list, llvm::dwarf::RangeListEncodingString,
+ List.CU->getCUNode()->getRangesBaseAddress() || DD.getDwarfVersion() >= 5,
+ [](auto, auto) {});
}
void DwarfDebug::emitDebugRangesImpl(const DwarfFile &Holder, MCSection *Section) {
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 1866c5b9e0e81..89acc0d9963e5 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -296,6 +296,8 @@ uint64_t MCAssembler::computeFragmentSize(const MCFragment &F) const {
return cast<MCDwarfLineAddrFragment>(F).getContents().size();
case MCFragment::FT_DwarfFrame:
return cast<MCDwarfCallFrameFragment>(F).getContents().size();
+ case MCFragment::FT_DwarfLoclistEntry:
+ return cast<MCDwarfLocListOffsetPairFragment>(F).getContents().size();
case MCFragment::FT_CVInlineLines:
return cast<MCCVInlineLineTableFragment>(F).getContents().size();
case MCFragment::FT_CVDefRange:
@@ -730,6 +732,12 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
OS << CF.getContents();
break;
}
+ case MCFragment::FT_DwarfLoclistEntry: {
+ const MCDwarfLocListOffsetPairFragment &OF =
+ cast<MCDwarfLocListOffsetPairFragment>(F);
+ OS << OF.getContents();
+ break;
+ }
case MCFragment::FT_CVInlineLines: {
const auto &OF = cast<MCCVInlineLineTableFragment>(F);
OS << OF.getContents();
@@ -1150,6 +1158,49 @@ bool MCAssembler::relaxDwarfCallFrameFragment(MCDwarfCallFrameFragment &DF) {
return OldSize != Data.size();
}
+bool MCAssembler::relaxDwarfLoclistEntry(MCDwarfLocListOffsetPairFragment &DF) {
+ SmallVectorImpl<char> &Data = DF.getContents();
+ raw_svector_ostream OSE(Data);
+
+ int64_t DiffAInt, DiffBInt;
+ bool Abs = DF.StartOffset->evaluateKnownAbsolute(DiffAInt, *this);
+ assert(Abs && "We created a loc/range list entry with an invalid expression");
+
+ Abs = DF.EndOffset->evaluateKnownAbsolute(DiffBInt, *this);
+ assert(Abs && "We created a loc/range list entry with an invalid expression");
+ (void)Abs;
+
+ unsigned OldSize = Data.size();
+ Data.clear();
+
+ // We could track the list entry kind encoding in a field, but it so happens
+ // that LLE and RLE offset_pair encodings are both 0x4.
+ static_assert((unsigned)dwarf::DW_LLE_offset_pair ==
+ (unsigned)dwarf::DW_RLE_offset_pair);
+ // DWARVv5 p44.
+ // Each location list entry begins with a single byte identifying the kind of
+ // that entry, followed by zero or more operands depending on the kind.
+ OSE << static_cast<uint8_t>(dwarf::DW_LLE_offset_pair);
+ // DWARFv5 p45, 54.
+ // [DW_LLE_offset_pair and DW_RLE_offset_pair have] two unsigned LEB128
+ // operands. The values of these operands are the starting and ending
+ // offsets, respectively, relative to the applicable base address, that
+ // define the address range.
+ encodeULEB128(DiffAInt, OSE);
+ encodeULEB128(DiffBInt, OSE);
+
+ // DWARFv5 p45.
+ // [DW_LLE_offset_pair] operands are followed by a counted location
+ // description.
+ if (unsigned Sz = DF.LocationDescriptionExpr.size()) {
+ encodeULEB128(Sz, OSE);
+ Data.append(DF.LocationDescriptionExpr.begin(),
+ DF.LocationDescriptionExpr.begin() + Sz);
+ }
+
+ return OldSize != Data.size();
+}
+
bool MCAssembler::relaxCVInlineLineTable(MCCVInlineLineTableFragment &F) {
unsigned OldSize = F.getContents().size();
getContext().getCVContext().encodeInlineLineTable(*this, F);
@@ -1198,6 +1249,8 @@ bool MCAssembler::relaxFragment(MCFragment &F) {
return relaxDwarfLineAddr(cast<MCDwarfLineAddrFragment>(F));
case MCFragment::FT_DwarfFrame:
return relaxDwarfCallFrameFragment(cast<MCDwarfCallFrameFragment>(F));
+ case MCFragment::FT_DwarfLoclistEntry:
+ return relaxDwarfLoclistEntry(cast<MCDwarfLocListOffsetPairFragment>(F));
case MCFragment::FT_LEB:
return relaxLEB(cast<MCLEBFragment>(F));
case MCFragment::FT_BoundaryAlign:
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index aa4dec0a8e9d9..2c2d2c24ca0c8 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -17,7 +17,9 @@
#include "llvm/MC/MCSectionMachO.h"
#include "llvm/MC/MCSymbol.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Format.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
#include <utility>
@@ -72,6 +74,10 @@ void MCFragment::destroy() {
case FT_PseudoProbe:
cast<MCPseudoProbeAddrFragment>(this)->~MCPseudoProbeAddrFragment();
return;
+ case FT_DwarfLoclistEntry:
+ cast<MCDwarfLocListOffsetPairFragment>(this)
+ ->~MCDwarfLocListOffsetPairFragment();
+ return;
}
}
@@ -108,6 +114,9 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
case MCFragment::FT_Org: OS << "MCOrgFragment"; break;
case MCFragment::FT_Dwarf: OS << "MCDwarfFragment"; break;
case MCFragment::FT_DwarfFrame: OS << "MCDwarfCallFrameFragment"; break;
+ case MCFragment::FT_DwarfLoclistEntry:
+ OS << "MCDwarfLocListOffsetPairFragment";
+ break;
case MCFragment::FT_LEB: OS << "MCLEBFragment"; break;
case MCFragment::FT_BoundaryAlign: OS<<"MCBoundaryAlignFragment"; break;
case MCFragment::FT_SymbolId: OS << "MCSymbolIdFragment"; break;
@@ -195,6 +204,21 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
OS << " AddrDelta:" << CF->getAddrDelta();
break;
}
+ case MCFragment::FT_DwarfLoclistEntry: {
+ const auto *LF = cast<MCDwarfLocListOffsetPairFragment>(this);
+ OS << "\n "
+ << " StartOffset: " << LF->StartOffset
+ << " EndOffset: " << LF->EndOffset;
+ if (!LF->LocationDescriptionExpr.empty()) {
+ OS << " Expr: [";
+ llvm::interleave(
+ LF->LocationDescriptionExpr,
+ [&](uint8_t C) { OS << format_hex_no_prefix(C, 2); },
+ [&]() { OS << " "; });
+ OS << "]";
+ }
+ break;
+ }
case MCFragment::FT_LEB: {
const auto *LF = cast<MCLEBFragment>(this);
OS << "\n ";
@@ -241,3 +265,14 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
OS << ">";
}
#endif
+
+MCDwarfLocListOffsetPairFragment::MCDwarfLocListOffsetPairFragment(
+ MCContext &Context, const MCSymbol *Base, const MCSymbol *Begin,
+ const MCSymbol *End)
+ : MCEncodedFragmentWithFixups<16, 0>(FT_DwarfLoclistEntry, false) {
+ const MCExpr *BaseSym = MCSymbolRefExpr::create(Base, Context);
+ StartOffset = MCBinaryExpr::createSub(MCSymbolRefExpr::create(Begin, Context),
+ BaseSym, Context);
+ EndOffset = MCBinaryExpr::createSub(MCSymbolRefExpr::create(End, Context),
+ BaseSym, Context);
+}
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index e959a242dfcf5..5d154086666f5 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -122,6 +122,34 @@ void MCObjectStreamer::emitAbsoluteSymbolDiffAsULEB128(const MCSymbol *Hi,
MCStreamer::emitAbsoluteSymbolDiffAsULEB128(Hi, Lo);
}
+MCDwarfLocListOffsetPairFragment *
+MCObjectStreamer::emitDwarfLocListOffsetPairEntry(int8_t OffsetPair,
+ const MCSymbol *Base,
+ const MCSymbol *Begin,
+ const MCSymbol *End,
+ StringRef EnumEle) {
+ // Heuristic: if we can emit one of the offsets as a constant now that
+ // that consumes less memory than creating a MCDwarfLocListOffsetPairFragment.
+ bool BeginOrEndInBaseFragment = Base->getFragment() == Begin->getFragment() ||
+ Base->getFragment() == End->getFragment();
+ // If the offset ulebs require linker-relaxable relocations then fall back to
+ // default uleb emission, rather than using MCDwarfLocListOffsetPairFragment.
+ // FIXME: Is there a better way to check this?
+ bool SameSection = &Base->getSection() == &End->getSection() &&
+ &End->getSection() == &Begin->getSection();
+ bool MayBeLinkerRelaxable =
+ Base->getSection().isLinkerRelaxable() || !SameSection;
+ if (BeginOrEndInBaseFragment || MayBeLinkerRelaxable)
+ return MCStreamer::emitDwarfLocListOffsetPairEntry(OffsetPair, Base, Begin,
+ End, EnumEle);
+
+ MCDwarfLocListOffsetPairFragment *Frag =
+ getContext().allocFragment<MCDwarfLocListOffsetPairFragment>(
+ getContext(), Base, Begin, End);
+ insert(Frag);
+ return Frag;
+}
+
void MCObjectStreamer::reset() {
if (Assembler) {
Assembler->reset();
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index 5f1fd57802c7b..77ac1067cc435 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -1249,6 +1249,20 @@ void MCStreamer::emitAbsoluteSymbolDiffAsULEB128(const MCSymbol *Hi,
emitULEB128Value(Diff);
}
+MCDwarfLocListOffsetPairFragment *MCStreamer::emitDwarfLocListOffsetPairEntry(
+ int8_t OffsetPair, const MCSymbol *Base, const MCSymbol *Begin,
+ const MCSymbol *End, StringRef EnumEle) {
+ // Base impl: emit offsets independently, possibly resulting in multiple
+ // fragments.
+ AddComment(EnumEle);
+ emitInt8(OffsetPair);
+ AddComment(" starting offset");
+ emitAbsoluteSymbolDiffAsULEB128(Begin, Base);
+ AddComment(" ending offset");
+ emitAbsoluteSymbolDiffAsULEB128(End, Base);
+ return nullptr;
+}
+
void MCStreamer::emitSubsectionsViaSymbols() {
llvm_unreachable(
"emitSubsectionsViaSymbols only supported on Mach-O targets");
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- llvm/include/llvm/CodeGen/AsmPrinter.h llvm/include/llvm/MC/MCAssembler.h llvm/include/llvm/MC/MCFragment.h llvm/include/llvm/MC/MCObjectStreamer.h llvm/include/llvm/MC/MCStreamer.h llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp llvm/lib/MC/MCAssembler.cpp llvm/lib/MC/MCFragment.cpp llvm/lib/MC/MCObjectStreamer.cpp llvm/lib/MC/MCStreamer.cppView the diff from clang-format here.diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index 2c2d2c24c..6ed87dbcb 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -78,7 +78,7 @@ void MCFragment::destroy() {
cast<MCDwarfLocListOffsetPairFragment>(this)
->~MCDwarfLocListOffsetPairFragment();
return;
- }
+ }
}
const MCSymbol *MCFragment::getAtom() const {
|
|
I understand your perspective, but I'm cautious about this change. However, this would introduce delicate code dealing with neighbor fragments and linker-relaxation (I don't fully recall how the following code works, even though I authored most of the linker relaxation improvements in recent years; https://maskray.me/blog/2021-03-14-the-dark-side-of-riscv-linker-relaxation search "label difference"). // Heuristic: if we can emit one of the offsets as a constant now, that
// consumes less memory than creating a MCDwarfLocListOffsetPairFragment.
bool BeginOrEndInBaseFragment = Base->getFragment() == Begin->getFragment() ||
Base->getFragment() == End->getFragment();
// If the offset ulebs require linker-relaxable relocations then fall back to
// default uleb emission, rather than using MCDwarfLocListOffsetPairFragment.
// FIXME: Is there a better way to check this?
bool SameSection = &Base->getSection() == &End->getSection() &&
&End->getSection() == &Begin->getSection();
bool MayBeLinkerRelaxable =
Base->getSection().isLinkerRelaxable() || !SameSection;
if (BeginOrEndInBaseFragment || MayBeLinkerRelaxable)
return MCStreamer::emitDwarfLocListOffsetPairEntry(OffsetPair, Base, Begin,
End, EnumEle);@aengelke has shared some ideas for improvement in this area but hasn’t had time to integrate them into LLVM. I’d prefer we explore general fragment content optimizations before introducing complex code like this. While I believe a custom fragment could offer greater efficiency than a generic fragment improvement, the latter might be sufficient to reduce the need for introducing a new fragment. Your debug info improvements are valuable, but do they truly require such intricate assembler changes? To make |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DWARF offset_pair kind RLE and LLEs encode two symbol offsets, and LLEs additionally encode a Location Description. At worst this results in two MCLEBFragments for the offsets (208 bytes each) and an MCDataFragment for the expression (208 bytes again).
Add a dedicated offset pair kind list entry fragment, MCDwarfLocListOffsetPairFragment (144 bytes). To avoid additional complexity and code duplication, if either of the offsets might span linker relaxable instructions we revert to default (pre-patch) behaviour. We also use that code path for Asm printing as it's easier to shuffle comments around that way.
This is the 2nd patch of two in which we try to reduce
-goverhead to "make room" for Key Instructions (first here, has more of an impact).Compile-time-tracker max-rss link. Slight reduction to average max-rss with multiple percentage point wins for sqlite3 and tramp3d-v4. Minor reduction of instructions:u too.
According to massif, a
-O2 -gbuild of sqlite3 from CTMark allocates 6.5% less memory with this patch.I tried various different configurations for this patch:
I'm not massively familiar with this area - I'm especially dubious of my bail-out-of-linker-relaxations code in
MCObjectStreamer::emitDwarfLocListOffsetPairEntry. Does it look alright, or is there something better to do there?I've not written a test yet - cooking that up now - I just thought it might be useful to get expert eyes on this in the mean time in case it's an undesirable direction.
Credit to jmorse for the find and patch, I just tidied it up and shuffled some bits around.