[GOFF] Add writing of text records #137235

redstar · 2025-04-24T18:57:28Z

Sections which are not allowed to carry data are marked as virtual. Only complication when writing out the text is that it must be written in chunks of 32k-1 bytes, which is done by having a wrapper stream writing those records.
Data of BSS sections is not written, since the contents is known to be zero. Instead, the fill byte value is used.

llvmbot · 2025-04-24T18:57:49Z

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-systemz

Author: Kai Nacke (redstar)

Changes

Sections which are not allowed to carry data are marked as virtual. Only complication when writing out the text is that it must be written in chunks of 32k-1 bytes, which is done by having a wrapper stream writing those records.
Data of BSS sections is not written, since the contents is known to be zero. Instead, the fill byte value is used.

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

8 Files Affected:

(modified) llvm/include/llvm/MC/MCContext.h (+4-2)
(modified) llvm/include/llvm/MC/MCSectionGOFF.h (+32-15)
(modified) llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (+3-2)
(modified) llvm/lib/MC/GOFFObjectWriter.cpp (+78)
(modified) llvm/lib/MC/MCContext.cpp (+12-7)
(modified) llvm/lib/MC/MCObjectFileInfo.cpp (+4-4)
(modified) llvm/test/CodeGen/SystemZ/zos-section-1.ll (+22-5)
(modified) llvm/test/CodeGen/SystemZ/zos-section-2.ll (+25-5)

diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h
index 8eb904966f4de..2cdba64be116d 100644
--- a/llvm/include/llvm/MC/MCContext.h
+++ b/llvm/include/llvm/MC/MCContext.h
@@ -366,7 +366,8 @@ class MCContext {
 
   template <typename TAttr>
   MCSectionGOFF *getGOFFSection(SectionKind Kind, StringRef Name,
-                                TAttr SDAttributes, MCSection *Parent);
+                                TAttr SDAttributes, MCSection *Parent,
+                                bool IsVirtual);
 
   /// Map of currently defined macros.
   StringMap<MCAsmMacro> MacroMap;
@@ -607,7 +608,8 @@ class MCContext {
   MCSectionGOFF *getGOFFSection(SectionKind Kind, StringRef Name,
                                 GOFF::SDAttr SDAttributes);
   MCSectionGOFF *getGOFFSection(SectionKind Kind, StringRef Name,
-                                GOFF::EDAttr EDAttributes, MCSection *Parent);
+                                GOFF::EDAttr EDAttributes, MCSection *Parent,
+                                bool IsVirtual);
   MCSectionGOFF *getGOFFSection(SectionKind Kind, StringRef Name,
                                 GOFF::PRAttr PRAttributes, MCSection *Parent);
 
diff --git a/llvm/include/llvm/MC/MCSectionGOFF.h b/llvm/include/llvm/MC/MCSectionGOFF.h
index b8b8cf112a34d..b2ca74c3ba78a 100644
--- a/llvm/include/llvm/MC/MCSectionGOFF.h
+++ b/llvm/include/llvm/MC/MCSectionGOFF.h
@@ -39,6 +39,9 @@ class MCSectionGOFF final : public MCSection {
   // The type of this section.
   GOFF::ESDSymbolType SymbolType;
 
+  // This section is a BSS section.
+  unsigned IsBSS : 1;
+
   // Indicates that the PR symbol needs to set the length of the section to a
   // non-zero value. This is only a problem with the ADA PR - the binder will
   // generate an error in this case.
@@ -50,26 +53,26 @@ class MCSectionGOFF final : public MCSection {
   friend class MCContext;
   friend class MCSymbolGOFF;
 
-  MCSectionGOFF(StringRef Name, SectionKind K, GOFF::SDAttr SDAttributes,
-                MCSectionGOFF *Parent)
-      : MCSection(SV_GOFF, Name, K.isText(), /*IsVirtual=*/false, nullptr),
+  MCSectionGOFF(StringRef Name, SectionKind K, bool IsVirtual,
+                GOFF::SDAttr SDAttributes, MCSectionGOFF *Parent)
+      : MCSection(SV_GOFF, Name, K.isText(), IsVirtual, nullptr),
         Parent(Parent), SDAttributes(SDAttributes),
-        SymbolType(GOFF::ESD_ST_SectionDefinition), RequiresNonZeroLength(0),
-        Emitted(0) {}
+        SymbolType(GOFF::ESD_ST_SectionDefinition), IsBSS(K.isBSS()),
+        RequiresNonZeroLength(0), Emitted(0) {}
 
-  MCSectionGOFF(StringRef Name, SectionKind K, GOFF::EDAttr EDAttributes,
-                MCSectionGOFF *Parent)
-      : MCSection(SV_GOFF, Name, K.isText(), /*IsVirtual=*/false, nullptr),
+  MCSectionGOFF(StringRef Name, SectionKind K, bool IsVirtual,
+                GOFF::EDAttr EDAttributes, MCSectionGOFF *Parent)
+      : MCSection(SV_GOFF, Name, K.isText(), IsVirtual, nullptr),
         Parent(Parent), EDAttributes(EDAttributes),
-        SymbolType(GOFF::ESD_ST_ElementDefinition), RequiresNonZeroLength(0),
-        Emitted(0) {}
+        SymbolType(GOFF::ESD_ST_ElementDefinition), IsBSS(K.isBSS()),
+        RequiresNonZeroLength(0), Emitted(0) {}
 
-  MCSectionGOFF(StringRef Name, SectionKind K, GOFF::PRAttr PRAttributes,
-                MCSectionGOFF *Parent)
-      : MCSection(SV_GOFF, Name, K.isText(), /*IsVirtual=*/false, nullptr),
+  MCSectionGOFF(StringRef Name, SectionKind K, bool IsVirtual,
+                GOFF::PRAttr PRAttributes, MCSectionGOFF *Parent)
+      : MCSection(SV_GOFF, Name, K.isText(), IsVirtual, nullptr),
         Parent(Parent), PRAttributes(PRAttributes),
-        SymbolType(GOFF::ESD_ST_PartReference), RequiresNonZeroLength(0),
-        Emitted(0) {}
+        SymbolType(GOFF::ESD_ST_PartReference), IsBSS(K.isBSS()),
+        RequiresNonZeroLength(0), Emitted(0) {}
 
 public:
   void printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
@@ -81,6 +84,9 @@ class MCSectionGOFF final : public MCSection {
   // Return the parent section.
   MCSectionGOFF *getParent() const { return Parent; }
 
+  // Returns true if this is a BSS section.
+  bool isBSS() const { return IsBSS; }
+
   // Returns the type of this section.
   GOFF::ESDSymbolType getSymbolType() const { return SymbolType; }
 
@@ -102,6 +108,17 @@ class MCSectionGOFF final : public MCSection {
     return PRAttributes;
   }
 
+  // Returns the text style for a section. Only defined for ED and PR sections.
+  GOFF::ESDTextStyle getTextStyle() const {
+    assert(isED() || isPR() || isVirtualSection() && "Expect ED or PR section");
+    if (isED())
+      return EDAttributes.TextStyle;
+    if (isPR())
+      return getParent()->getEDAttributes().TextStyle;
+    // Virtual sections have no data, so byte orientation is fine.
+    return GOFF::ESD_TS_ByteOriented;
+  }
+
   bool requiresNonZeroLength() const { return RequiresNonZeroLength; }
 
   void setName(StringRef SectionName) { Name = SectionName; }
diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 5460c84beffc1..91e2c25809280 100644
--- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -2801,7 +2801,8 @@ MCSection *TargetLoweringObjectFileGOFF::getSectionForLSDA(
                    GOFF::ESD_TS_ByteOriented, GOFF::ESD_BA_Merge,
                    GOFF::LOADBEHAVIOR, GOFF::ESD_RQ_0,
                    GOFF::ESD_ALIGN_Doubleword},
-      static_cast<MCSectionGOFF *>(TextSection)->getParent());
+      static_cast<MCSectionGOFF *>(TextSection)->getParent(),
+      /*IsVirtual=*/true);
   return getContext().getGOFFSection(
       SectionKind::getData(), Name,
       GOFF::PRAttr{true, false, GOFF::ESD_EXE_DATA, GOFF::LINKAGE,
@@ -2833,7 +2834,7 @@ MCSection *TargetLoweringObjectFileGOFF::SelectSectionForGlobal(
         GOFF::EDAttr{false, GOFF::RMODE, GOFF::ESD_NS_Parts,
                      GOFF::ESD_TS_ByteOriented, GOFF::ESD_BA_Merge,
                      GOFF::ESD_LB_Deferred, GOFF::ESD_RQ_0, Align},
-        SD);
+        SD, /*IsVirtual=*/true);
     return getContext().getGOFFSection(
         Kind, Symbol->getName(),
         GOFF::PRAttr{false, false, GOFF::ESD_EXE_DATA, GOFF::LINKAGE,
diff --git a/llvm/lib/MC/GOFFObjectWriter.cpp b/llvm/lib/MC/GOFFObjectWriter.cpp
index 8bedc08c3066f..119cab6e6ef7d 100644
--- a/llvm/lib/MC/GOFFObjectWriter.cpp
+++ b/llvm/lib/MC/GOFFObjectWriter.cpp
@@ -276,6 +276,7 @@ class GOFFWriter {
 
   void writeHeader();
   void writeSymbol(const GOFFSymbol &Symbol);
+  void writeText(const MCSectionGOFF *MC);
   void writeEnd();
 
   void defineSectionSymbols(const MCSectionGOFF &Section);
@@ -407,6 +408,80 @@ void GOFFWriter::writeSymbol(const GOFFSymbol &Symbol) {
   OS.write(Name.data(), NameLength); // Name
 }
 
+namespace {
+/// Adapter stream to write a text section.
+class TextStream : public raw_ostream {
+  /// The underlying GOFFOstream.
+  GOFFOstream &OS;
+
+  /// The buffer size is the maximum number of bytes in a TXT section.
+  static constexpr size_t BufferSize = GOFF::MaxDataLength;
+
+  /// Static allocated buffer for the stream, used by the raw_ostream class. The
+  /// buffer is sized to hold the payload of a logical TXT record.
+  char Buffer[BufferSize];
+
+  /// The offset for the next TXT record. This is equal to the number of bytes
+  /// written.
+  size_t Offset;
+
+  /// The Esdid of the GOFF section.
+  const uint32_t EsdId;
+
+  /// The record style.
+  const GOFF::ESDTextStyle RecordStyle;
+
+  /// See raw_ostream::write_impl.
+  void write_impl(const char *Ptr, size_t Size) override;
+
+  uint64_t current_pos() const override { return Offset; }
+
+public:
+  explicit TextStream(GOFFOstream &OS, uint32_t EsdId,
+                      GOFF::ESDTextStyle RecordStyle)
+      : OS(OS), Offset(0), EsdId(EsdId), RecordStyle(RecordStyle) {
+    SetBuffer(Buffer, sizeof(Buffer));
+  }
+
+  ~TextStream() { flush(); }
+};
+
+void TextStream::write_impl(const char *Ptr, size_t Size) {
+  size_t WrittenLength = 0;
+
+  // We only have signed 32bits of offset.
+  if (Offset + Size > std::numeric_limits<int32_t>::max())
+    report_fatal_error("TXT section too large");
+
+  while (WrittenLength < Size) {
+    size_t ToWriteLength =
+        std::min(Size - WrittenLength, size_t(GOFF::MaxDataLength));
+
+    OS.newRecord(GOFF::RT_TXT);
+    OS.writebe<uint8_t>(GOFF::Flags(4, 4, RecordStyle)); // Text Record Style
+    OS.writebe<uint32_t>(EsdId);                         // Element ESDID
+    OS.writebe<uint32_t>(0);                             // Reserved
+    OS.writebe<uint32_t>(static_cast<uint32_t>(Offset)); // Offset
+    OS.writebe<uint32_t>(0);                      // Text Field True Length
+    OS.writebe<uint16_t>(0);                      // Text Encoding
+    OS.writebe<uint16_t>(ToWriteLength);          // Data Length
+    OS.write(Ptr + WrittenLength, ToWriteLength); // Data
+
+    WrittenLength += ToWriteLength;
+    Offset += ToWriteLength;
+  }
+}
+} // namespace
+
+void GOFFWriter::writeText(const MCSectionGOFF *Section) {
+  // A BSS section contains only zeros, no need to write this.
+  if (Section->isBSS())
+    return;
+
+  TextStream S(OS, Section->getOrdinal(), Section->getTextStyle());
+  Asm.writeSectionData(S, Section);
+}
+
 void GOFFWriter::writeEnd() {
   uint8_t F = GOFF::END_EPR_None;
   uint8_t AMODE = 0;
@@ -430,6 +505,9 @@ uint64_t GOFFWriter::writeObject() {
 
   defineSymbols();
 
+  for (const MCSection &Section : Asm)
+    writeText(static_cast<const MCSectionGOFF*>(&Section));
+
   writeEnd();
 
   // Make sure all records are written.
diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index c00a870051c49..e431a2e8a1a54 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -674,7 +674,8 @@ MCContext::getELFUniqueIDForEntsize(StringRef SectionName, unsigned Flags,
 
 template <typename TAttr>
 MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
-                                         TAttr Attributes, MCSection *Parent) {
+                                         TAttr Attributes, MCSection *Parent,
+                                         bool IsVirtual) {
   std::string UniqueName(Name);
   if (Parent) {
     UniqueName.append("/").append(Parent->getName());
@@ -688,8 +689,9 @@ MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
     return Iter->second;
 
   StringRef CachedName = StringRef(Iter->first.c_str(), Name.size());
-  MCSectionGOFF *GOFFSection = new (GOFFAllocator.Allocate()) MCSectionGOFF(
-      CachedName, Kind, Attributes, static_cast<MCSectionGOFF *>(Parent));
+  MCSectionGOFF *GOFFSection = new (GOFFAllocator.Allocate())
+      MCSectionGOFF(CachedName, Kind, IsVirtual, Attributes,
+                    static_cast<MCSectionGOFF *>(Parent));
   Iter->second = GOFFSection;
   allocInitialFragment(*GOFFSection);
   return GOFFSection;
@@ -697,19 +699,22 @@ MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
 
 MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
                                          GOFF::SDAttr SDAttributes) {
-  return getGOFFSection<GOFF::SDAttr>(Kind, Name, SDAttributes, nullptr);
+  return getGOFFSection<GOFF::SDAttr>(Kind, Name, SDAttributes, nullptr,
+                                      /*IsVirtual=*/true);
 }
 
 MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
                                          GOFF::EDAttr EDAttributes,
-                                         MCSection *Parent) {
-  return getGOFFSection<GOFF::EDAttr>(Kind, Name, EDAttributes, Parent);
+                                         MCSection *Parent, bool IsVirtual) {
+  return getGOFFSection<GOFF::EDAttr>(Kind, Name, EDAttributes, Parent,
+                                      IsVirtual);
 }
 
 MCSectionGOFF *MCContext::getGOFFSection(SectionKind Kind, StringRef Name,
                                          GOFF::PRAttr PRAttributes,
                                          MCSection *Parent) {
-  return getGOFFSection<GOFF::PRAttr>(Kind, Name, PRAttributes, Parent);
+  return getGOFFSection<GOFF::PRAttr>(Kind, Name, PRAttributes, Parent,
+                                      /*IsVirtual=*/false);
 }
 
 MCSectionCOFF *MCContext::getCOFFSection(StringRef Section,
diff --git a/llvm/lib/MC/MCObjectFileInfo.cpp b/llvm/lib/MC/MCObjectFileInfo.cpp
index e90797f05073d..2f9d19fab9802 100644
--- a/llvm/lib/MC/MCObjectFileInfo.cpp
+++ b/llvm/lib/MC/MCObjectFileInfo.cpp
@@ -557,7 +557,7 @@ void MCObjectFileInfo::initGOFFMCObjectFileInfo(const Triple &T) {
                    GOFF::ESD_TS_ByteOriented, GOFF::ESD_BA_Merge,
                    GOFF::ESD_LB_Deferred, GOFF::ESD_RQ_1,
                    GOFF::ESD_ALIGN_Quadword},
-      RootSDSection);
+      RootSDSection, /*IsVirtual=*/true);
   ADASection = Ctx->getGOFFSection(
       SectionKind::getData(), "#S",
       GOFF::PRAttr{false, false, GOFF::ESD_EXE_DATA, GOFF::ESD_LT_XPLink,
@@ -570,7 +570,7 @@ void MCObjectFileInfo::initGOFFMCObjectFileInfo(const Triple &T) {
                    GOFF::ESD_TS_ByteOriented, GOFF::ESD_BA_Concatenate,
                    GOFF::ESD_LB_Initial, GOFF::ESD_RQ_0,
                    GOFF::ESD_ALIGN_Doubleword},
-      RootSDSection);
+      RootSDSection, /*IsVirtual=*/false);
 
   MCSectionGOFF *PPA2ListEDSection = Ctx->getGOFFSection(
       SectionKind::getMetadata(), GOFF::CLASS_PPA2,
@@ -578,7 +578,7 @@ void MCObjectFileInfo::initGOFFMCObjectFileInfo(const Triple &T) {
                    GOFF::ESD_TS_ByteOriented, GOFF::ESD_BA_Merge,
                    GOFF::ESD_LB_Initial, GOFF::ESD_RQ_0,
                    GOFF::ESD_ALIGN_Doubleword},
-      RootSDSection);
+      RootSDSection, /*IsVirtual=*/true);
   PPA2ListSection = Ctx->getGOFFSection(
       SectionKind::getData(), ".&ppa2",
       GOFF::PRAttr{true, false, GOFF::ESD_EXE_DATA, GOFF::ESD_LT_OS,
@@ -591,7 +591,7 @@ void MCObjectFileInfo::initGOFFMCObjectFileInfo(const Triple &T) {
                    GOFF::ESD_TS_Structured, GOFF::ESD_BA_Concatenate,
                    GOFF::ESD_LB_NoLoad, GOFF::ESD_RQ_0,
                    GOFF::ESD_ALIGN_Doubleword},
-      RootSDSection);
+      RootSDSection, /*IsVirtual=*/false);
 }
 
 void MCObjectFileInfo::initCOFFMCObjectFileInfo(const Triple &T) {
diff --git a/llvm/test/CodeGen/SystemZ/zos-section-1.ll b/llvm/test/CodeGen/SystemZ/zos-section-1.ll
index 74f4e272005f8..a1b33c6925b2f 100644
--- a/llvm/test/CodeGen/SystemZ/zos-section-1.ll
+++ b/llvm/test/CodeGen/SystemZ/zos-section-1.ll
@@ -104,9 +104,26 @@ entry:
 ; CHECK-NEXT: 0000300 00 00 00 00 00 00 00 00 00 00 00 00 04 00 00 02
 ; CHECK-NEXT: 0000310 00 01 20 00 00 00 00 06 a3 85 a2 a3 7b c3 00 00
 
+; Text record for the code section C_CODE64.
+; The regular expression matches the lower byte of the length.
+; CHECK-NEXT: 0000320 03 11 00 00 [[C_CODE64]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000330 00 00 00 00 00 00 00 {{..}} 00 c3 00 c5 00 c5 00 f1
+
+; Text record for the section .&ppa2.
+; CHECK:      00003c0 03 10 00 00 [[PPA2]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 00003d0 00 00 00 00 00 00 00 {{..}} {{.*}}
+
+; Text record for the ADA section test#S.
+; CHECK:      0000410 03 10 00 00 [[TESTS]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000420 00 00 00 00 00 00 00 {{..}} {{.*}}
+
+; Text record for the section B_IDRL.
+; CHECK:      0000460 03 10 00 01 [[BIDRL]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000470 00 00 00 00 00 00 00 {{..}} {{.*}}
+
 ; End record.
-; CHECK-NEXT: 0000320 03 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000330 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000340 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000350 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000360 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK:      00004b0 03 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 00004c0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 00004d0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 00004e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 00004f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
diff --git a/llvm/test/CodeGen/SystemZ/zos-section-2.ll b/llvm/test/CodeGen/SystemZ/zos-section-2.ll
index 25803c3bbfb0c..2eb9fa28fb7ab 100644
--- a/llvm/test/CodeGen/SystemZ/zos-section-2.ll
+++ b/llvm/test/CodeGen/SystemZ/zos-section-2.ll
@@ -147,9 +147,29 @@ source_filename = "test.ll"
 ; CHECK-NEXT: 00004e0 00 00 00 00 00 00 00 00 00 00 00 00 04 00 00 02
 ; CHECK-NEXT: 00004f0 00 01 20 00 00 00 00 06 a3 85 a2 a3 7b c3 00 00
 
+; Text record for the code section C_CODE64.
+; The regular expression matches the lower byte of the length.
+; CHECK-NEXT: 0000500 03 10 00 00 [[C_CODE64]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000510 00 00 00 00 00 00 00 {{..}} {{.*}}
+
+; Text record for the section .&ppa2.
+; CHECK:      0000550 03 10 00 00 [[PPA2]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000560 00 00 00 00 00 00 00 {{..}} {{.*}}
+
+; Text record for the section data.
+; Length is 4, and the content is 0x2a = 42.
+; CHECK:      00005a0 03 10 00 00 [[DATA_PR]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 00005b0 00 00 00 00 00 00 00 04 00 00 00 2a 00 00 00 00
+
+; There is no text record for section bss!
+
+; Text record for the section B_IDRL.
+; CHECK:      00005f0 03 10 00 01 [[BIDRL]] 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000600 00 00 00 00 00 00 00 {{..}} {{.*}}
+
 ; End record.
-; CHECK-NEXT: 0000500 03 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000510 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000520 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000530 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-; CHECK-NEXT: 0000540 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK:      0000640 03 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000650 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000660 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000670 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+; CHECK-NEXT: 0000680 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

github-actions · 2025-04-24T19:00:58Z

✅ With the latest revision this PR passed the C/C++ code formatter.

uweigand

Why do we need all that "virtual section" stuff? Wouldn't it suffice to exit early from ::writeText if the section length is zero?

uweigand · 2025-04-25T13:09:16Z

Also, the new tests seem to be failing in CI.

redstar · 2025-04-29T14:34:29Z

Why do we need all that "virtual section" stuff? Wouldn't it suffice to exit early from ::writeText if the section length is zero?

A virtual sections cannot contain data, and the MC layer checks that this is the case. This is the benefit I see in using a virtual sections. But of course, it is sufficient to just check the section length.

redstar · 2025-04-29T14:39:21Z

Also, the new tests seem to be failing in CI.

It's also failing in the base PR. Looks like I made a mistake with my last update.

Sections which are not allowed to carry data are marked as virtual. Only complication when writing out the text is that it must be written in chunks of 32k-1 bytes, which is done by having a wrapper stream writing those records. Data of BSS sections is not written, since the contents is known to be zero. Instead, the fill byte value is used.

redstar · 2025-04-30T14:32:32Z

I had to force-push to get the fixed test cases.

uweigand · 2025-04-30T16:17:08Z

llvm/lib/MC/MCContext.cpp

-  return getGOFFSection<GOFF::EDAttr>(Kind, Name, EDAttributes, Parent);
+                                         MCSection *Parent, bool IsVirtual) {
+  return getGOFFSection<GOFF::EDAttr>(Kind, Name, EDAttributes, Parent,
+                                      IsVirtual);


Can we deduce the IsVirtual property from EDAttributes.BindAlgorithm == GOFF::ESD_BA_Merge here? Having to explicitly specific it with every caller seems error-prone.

Good point, changed.

uweigand

LGTM now, thanks!

uweigand · 2025-05-07T08:18:28Z

llvm/lib/MC/MCObjectFileInfo.cpp

@@ -571,7 +571,6 @@ void MCObjectFileInfo::initGOFFMCObjectFileInfo(const Triple &T) {
                   GOFF::ESD_LB_Initial, GOFF::ESD_RQ_0,
                   GOFF::ESD_ALIGN_Doubleword, 0},
      RootSDSection);
-


This change is spurious now.

MaskRay · 2025-05-09T06:24:21Z

llvm/include/llvm/MC/MCSectionGOFF.h

-  MCSectionGOFF(StringRef Name, SectionKind K, GOFF::SDAttr SDAttributes,
-                MCSectionGOFF *Parent)
-      : MCSection(SV_GOFF, Name, K.isText(), /*IsVirtual=*/false, nullptr),
+  MCSectionGOFF(StringRef Name, SectionKind K, bool IsVirtual,


If it's BSS, call it BSS.

Virtual seems a weird naming convention when the initial Mach-O MC code was added. New code doesn't need to endorse the weird naming.

It's not actually about BSS (those are named BSS - but they're not virtual). The "virtual" marker is for sections that represent those GOFF symbols that do not themselves contain text, but have nested GOFF sections which do. For example the section representing a SD GOFF section does not contain text directly, but it does contain ED sections; and sections representing a ED symbol with the merge attribute do not contain text directly, but contain PR sections.

Exactly. The idea is to trigger the check in MCAssembler::writeSectionData() for those sections which are not allowed to contain text.

uweigand · 2025-05-09T06:24:55Z

Huh. Looks like the new test case now fails ...

redstar added the backend:SystemZ label Apr 24, 2025

redstar requested review from MaskRay, uweigand and Everybody0523 April 24, 2025 18:57

redstar self-assigned this Apr 24, 2025

llvmbot added the mc Machine (object) code label Apr 24, 2025

uweigand reviewed Apr 25, 2025

View reviewed changes

redstar added 3 commits April 30, 2025 10:29

Fix formatting & move method out of anonymous namespace

37f7a1e

Fix test cases

a5e31c1

redstar force-pushed the users/redstar/goffwriter-4 branch from 6001464 to a5e31c1 Compare April 30, 2025 14:31

uweigand reviewed Apr 30, 2025

View reviewed changes

Derive IsVirtual from the given attributes

94f7af1

uweigand approved these changes May 7, 2025

View reviewed changes

Fix accidental line removal.

37581cf

MaskRay reviewed May 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GOFF] Add writing of text records #137235

[GOFF] Add writing of text records #137235

redstar commented Apr 24, 2025

llvmbot commented Apr 24, 2025 •

edited

Loading

github-actions bot commented Apr 24, 2025 •

edited

Loading

uweigand left a comment

uweigand commented Apr 25, 2025

redstar commented Apr 29, 2025

redstar commented Apr 29, 2025

redstar commented Apr 30, 2025

uweigand Apr 30, 2025

redstar May 6, 2025

uweigand left a comment

uweigand May 7, 2025

redstar May 8, 2025

MaskRay May 9, 2025

uweigand May 9, 2025

redstar May 9, 2025

uweigand commented May 9, 2025

[GOFF] Add writing of text records #137235

Are you sure you want to change the base?

[GOFF] Add writing of text records #137235

Conversation

redstar commented Apr 24, 2025

llvmbot commented Apr 24, 2025 • edited Loading

github-actions bot commented Apr 24, 2025 • edited Loading

uweigand left a comment

Choose a reason for hiding this comment

uweigand commented Apr 25, 2025

redstar commented Apr 29, 2025

redstar commented Apr 29, 2025

redstar commented Apr 30, 2025

uweigand Apr 30, 2025

Choose a reason for hiding this comment

redstar May 6, 2025

Choose a reason for hiding this comment

uweigand left a comment

Choose a reason for hiding this comment

uweigand May 7, 2025

Choose a reason for hiding this comment

redstar May 8, 2025

Choose a reason for hiding this comment

MaskRay May 9, 2025

Choose a reason for hiding this comment

uweigand May 9, 2025

Choose a reason for hiding this comment

redstar May 9, 2025

Choose a reason for hiding this comment

uweigand commented May 9, 2025

llvmbot commented Apr 24, 2025 •

edited

Loading

github-actions bot commented Apr 24, 2025 •

edited

Loading