[GOFF] Introduce GOFFWriter class

[redstar]

The GOFFWriter has 2 purposes:

• Simplify resource management
• Enable writing of split DWARF files

It follows the design of the other writer classes. No added functionality at this point.

[llvmbot]

@llvm/pr-subscribers-mc

Author: Kai Nacke (redstar)

Changes

The GOFFWriter has 2 purposes:

• Simplify resource management
• Enable writing of split DWARF files

It follows the design of the other writer classes. No added functionality at this point.

---

Full diff: https://github.com/llvm/llvm-project/pull/131216.diff

1 Files Affected:

• (modified) llvm/lib/MC/GOFFObjectWriter.cpp (+41-23)

diff --git a/llvm/lib/MC/GOFFObjectWriter.cpp b/llvm/lib/MC/GOFFObjectWriter.cpp
index 85deebd89d1f6..4ee8e1487751f 100644
--- a/llvm/lib/MC/GOFFObjectWriter.cpp
+++ b/llvm/lib/MC/GOFFObjectWriter.cpp
@@ -223,34 +223,23 @@ void GOFFOstream::finalizeRecord() {
 }
 
 namespace {
-
-class GOFFObjectWriter : public MCObjectWriter {
-  // The target specific GOFF writer instance.
-  std::unique_ptr<MCGOFFObjectTargetWriter> TargetObjectWriter;
-
-  // The stream used to write the GOFF records.
+class GOFFWriter {
   GOFFOstream OS;
+  [[maybe_unused]] MCAssembler &Asm;
 
-public:
-  GOFFObjectWriter(std::unique_ptr<MCGOFFObjectTargetWriter> MOTW,
-                   raw_pwrite_stream &OS)
-      : TargetObjectWriter(std::move(MOTW)), OS(OS) {}
-
-  ~GOFFObjectWriter() override {}
-
-  // Write GOFF records.
   void writeHeader();
   void writeEnd();
 
-  // Implementation of the MCObjectWriter interface.
-  void recordRelocation(MCAssembler &Asm, const MCFragment *Fragment,
-                        const MCFixup &Fixup, MCValue Target,
-                        uint64_t &FixedValue) override {}
-  uint64_t writeObject(MCAssembler &Asm) override;
+public:
+  GOFFWriter(raw_pwrite_stream &OS, MCAssembler &Asm);
+  uint64_t writeObject();
 };
-} // end anonymous namespace
+} // namespace
+
+GOFFWriter::GOFFWriter(raw_pwrite_stream &OS, MCAssembler &Asm)
+    : OS(OS), Asm(Asm) {}
 
-void GOFFObjectWriter::writeHeader() {
+void GOFFWriter::writeHeader() {
   OS.newRecord(GOFF::RT_HDR);
   OS.write_zeros(1);       // Reserved
   OS.writebe<uint32_t>(0); // Target Hardware Environment
@@ -264,7 +253,7 @@ void GOFFObjectWriter::writeHeader() {
   OS.write_zeros(6);       // Reserved
 }
 
-void GOFFObjectWriter::writeEnd() {
+void GOFFWriter::writeEnd() {
   uint8_t F = GOFF::END_EPR_None;
   uint8_t AMODE = 0;
   uint32_t ESDID = 0;
@@ -282,7 +271,7 @@ void GOFFObjectWriter::writeEnd() {
   OS.writebe<uint32_t>(ESDID); // ESDID (of entry point)
 }
 
-uint64_t GOFFObjectWriter::writeObject(MCAssembler &Asm) {
+uint64_t GOFFWriter::writeObject() {
   writeHeader();
   writeEnd();
 
@@ -295,6 +284,35 @@ uint64_t GOFFObjectWriter::writeObject(MCAssembler &Asm) {
   return OS.getWrittenSize();
 }
 
+namespace {
+
+class GOFFObjectWriter : public MCObjectWriter {
+  // The target specific GOFF writer instance.
+  std::unique_ptr<MCGOFFObjectTargetWriter> TargetObjectWriter;
+
+  // The stream used to write the GOFF records.
+  raw_pwrite_stream &OS;
+
+public:
+  GOFFObjectWriter(std::unique_ptr<MCGOFFObjectTargetWriter> MOTW,
+                   raw_pwrite_stream &OS)
+      : TargetObjectWriter(std::move(MOTW)), OS(OS) {}
+
+  ~GOFFObjectWriter() override {}
+
+  // Implementation of the MCObjectWriter interface.
+  void recordRelocation(MCAssembler &Asm, const MCFragment *Fragment,
+                        const MCFixup &Fixup, MCValue Target,
+                        uint64_t &FixedValue) override {}
+  uint64_t writeObject(MCAssembler &Asm) override;
+};
+} // end anonymous namespace
+
+uint64_t GOFFObjectWriter::writeObject(MCAssembler &Asm) {
+  uint64_t Size = GOFFWriter(OS, Asm).writeObject();
+  return Size;
+}
+
 std::unique_ptr<MCObjectWriter>
 llvm::createGOFFObjectWriter(std::unique_ptr<MCGOFFObjectTargetWriter> MOTW,
                              raw_pwrite_stream &OS) {

[MaskRay]

If you will change AsmPrinter or a targetstreamer to be aware of GOFF-specifics, it would be better to expose the class to a header.

In 70c52b6 , I exported ELF and enabled a lot of simplification to MCAssembler/MCELFStreamer.

[redstar]

That is a good point. I think we can also make use of a public available GOFFWriter. Only downside is that it pulls a couple of internal definitions into the header file, but that is the same as in the ELF implementation. Thanks for pointing that out.

[redstar]

Ok, with the 2nd try it should now be correct....

[redstar]

Is the change ok?

[redstar]

More comments on this PR?

[redstar]

Gentle ping. :-)

[redstar]

Gentle ping :-)

[uweigand]

Hi @redstar, looks like this no longer merges cleanly and has test failures now.

[redstar]

I'll rebase all the PRs...

[redstar]

Hopefully this gives a clean build. Ther merge conflict & build failures were caused by the removal of the Asm parameter in recordRelocation() and writeObject().

[uweigand]

This refactoring brings GOFF in line with other formats. LGTM.