[llvm] [BOLT][DWARF][NFC] Split DIEBuilder::finish (PR #101244)
Sayhaan Siddiqui via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 30 14:44:50 PDT 2024
https://github.com/sayhaan updated https://github.com/llvm/llvm-project/pull/101244
>From 9afce89392bac3656f5a5d52548c110a1bdd2a2b Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Tue, 30 Jul 2024 14:24:00 -0700
Subject: [PATCH 1/3] [BOLT][DWARF][NFC] Split DIEBuilder::finish
---
bolt/include/bolt/Core/DIEBuilder.h | 11 +++--
bolt/lib/Core/DIEBuilder.cpp | 64 +++++++++++++++++++++--------
bolt/lib/Rewrite/DWARFRewriter.cpp | 22 +++++++---
3 files changed, 72 insertions(+), 25 deletions(-)
diff --git a/bolt/include/bolt/Core/DIEBuilder.h b/bolt/include/bolt/Core/DIEBuilder.h
index 0b840c142ed81..4895d384e3c92 100644
--- a/bolt/include/bolt/Core/DIEBuilder.h
+++ b/bolt/include/bolt/Core/DIEBuilder.h
@@ -207,9 +207,11 @@ class DIEBuilder {
/// Along with current CU, and DIE being processed and the new DIE offset to
/// be updated, it takes in Parents vector that can be empty if this DIE has
/// no parents.
- uint32_t finalizeDIEs(DWARFUnit &CU, DIE &Die,
- std::optional<BOLTDWARF5AccelTableData *> Parent,
- uint32_t NumberParentsInChain, uint32_t &CurOffset);
+ uint32_t finalizeDIEs(DWARFUnit &CU, DIE &Die, uint32_t &CurOffset);
+
+ void populateDebugNamesTable(DWARFUnit &CU, DIE &Die,
+ std::optional<BOLTDWARF5AccelTableData *> Parent,
+ uint32_t NumberParentsInChain);
void registerUnit(DWARFUnit &DU, bool NeedSort);
@@ -338,6 +340,9 @@ class DIEBuilder {
/// Finish current DIE construction.
void finish();
+ /// Update debug names table.
+ void updateDebugNamesTable();
+
// Interface to edit DIE
template <class T> T *allocateDIEValue() {
return new (getState().DIEAlloc) T;
diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp
index 8f6195f6b6ea1..4f0a97cd31b74 100644
--- a/bolt/lib/Core/DIEBuilder.cpp
+++ b/bolt/lib/Core/DIEBuilder.cpp
@@ -461,17 +461,11 @@ getUnitForOffset(DIEBuilder &Builder, DWARFContext &DWCtx,
return nullptr;
}
-uint32_t
-DIEBuilder::finalizeDIEs(DWARFUnit &CU, DIE &Die,
- std::optional<BOLTDWARF5AccelTableData *> Parent,
- uint32_t NumberParentsInChain, uint32_t &CurOffset) {
+uint32_t DIEBuilder::finalizeDIEs(DWARFUnit &CU, DIE &Die,
+ uint32_t &CurOffset) {
getState().DWARFDieAddressesParsed.erase(Die.getOffset());
uint32_t CurSize = 0;
Die.setOffset(CurOffset);
- std::optional<BOLTDWARF5AccelTableData *> NameEntry =
- DebugNamesTable.addAccelTableEntry(
- CU, Die, SkeletonCU ? SkeletonCU->getDWOId() : std::nullopt,
- NumberParentsInChain, Parent);
// It is possible that an indexed debugging information entry has a parent
// that is not indexed (for example, if its parent does not have a name
// attribute). In such a case, a parent attribute may point to a nameless
@@ -485,18 +479,13 @@ DIEBuilder::finalizeDIEs(DWARFUnit &CU, DIE &Die,
// If Parent is nullopt and NumberParentsInChain is not zero, then forward
// declaration was encountered in this DF traversal. Propagating nullopt for
// Parent to children.
- if (!Parent && NumberParentsInChain)
- NameEntry = std::nullopt;
- if (NameEntry)
- ++NumberParentsInChain;
for (DIEValue &Val : Die.values())
CurSize += Val.sizeOf(CU.getFormParams());
CurSize += getULEB128Size(Die.getAbbrevNumber());
CurOffset += CurSize;
for (DIE &Child : Die.children()) {
- uint32_t ChildSize =
- finalizeDIEs(CU, Child, NameEntry, NumberParentsInChain, CurOffset);
+ uint32_t ChildSize = finalizeDIEs(CU, Child, CurOffset);
CurSize += ChildSize;
}
// for children end mark.
@@ -514,10 +503,9 @@ void DIEBuilder::finish() {
DIE *UnitDIE = getUnitDIEbyUnit(CU);
uint32_t HeaderSize = CU.getHeaderSize();
uint32_t CurOffset = HeaderSize;
- DebugNamesTable.setCurrentUnit(CU, UnitStartOffset);
std::vector<std::optional<BOLTDWARF5AccelTableData *>> Parents;
Parents.push_back(std::nullopt);
- finalizeDIEs(CU, *UnitDIE, std::nullopt, 0, CurOffset);
+ finalizeDIEs(CU, *UnitDIE, CurOffset);
DWARFUnitInfo &CurUnitInfo = getUnitInfoByDwarfUnit(CU);
CurUnitInfo.UnitOffset = UnitStartOffset;
@@ -535,11 +523,12 @@ void DIEBuilder::finish() {
finalizeCU(*CU, TypeUnitStartOffset);
}
+ uint64_t UnitSizes = UnitSize;
for (DWARFUnit *CU : getState().DUList) {
// Skipping DWARF4 types.
if (CU->getVersion() < 5 && CU->isTypeUnit())
continue;
- finalizeCU(*CU, UnitSize);
+ finalizeCU(*CU, UnitSizes);
}
if (opts::Verbosity >= 1) {
if (!getState().DWARFDieAddressesParsed.empty())
@@ -548,6 +537,47 @@ void DIEBuilder::finish() {
dbgs() << Twine::utohexstr(Address) << "\n";
}
}
+}
+
+void DIEBuilder::populateDebugNamesTable(
+ DWARFUnit &CU, DIE &Die, std::optional<BOLTDWARF5AccelTableData *> Parent,
+ uint32_t NumberParentsInChain) {
+ std::optional<BOLTDWARF5AccelTableData *> NameEntry =
+ DebugNamesTable.addAccelTableEntry(
+ CU, Die, SkeletonCU ? SkeletonCU->getDWOId() : std::nullopt,
+ NumberParentsInChain, Parent);
+ if (!Parent && NumberParentsInChain)
+ NameEntry = std::nullopt;
+ if (NameEntry)
+ ++NumberParentsInChain;
+
+ for (DIE &Child : Die.children())
+ populateDebugNamesTable(CU, Child, NameEntry, NumberParentsInChain);
+}
+
+void DIEBuilder::updateDebugNamesTable() {
+ auto finalizeDebugNamesTableForCU = [&](DWARFUnit &CU,
+ uint64_t &UnitStartOffset) -> void {
+ DIE *UnitDIE = getUnitDIEbyUnit(CU);
+ DebugNamesTable.setCurrentUnit(CU, UnitStartOffset);
+ populateDebugNamesTable(CU, *UnitDIE, std::nullopt, 0);
+
+ DWARFUnitInfo &CurUnitInfo = getUnitInfoByDwarfUnit(CU);
+ UnitStartOffset += CurUnitInfo.UnitLength;
+ };
+
+ uint64_t TypeUnitStartOffset = 0;
+ for (DWARFUnit *CU : getState().DUList) {
+ if (!(CU->getVersion() < 5 && CU->isTypeUnit()))
+ break;
+ finalizeDebugNamesTableForCU(*CU, TypeUnitStartOffset);
+ }
+
+ for (DWARFUnit *CU : getState().DUList) {
+ if (CU->getVersion() < 5 && CU->isTypeUnit())
+ continue;
+ finalizeDebugNamesTableForCU(*CU, UnitSize);
+ }
updateReferences();
}
diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp
index beef1a8f902ad..f20aec81eb259 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -673,9 +673,8 @@ void DWARFRewriter::updateDebugInfo() {
DebugRangesSectionWriter &TempRangesSectionWriter,
DebugAddrWriter &AddressWriter,
const std::string &DWOName,
- const std::optional<std::string> &DwarfOutputPath) {
- DIEBuilder DWODIEBuilder(BC, &(SplitCU).getContext(), DebugNamesTable,
- &Unit);
+ const std::optional<std::string> &DwarfOutputPath,
+ DIEBuilder &DWODIEBuilder) {
DWODIEBuilder.buildDWOUnit(SplitCU);
DebugStrOffsetsWriter DWOStrOffstsWriter(BC);
DebugStrWriter DWOStrWriter((SplitCU).getContext(), true);
@@ -740,6 +739,7 @@ void DWARFRewriter::updateDebugInfo() {
finalizeTypeSections(DIEBlder, *Streamer, GDBIndexSection);
CUPartitionVector PartVec = partitionCUs(*BC.DwCtx);
+ llvm::DenseMap<uint64_t, std::unique_ptr<DIEBuilder>> DWODIEBuildersByCU;
for (std::vector<DWARFUnit *> &Vec : PartVec) {
DIEBlder.buildCompileUnits(Vec);
for (DWARFUnit *CU : DIEBlder.getProcessedCUs()) {
@@ -761,13 +761,23 @@ void DWARFRewriter::updateDebugInfo() {
: std::optional<std::string>(opts::DwarfOutputPath.c_str());
std::string DWOName = DIEBlder.updateDWONameCompDir(
*StrOffstsWriter, *StrWriter, *CU, DwarfOutputPath, std::nullopt);
+ auto DWODIEBuilderPtr = std::make_unique<DIEBuilder>(
+ BC, &(**SplitCU).getContext(), DebugNamesTable, CU);
+ DWODIEBuildersByCU[CU->getOffset()] = std::move(DWODIEBuilderPtr);
+ DIEBuilder &DWODIEBuilder = *DWODIEBuildersByCU[CU->getOffset()].get();
if (CU->getVersion() >= 5)
StrOffstsWriter->finalizeSection(*CU, DIEBlder);
processSplitCU(*CU, **SplitCU, DIEBlder, *TempRangesSectionWriter,
- AddressWriter, DWOName, DwarfOutputPath);
+ AddressWriter, DWOName, DwarfOutputPath, DWODIEBuilder);
}
- for (DWARFUnit *CU : DIEBlder.getProcessedCUs())
+ for (DWARFUnit *CU : DIEBlder.getProcessedCUs()) {
+ auto DWODIEBuilderIterator = DWODIEBuildersByCU.find(CU->getOffset());
+ if (DWODIEBuilderIterator != DWODIEBuildersByCU.end()) {
+ DIEBuilder &DWODIEBuilder = *DWODIEBuilderIterator->second.get();
+ DWODIEBuilder.updateDebugNamesTable();
+ }
processMainBinaryCU(*CU, DIEBlder);
+ }
finalizeCompileUnits(DIEBlder, *Streamer, OffsetMap,
DIEBlder.getProcessedCUs(), *FinalAddrWriter);
}
@@ -1468,6 +1478,7 @@ CUOffsetMap DWARFRewriter::finalizeTypeSections(DIEBuilder &DIEBlder,
// generate and populate abbrevs here
DIEBlder.generateAbbrevs();
DIEBlder.finish();
+ DIEBlder.updateDebugNamesTable();
SmallVector<char, 20> OutBuffer;
std::shared_ptr<raw_svector_ostream> ObjOS =
std::make_shared<raw_svector_ostream>(OutBuffer);
@@ -1672,6 +1683,7 @@ void DWARFRewriter::finalizeCompileUnits(DIEBuilder &DIEBlder,
}
DIEBlder.generateAbbrevs();
DIEBlder.finish();
+ DIEBlder.updateDebugNamesTable();
// generate debug_info and CUMap
for (DWARFUnit *CU : CUs) {
emitUnit(DIEBlder, Streamer, *CU);
>From 6206814b2e6892fab323c56af52251ccc1d5b57d Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Tue, 30 Jul 2024 14:33:07 -0700
Subject: [PATCH 2/3] Update comments
---
bolt/include/bolt/Core/DIEBuilder.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bolt/include/bolt/Core/DIEBuilder.h b/bolt/include/bolt/Core/DIEBuilder.h
index 4895d384e3c92..3d19d198d4dd7 100644
--- a/bolt/include/bolt/Core/DIEBuilder.h
+++ b/bolt/include/bolt/Core/DIEBuilder.h
@@ -203,12 +203,13 @@ class DIEBuilder {
/// Update references once the layout is finalized.
void updateReferences();
- /// Update the Offset and Size of DIE, populate DebugNames table.
+ /// Update the Offset and Size of DIE.
/// Along with current CU, and DIE being processed and the new DIE offset to
/// be updated, it takes in Parents vector that can be empty if this DIE has
/// no parents.
uint32_t finalizeDIEs(DWARFUnit &CU, DIE &Die, uint32_t &CurOffset);
+ /// Populates DebugNames table
void populateDebugNamesTable(DWARFUnit &CU, DIE &Die,
std::optional<BOLTDWARF5AccelTableData *> Parent,
uint32_t NumberParentsInChain);
>From 520ac71f41892b2f8c44a1c25989e65f60bafebd Mon Sep 17 00:00:00 2001
From: Sayhaan Siddiqui <sayhaan at meta.com>
Date: Tue, 30 Jul 2024 14:44:32 -0700
Subject: [PATCH 3/3] Formatting change
---
bolt/include/bolt/Core/DIEBuilder.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bolt/include/bolt/Core/DIEBuilder.h b/bolt/include/bolt/Core/DIEBuilder.h
index 3d19d198d4dd7..5d565e8cec20b 100644
--- a/bolt/include/bolt/Core/DIEBuilder.h
+++ b/bolt/include/bolt/Core/DIEBuilder.h
@@ -209,7 +209,7 @@ class DIEBuilder {
/// no parents.
uint32_t finalizeDIEs(DWARFUnit &CU, DIE &Die, uint32_t &CurOffset);
- /// Populates DebugNames table
+ /// Populates DebugNames table.
void populateDebugNamesTable(DWARFUnit &CU, DIE &Die,
std::optional<BOLTDWARF5AccelTableData *> Parent,
uint32_t NumberParentsInChain);
More information about the llvm-commits
mailing list