[DWARFLinker][NFC] Decrease DWARFLinker dependence on DwarfStreamer. #77932
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
avl-llvm
requested review from
JDevlieghere,
adrian-prantl and
felipepiovezan
|
@llvm/pr-subscribers-debuginfo Author: Alexey Lapshin (avl-llvm) ChangesThis patch is extracted from #74725. The DwarfStreamer interface looks overcomplicated and has unnecessary dependencies:
This patch avoids creation of DwarfStreamer by DWARFLinker and simplifies interface:
Patch is 64.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77932.diff 22 Files Affected:
diff --git a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h
index d3aaa3baadc47d..bacf07beed95eb 100644
--- a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h
@@ -59,7 +59,8 @@ class DwarfEmitter {
virtual ~DwarfEmitter() = default;
/// Emit section named SecName with data SecData.
- virtual void emitSectionContents(StringRef SecData, StringRef SecName) = 0;
+ virtual void emitSectionContents(StringRef SecData,
+ DebugSectionKind SecKind) = 0;
/// Emit the abbreviation table \p Abbrevs to the .debug_abbrev section.
virtual void
@@ -205,17 +206,6 @@ class DwarfEmitter {
/// Dump the file to the disk.
virtual void finish() = 0;
-
- /// Emit the swift_ast section stored in \p Buffer.
- virtual void emitSwiftAST(StringRef Buffer) = 0;
-
- /// Emit the swift reflection section stored in \p Buffer.
- virtual void emitSwiftReflectionSection(
- llvm::binaryformat::Swift5ReflectionSectionKind ReflSectionKind,
- StringRef Buffer, uint32_t Alignment, uint32_t Size) = 0;
-
- /// Returns underlying AsmPrinter.
- virtual AsmPrinter &getAsmPrinter() const = 0;
};
class DwarfStreamer;
@@ -249,10 +239,10 @@ class DWARFLinker : public DWARFLinkerBase {
StringsTranslator);
}
- Error createEmitter(const Triple &TheTriple, OutputFileType FileType,
- raw_pwrite_stream &OutFile);
-
- DwarfEmitter *getEmitter();
+ /// Set output DWARF emitter.
+ void setOutputDWARFEmitter(DwarfEmitter *Emitter) {
+ TheDwarfEmitter = Emitter;
+ }
/// Add object file to be linked. Pre-load compile unit die. Call
/// \p OnCUDieLoaded for each compile unit die. If specified \p File
@@ -779,7 +769,7 @@ class DWARFLinker : public DWARFLinkerBase {
BumpPtrAllocator DIEAlloc;
/// @}
- std::unique_ptr<DwarfStreamer> TheDwarfEmitter;
+ DwarfEmitter *TheDwarfEmitter = nullptr;
std::vector<LinkContext> ObjectContexts;
/// The CIEs that have been emitted in the output section. The actual CIE
diff --git a/llvm/include/llvm/DWARFLinker/Classic/DWARFStreamer.h b/llvm/include/llvm/DWARFLinker/Classic/DWARFStreamer.h
index f010c348f12148..bebdfd5e60257e 100644
--- a/llvm/include/llvm/DWARFLinker/Classic/DWARFStreamer.h
+++ b/llvm/include/llvm/DWARFLinker/Classic/DWARFStreamer.h
@@ -45,18 +45,23 @@ class DwarfStreamer : public DwarfEmitter {
public:
DwarfStreamer(DWARFLinkerBase::OutputFileType OutFileType,
raw_pwrite_stream &OutFile,
- std::function<StringRef(StringRef Input)> Translator,
+ DWARFLinkerBase::TranslatorFuncTy Translator,
DWARFLinkerBase::MessageHandlerTy Warning)
: OutFile(OutFile), OutFileType(OutFileType), Translator(Translator),
WarningHandler(Warning) {}
virtual ~DwarfStreamer() = default;
+ static Expected<std::unique_ptr<DwarfStreamer>> createStreamer(
+ const Triple &TheTriple, DWARFLinkerBase::OutputFileType FileType,
+ raw_pwrite_stream &OutFile, DWARFLinkerBase::TranslatorFuncTy Translator,
+ DWARFLinkerBase::MessageHandlerTy Warning);
+
Error init(Triple TheTriple, StringRef Swift5ReflectionSegmentName);
/// Dump the file to the disk.
void finish() override;
- AsmPrinter &getAsmPrinter() const override { return *Asm; }
+ AsmPrinter &getAsmPrinter() const { return *Asm; }
/// Set the current output section to debug_info and change
/// the MC Dwarf version to \p DwarfVersion.
@@ -77,7 +82,8 @@ class DwarfStreamer : public DwarfEmitter {
unsigned DwarfVersion) override;
/// Emit contents of section SecName From Obj.
- void emitSectionContents(StringRef SecData, StringRef SecName) override;
+ void emitSectionContents(StringRef SecData,
+ DebugSectionKind SecKind) override;
/// Emit the string table described by \p Pool into .debug_str table.
void emitStrings(const NonRelocatableStringpool &Pool) override;
@@ -91,12 +97,12 @@ class DwarfStreamer : public DwarfEmitter {
void emitLineStrings(const NonRelocatableStringpool &Pool) override;
/// Emit the swift_ast section stored in \p Buffer.
- void emitSwiftAST(StringRef Buffer) override;
+ void emitSwiftAST(StringRef Buffer);
/// Emit the swift reflection section stored in \p Buffer.
void emitSwiftReflectionSection(
llvm::binaryformat::Swift5ReflectionSectionKind ReflSectionKind,
- StringRef Buffer, uint32_t Alignment, uint32_t Size) override;
+ StringRef Buffer, uint32_t Alignment, uint32_t Size);
/// Emit debug ranges(.debug_ranges, .debug_rnglists) header.
MCSymbol *emitDwarfDebugRangeListHeader(const CompileUnit &Unit) override;
@@ -215,6 +221,8 @@ class DwarfStreamer : public DwarfEmitter {
WarningHandler(Warning, Context, nullptr);
}
+ MCSection *getMCSection(DebugSectionKind SecKind);
+
void emitMacroTableImpl(const DWARFDebugMacro *MacroTable,
const Offset2UnitMap &UnitMacroMap,
OffsetsStringPool &StringPool, uint64_t &OutOffset);
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinkerBase.h b/llvm/include/llvm/DWARFLinker/DWARFLinkerBase.h
index 626fb53d90f9fa..5c811b668f0a31 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinkerBase.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinkerBase.h
@@ -23,6 +23,53 @@ class DWARFUnit;
namespace dwarf_linker {
+/// List of tracked debug tables.
+enum class DebugSectionKind : uint8_t {
+ DebugInfo = 0,
+ DebugLine,
+ DebugFrame,
+ DebugRange,
+ DebugRngLists,
+ DebugLoc,
+ DebugLocLists,
+ DebugARanges,
+ DebugAbbrev,
+ DebugMacinfo,
+ DebugMacro,
+ DebugAddr,
+ DebugStr,
+ DebugLineStr,
+ DebugStrOffsets,
+ DebugPubNames,
+ DebugPubTypes,
+ DebugNames,
+ AppleNames,
+ AppleNamespaces,
+ AppleObjC,
+ AppleTypes,
+ NumberOfEnumEntries // must be last
+};
+
+static constexpr size_t SectionKindsNum =
+ static_cast<size_t>(DebugSectionKind::NumberOfEnumEntries);
+
+static constexpr StringLiteral SectionNames[SectionKindsNum] = {
+ "debug_info", "debug_line", "debug_frame", "debug_ranges",
+ "debug_rnglists", "debug_loc", "debug_loclists", "debug_aranges",
+ "debug_abbrev", "debug_macinfo", "debug_macro", "debug_addr",
+ "debug_str", "debug_line_str", "debug_str_offsets", "debug_pubnames",
+ "debug_pubtypes", "debug_names", "apple_names", "apple_namespac",
+ "apple_objc", "apple_types"};
+
+/// Return the name of the section.
+static constexpr const StringLiteral &
+getSectionName(DebugSectionKind SectionKind) {
+ return SectionNames[static_cast<uint8_t>(SectionKind)];
+}
+
+/// Recognise the table name and match it with the DebugSectionKind.
+std::optional<DebugSectionKind> parseDebugTableName(StringRef Name);
+
/// The base interface for DWARFLinker implementations.
class DWARFLinkerBase {
public:
diff --git a/llvm/include/llvm/DWARFLinker/Parallel/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/Parallel/DWARFLinker.h
index c38a9906940ec5..99506e28a6ae61 100644
--- a/llvm/include/llvm/DWARFLinker/Parallel/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/Parallel/DWARFLinker.h
@@ -89,30 +89,34 @@ namespace llvm {
namespace dwarf_linker {
namespace parallel {
-/// ExtraDwarfEmitter allows adding extra data to the DWARFLinker output.
-/// The finish() method should be called after all extra data are emitted.
-class ExtraDwarfEmitter {
-public:
- virtual ~ExtraDwarfEmitter() = default;
-
- /// Dump the file to the disk.
- virtual void finish() = 0;
-
- /// Emit section named SecName with data SecData.
- virtual void emitSectionContents(StringRef SecData, StringRef SecName) = 0;
-
- /// Emit the swift_ast section stored in \p Buffer.
- virtual void emitSwiftAST(StringRef Buffer) = 0;
-
- /// Emit the swift reflection section stored in \p Buffer.
- virtual void emitSwiftReflectionSection(
- llvm::binaryformat::Swift5ReflectionSectionKind ReflSectionKind,
- StringRef Buffer, uint32_t Alignment, uint32_t Size) = 0;
-
- /// Returns underlying AsmPrinter.
- virtual AsmPrinter &getAsmPrinter() const = 0;
+/// This structure keeps data of the concrete section.
+struct SectionDescriptorBase {
+ SectionDescriptorBase(DebugSectionKind SectionKind, dwarf::FormParams Format,
+ llvm::endianness Endianess)
+ : SectionKind(SectionKind), Format(Format), Endianess(Endianess) {}
+ virtual ~SectionDescriptorBase() = default;
+ /// Returns section content.
+ virtual StringRef getContents() = 0;
+ /// Returns section kind.
+ DebugSectionKind getKind() { return SectionKind; }
+ /// Returns section name.
+ const StringLiteral &getName() const { return getSectionName(SectionKind); }
+ /// Returns endianess used by section.
+ llvm::endianness getEndianess() const { return Endianess; }
+ /// Returns FormParams used by section.
+ dwarf::FormParams getFormParams() const { return Format; }
+
+protected:
+ /// The section kind.
+ DebugSectionKind SectionKind = DebugSectionKind::NumberOfEnumEntries;
+ /// Output format.
+ dwarf::FormParams Format = {4, 4, dwarf::DWARF32};
+ llvm::endianness Endianess = llvm::endianness::little;
};
+using SectionHandlerTy =
+ std::function<void(std::shared_ptr<SectionDescriptorBase> Section)>;
+
class DWARFLinker : public DWARFLinkerBase {
public:
virtual ~DWARFLinker() = default;
@@ -122,12 +126,9 @@ class DWARFLinker : public DWARFLinkerBase {
createLinker(MessageHandlerTy ErrorHandler, MessageHandlerTy WarningHandler,
TranslatorFuncTy StringsTranslator = nullptr);
- /// Creates emitter for output dwarf.
- virtual Error createEmitter(const Triple &TheTriple, OutputFileType FileType,
- raw_pwrite_stream &OutFile) = 0;
-
- /// Returns previously created dwarf emitter. May be nullptr.
- virtual ExtraDwarfEmitter *getEmitter() = 0;
+ /// Set output DWARF handler.
+ virtual void setOutputDWARFHandler(const Triple &TargetTriple,
+ SectionHandlerTy SectionHandler) = 0;
};
} // end of namespace parallel
diff --git a/llvm/lib/DWARFLinker/CMakeLists.txt b/llvm/lib/DWARFLinker/CMakeLists.txt
index 73055a96d4a947..c0568c42b0141b 100644
--- a/llvm/lib/DWARFLinker/CMakeLists.txt
+++ b/llvm/lib/DWARFLinker/CMakeLists.txt
@@ -1,4 +1,5 @@
add_llvm_component_library(LLVMDWARFLinkerBase
+ DWARFLinkerBase.cpp
Utils.cpp
ADDITIONAL_HEADER_DIRS
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
index ac2c26e5224078..e35febf205dfa8 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
@@ -2644,19 +2644,22 @@ uint64_t DWARFLinker::DIECloner::cloneAllCompileUnits(
void DWARFLinker::copyInvariantDebugSection(DWARFContext &Dwarf) {
TheDwarfEmitter->emitSectionContents(Dwarf.getDWARFObj().getLocSection().Data,
- "debug_loc");
+ DebugSectionKind::DebugLoc);
TheDwarfEmitter->emitSectionContents(
- Dwarf.getDWARFObj().getRangesSection().Data, "debug_ranges");
+ Dwarf.getDWARFObj().getRangesSection().Data,
+ DebugSectionKind::DebugRange);
TheDwarfEmitter->emitSectionContents(
- Dwarf.getDWARFObj().getFrameSection().Data, "debug_frame");
+ Dwarf.getDWARFObj().getFrameSection().Data, DebugSectionKind::DebugFrame);
TheDwarfEmitter->emitSectionContents(Dwarf.getDWARFObj().getArangesSection(),
- "debug_aranges");
+ DebugSectionKind::DebugARanges);
TheDwarfEmitter->emitSectionContents(
- Dwarf.getDWARFObj().getAddrSection().Data, "debug_addr");
+ Dwarf.getDWARFObj().getAddrSection().Data, DebugSectionKind::DebugAddr);
TheDwarfEmitter->emitSectionContents(
- Dwarf.getDWARFObj().getRnglistsSection().Data, "debug_rnglists");
+ Dwarf.getDWARFObj().getRnglistsSection().Data,
+ DebugSectionKind::DebugRngLists);
TheDwarfEmitter->emitSectionContents(
- Dwarf.getDWARFObj().getLoclistsSection().Data, "debug_loclists");
+ Dwarf.getDWARFObj().getLoclistsSection().Data,
+ DebugSectionKind::DebugLocLists);
}
void DWARFLinker::addObjectFile(DWARFFile &File, ObjFileLoaderTy Loader,
@@ -2848,7 +2851,7 @@ Error DWARFLinker::link() {
SizeByObject[OptContext.File.FileName].Input =
getDebugInfoSize(*OptContext.File.Dwarf);
SizeByObject[OptContext.File.FileName].Output =
- DIECloner(*this, TheDwarfEmitter.get(), OptContext.File, DIEAlloc,
+ DIECloner(*this, TheDwarfEmitter, OptContext.File, DIEAlloc,
OptContext.CompileUnits, Options.Update, DebugStrPool,
DebugLineStrPool, StringOffsetPool)
.cloneAllCompileUnits(*OptContext.File.Dwarf, OptContext.File,
@@ -3011,7 +3014,7 @@ Error DWARFLinker::cloneModuleUnit(LinkContext &Context, RefModuleUnit &Unit,
UnitListTy CompileUnits;
CompileUnits.emplace_back(std::move(Unit.Unit));
assert(TheDwarfEmitter);
- DIECloner(*this, TheDwarfEmitter.get(), Unit.File, DIEAlloc, CompileUnits,
+ DIECloner(*this, TheDwarfEmitter, Unit.File, DIEAlloc, CompileUnits,
Options.Update, DebugStrPool, DebugLineStrPool, StringOffsetPool)
.cloneAllCompileUnits(*Unit.File.Dwarf, Unit.File,
Unit.File.Dwarf->isLittleEndian());
@@ -3030,16 +3033,4 @@ void DWARFLinker::verifyInput(const DWARFFile &File) {
}
}
-Error DWARFLinker::createEmitter(const Triple &TheTriple,
- OutputFileType FileType,
- raw_pwrite_stream &OutFile) {
-
- TheDwarfEmitter = std::make_unique<DwarfStreamer>(
- FileType, OutFile, StringsTranslator, WarningHandler);
-
- return TheDwarfEmitter->init(TheTriple, "__DWARF");
-}
-
-DwarfEmitter *DWARFLinker::getEmitter() { return TheDwarfEmitter.get(); }
-
} // namespace llvm
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFStreamer.cpp b/llvm/lib/DWARFLinker/Classic/DWARFStreamer.cpp
index 020bbb06449d3a..de14b4b2eb6c5f 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFStreamer.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFStreamer.cpp
@@ -30,6 +30,20 @@ using namespace llvm;
using namespace dwarf_linker;
using namespace dwarf_linker::classic;
+Expected<std::unique_ptr<DwarfStreamer>> DwarfStreamer::createStreamer(
+ const Triple &TheTriple, DWARFLinkerBase::OutputFileType FileType,
+ raw_pwrite_stream &OutFile, DWARFLinkerBase::TranslatorFuncTy Translator,
+ DWARFLinkerBase::MessageHandlerTy Warning) {
+ DwarfStreamer *Streamer =
+ new DwarfStreamer(FileType, OutFile, Translator, Warning);
+ if (Error Err = Streamer->init(TheTriple, "__DWARF")) {
+ delete Streamer;
+ return std::move(Err);
+ }
+
+ return std::unique_ptr<DwarfStreamer>(Streamer);
+}
+
Error DwarfStreamer::init(Triple TheTriple,
StringRef Swift5ReflectionSegmentName) {
std::string ErrorStr;
@@ -212,30 +226,72 @@ void DwarfStreamer::emitDIE(DIE &Die) {
}
/// Emit contents of section SecName From Obj.
-void DwarfStreamer::emitSectionContents(StringRef SecData, StringRef SecName) {
- MCSection *Section =
- StringSwitch<MCSection *>(SecName)
- .Case("debug_line", MC->getObjectFileInfo()->getDwarfLineSection())
- .Case("debug_loc", MC->getObjectFileInfo()->getDwarfLocSection())
- .Case("debug_ranges",
- MC->getObjectFileInfo()->getDwarfRangesSection())
- .Case("debug_frame", MC->getObjectFileInfo()->getDwarfFrameSection())
- .Case("debug_aranges",
- MC->getObjectFileInfo()->getDwarfARangesSection())
- .Case("debug_addr", MC->getObjectFileInfo()->getDwarfAddrSection())
- .Case("debug_rnglists",
- MC->getObjectFileInfo()->getDwarfRnglistsSection())
- .Case("debug_loclists",
- MC->getObjectFileInfo()->getDwarfLoclistsSection())
- .Default(nullptr);
-
- if (Section) {
+void DwarfStreamer::emitSectionContents(StringRef SecData,
+ DebugSectionKind SecKind) {
+ if (SecData.empty())
+ return;
+
+ if (MCSection *Section = getMCSection(SecKind)) {
MS->switchSection(Section);
MS->emitBytes(SecData);
}
}
+MCSection *DwarfStreamer::getMCSection(DebugSectionKind SecKind) {
+ switch (SecKind) {
+ case DebugSectionKind::DebugInfo:
+ return MC->getObjectFileInfo()->getDwarfInfoSection();
+ case DebugSectionKind::DebugLine:
+ return MC->getObjectFileInfo()->getDwarfLineSection();
+ case DebugSectionKind::DebugFrame:
+ return MC->getObjectFileInfo()->getDwarfFrameSection();
+ case DebugSectionKind::DebugRange:
+ return MC->getObjectFileInfo()->getDwarfRangesSection();
+ case DebugSectionKind::DebugRngLists:
+ return MC->getObjectFileInfo()->getDwarfRnglistsSection();
+ case DebugSectionKind::DebugLoc:
+ return MC->getObjectFileInfo()->getDwarfLocSection();
+ case DebugSectionKind::DebugLocLists:
+ return MC->getObjectFileInfo()->getDwarfLoclistsSection();
+ case DebugSectionKind::DebugARanges:
+ return MC->getObjectFileInfo()->getDwarfARangesSection();
+ case DebugSectionKind::DebugAbbrev:
+ return MC->getObjectFileInfo()->getDwarfAbbrevSection();
+ case DebugSectionKind::DebugMacinfo:
+ return MC->getObjectFileInfo()->getDwarfMacinfoSection();
+ case DebugSectionKind::DebugMacro:
+ return MC->getObjectFileInfo()->getDwarfMacroSection();
+ case DebugSectionKind::DebugAddr:
+ return MC->getObjectFileInfo()->getDwarfAddrSection();
+ case DebugSectionKind::DebugStr:
+ return MC->getObjectFileInfo()->getDwarfStrSection();
+ case DebugSectionKind::DebugLineStr:
+ return MC->getObjectFileInfo()->getDwarfLineStrSection();
+ case DebugSectionKind::DebugStrOffsets:
+ return MC->getObjectFileInfo()->getDwarfStrOffSection();
+ case DebugSectionKind::DebugPubNames:
+ return MC->getObjectFileInfo()->getDwarfPubNamesSection();
+ case DebugSectionKind::DebugPubTypes:
+ return MC->getObjectFileInfo()->getDwarfPubTypesSection();
+ case DebugSectionKind::DebugNames:
+ return MC->getObjectFileInfo()->getDwarfDebugNamesSection();
+ case DebugSectionKind::AppleNames:
+ return MC->getObjectFileInfo()->getDwarfAccelNamesSection();
+ case DebugSectionKind::AppleNamespaces:
+ return MC->getObjectFileInfo()->getDwarfAccelNamespaceSection();
+ case DebugSectionKind::AppleObjC:
+ return MC->getObjectFileInfo()->getDwarfAccelObjCSection();
+ case DebugSectionKind::AppleTypes:
+ return MC->getObjectFileInfo()->getDwarfAccelTypesSection();
+ case DebugSectionKind::NumberOfEnumEntries:
+ llvm_unreachable("Unknown DebugSectionKind value");
+ break;
+ }
+
+ return nullptr;
+}
+
/// Emit the debug_str section stored in \p Pool.
void DwarfStreamer::emitStrings(const NonRelocatableStringpool &Pool) {
Asm->OutStreamer->switchSection(MOFI->getDwarfStrSection());
diff --git a/llvm/lib/DWARFLinker/DWARFLinkerBase.cpp b/llvm/lib/DWARFLinker/DWARFLinkerBase.cpp
new file mode 100644
index 00000000000000..7ba98d43274fa3
--- /dev/null
+++ b/llvm/lib/DWARFLinker/DWARFLinkerBase.cpp
@@ -0,0 +1,64 @@
+//=== DWARFLinkerBase.cpp -------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DWARFLinker/DWARFLinkerBase.h"
+#include "llvm/ADT/StringSwitch.h"
+
+using namespace llvm;
+using namespace llvm::dwarf_linker;
+
+std::optional<DebugSectionKind>
+llvm::dwarf_linker::parseDebugTableName(llvm::StringRef SecName) {
+ return llvm::StringSwitch<std::optional<DebugSectionKind>>(
+ SecName.substr(SecName.find_first_not_of("._")))
+ .Case(getSectionName(DebugSectionKind::DebugInfo),
+ DebugSectionKind::DebugInfo)
+ .Case(getSectionName(DebugSectionKind::DebugLine),
+ DebugSectionKind::DebugLine)
+ .Case(getSect...
[truncated]
|
JDevlieghere
approved these changes
Jan 17, 2024
There was a problem hiding this comment.
Before this patch, the DWARFLinker was the one responsible for creating the streamer, which meant that both the classic and parallel implementation needed to conform to the same interface. After this patch, the concrete implementations instantiate their own streamer, which means that the classic linker can support the Swift sections while the parallel implementation can ignore them for now.
Makes sense to me. LGTM!
Comment on lines
37
to
44
| DwarfStreamer *Streamer = | ||
| new DwarfStreamer(FileType, OutFile, Translator, Warning); | ||
| if (Error Err = Streamer->init(TheTriple, "__DWARF")) { | ||
| delete Streamer; | ||
| return std::move(Err); | ||
| } | ||
|
|
||
| return std::unique_ptr<DwarfStreamer>(Streamer); |
There was a problem hiding this comment.
Can we use make_unique above and avoid the naked delete?
This patch is extracted from llvm#74725. The DwarfStreamer interface looks overcomplicated and has unnecessary dependencies: 1. it contains emitSwiftAST, emitSwiftReflectionSection methods which are not used by DWARFLinker. 2. its interface uses DWARFLinker classes(CompileUnit) which makes it dependend on DWARFLinker. 3. it has "AsmPrinter &getAsmPrinter()" method which provides very low level interface. This patch avoids creation of DwarfStreamer by DWARFLinker and simplifies interface: 1. dwarf_linker::classic. Now client of DWARFLinker creates DwarfStreamer and pass it to the DWARFLinker through DwarfEmitter interface. It simplifies dependence. Later it would be good to remove class DwarfStreamer from dwarf_linker::classic completely. 2. dwarf_linker::parallel. Now client of DWARFLinker sets handler of output debug sections to the DWARFLinker. It simplifies dependence to following small interface: using SectionHandlerTy = std::function<void(std::shared_ptr<SectionDescriptorBase> Section)>; virtual void setOutputDWARFHandler(const Triple &TargetTriple, SectionHandlerTy SectionHandler) = 0;
avl-llvm
force-pushed
the
avl-llvm/dwarfstreamer-dependence
branch
from
5883f6e to
15144b8
Compare
|
addressed comments. |
felipepiovezan
pushed a commit
to felipepiovezan/llvm-project
that referenced
this pull request
Feb 2, 2024
…lvm#77932) This patch is extracted from llvm#74725. The DwarfStreamer interface looks overcomplicated and has unnecessary dependencies. This patch avoids creation of DwarfStreamer by DWARFLinker and simplifies interface. (cherry picked from commit 9ff4be6)
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.
This patch is extracted from #74725.
The DwarfStreamer interface looks overcomplicated and has unnecessary dependencies:
it contains emitSwiftAST, emitSwiftReflectionSection methods which are not used by DWARFLinker.
its interface uses DWARFLinker classes(CompileUnit) which makes it dependend on DWARFLinker.
it has "AsmPrinter &getAsmPrinter()" method which provides very low level interface.
This patch avoids creation of DwarfStreamer by DWARFLinker and simplifies interface:
dwarf_linker::classic.
Now client of DWARFLinker creates DwarfStreamer and pass it to the DWARFLinker through DwarfEmitter interface. It simplifies dependence. Later it would be good to remove class DwarfStreamer from dwarf_linker::classic completely.
dwarf_linker::parallel.
Now client of DWARFLinker sets handler of output debug sections to the DWARFLinker. It simplifies dependence to following small interface: