[llvm] [GOFF] Refactor writing GOFF records (PR #93855)

Kai Nacke via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 7 10:15:09 PDT 2024


https://github.com/redstar updated https://github.com/llvm/llvm-project/pull/93855

>From da97d3d33ca68787981f9d43b7706b4c160500e4 Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Mon, 13 May 2024 14:53:04 -0400
Subject: [PATCH 1/4] [GOFF] Refactor writing GOFF records

A GOFF file is basically a sequence of records. Change the YAML mapping for GOFF files to reflect this. As result, creating files for testing, e.g. without a module header, becomes possible.
---
 llvm/include/llvm/ObjectYAML/GOFFYAML.h       |  86 +++++-
 llvm/lib/ObjectYAML/GOFFEmitter.cpp           | 267 ++++++++----------
 llvm/lib/ObjectYAML/GOFFYAML.cpp              |  94 ++++--
 .../yaml2obj/GOFF/GOFF-header-settings.yaml   |  26 --
 .../tools/yaml2obj/GOFF/GOFF-no-header.yaml   |   7 -
 .../tools/yaml2obj/GOFF/end-amode-const.yaml  |  15 +
 .../tools/yaml2obj/GOFF/end-long-name.yaml    |  70 +++++
 llvm/test/tools/yaml2obj/GOFF/end-only.yaml   |  22 ++
 .../{GOFF-header-end.yaml => header-end.yaml} |  14 +-
 .../test/tools/yaml2obj/GOFF/header-only.yaml |  18 ++
 .../tools/yaml2obj/GOFF/invalid-record.yaml   |   7 +
 11 files changed, 403 insertions(+), 223 deletions(-)
 delete mode 100644 llvm/test/tools/yaml2obj/GOFF/GOFF-header-settings.yaml
 delete mode 100644 llvm/test/tools/yaml2obj/GOFF/GOFF-no-header.yaml
 create mode 100644 llvm/test/tools/yaml2obj/GOFF/end-amode-const.yaml
 create mode 100644 llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
 create mode 100644 llvm/test/tools/yaml2obj/GOFF/end-only.yaml
 rename llvm/test/tools/yaml2obj/GOFF/{GOFF-header-end.yaml => header-end.yaml} (69%)
 create mode 100644 llvm/test/tools/yaml2obj/GOFF/header-only.yaml
 create mode 100644 llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml

diff --git a/llvm/include/llvm/ObjectYAML/GOFFYAML.h b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
index f9bf45e95bd3a..16219058e969b 100644
--- a/llvm/include/llvm/ObjectYAML/GOFFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
@@ -25,25 +25,87 @@ namespace llvm {
 // to use yaml::IO, we use these structures which are closer to the source.
 namespace GOFFYAML {
 
-struct FileHeader {
-  uint32_t TargetEnvironment = 0;
-  uint32_t TargetOperatingSystem = 0;
-  uint16_t CCSID = 0;
-  StringRef CharacterSetName;
-  StringRef LanguageProductIdentifier;
-  uint32_t ArchitectureLevel = 0;
-  std::optional<uint16_t> InternalCCSID;
-  std::optional<uint8_t> TargetSoftwareEnvironment;
+LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_AMODE)
+LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ENDFLAGS)
+
+// The GOFF format uses different kinds of logical records. The format imposes
+// some rules on those records (e.g. the module header must come first, no
+// forward references to records, etc.). However, to be able to specify invalid
+// GOFF files, we treat all records the same way.
+struct RecordBase {
+  enum RecordBaseKind {
+    RBK_ModuleHeader,
+    RBK_RelocationDirectory,
+    RBK_Symbol,
+    RBK_Text,
+    RBK_DeferredLength,
+    RBK_EndOfModule
+  };
+
+private:
+  const RecordBaseKind Kind;
+
+protected:
+  RecordBase(RecordBaseKind Kind) : Kind(Kind) {}
+
+public:
+  RecordBaseKind getKind() const { return Kind; }
+};
+using RecordPtr = std::unique_ptr<RecordBase>;
+
+struct ModuleHeader : public RecordBase {
+  ModuleHeader() : RecordBase(RBK_ModuleHeader) {}
+
+  uint32_t ArchitectureLevel;
+  uint16_t PropertiesLength;
+  std::optional<yaml::BinaryRef> Properties;
+
+  static bool classof(const RecordBase *S) {
+    return S->getKind() == RBK_ModuleHeader;
+  }
+};
+
+struct EndOfModule : public RecordBase {
+  EndOfModule() : RecordBase(RBK_EndOfModule) {}
+
+  GOFF_ENDFLAGS Flags;
+  GOFF_AMODE AMODE;
+  uint32_t RecordCount;
+  uint32_t ESDID;
+  uint32_t Offset;
+  uint16_t NameLength;
+  StringRef EntryName;
+
+  static bool classof(const RecordBase *S) {
+    return S->getKind() == RBK_EndOfModule;
+  }
 };
 
 struct Object {
-  FileHeader Header;
-  Object();
+  // A GOFF file is a sequence of records.
+  std::vector<RecordPtr> Records;
 };
 } // end namespace GOFFYAML
 } // end namespace llvm
 
-LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::FileHeader)
+LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_AMODE)
+LLVM_YAML_DECLARE_ENUM_TRAITS(GOFFYAML::GOFF_ENDFLAGS)
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(GOFFYAML::RecordPtr)
+LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::RecordPtr)
+
+LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::ModuleHeader)
+LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::EndOfModule)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(GOFFYAML::Object)
 
+namespace llvm {
+namespace yaml {
+
+template <> struct CustomMappingTraits<GOFFYAML::RecordPtr> {
+  static void inputOne(IO &IO, StringRef Key, GOFFYAML::RecordPtr &Elem);
+  static void output(IO &IO, GOFFYAML::RecordPtr &Elem);
+};
+
+} // namespace yaml
+} // namespace llvm
 #endif // LLVM_OBJECTYAML_GOFFYAML_H
diff --git a/llvm/lib/ObjectYAML/GOFFEmitter.cpp b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
index 345904407e1d2..a702aefd5076d 100644
--- a/llvm/lib/ObjectYAML/GOFFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
@@ -11,11 +11,12 @@
 ///
 //===----------------------------------------------------------------------===//
 
-#include "llvm/ADT/IndexedMap.h"
-#include "llvm/ObjectYAML/ObjectYAML.h"
+#include "llvm/BinaryFormat/GOFF.h"
+#include "llvm/ObjectYAML/GOFFYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/ConvertEBCDIC.h"
 #include "llvm/Support/Endian.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
@@ -25,10 +26,10 @@ namespace {
 // Common flag values on records.
 enum {
   // Flag: This record is continued.
-  Rec_Continued = 1,
+  Rec_Continued = 1 << 0,
 
   // Flag: This record is a continuation.
-  Rec_Continuation = 1 << (8 - 6 - 1),
+  Rec_Continuation = 1 << 1,
 };
 
 template <typename ValueType> struct BinaryBeImpl {
@@ -62,119 +63,90 @@ ZerosImpl zeros(const size_t NumBytes) { return ZerosImpl{NumBytes}; }
 
 // The GOFFOstream is responsible to write the data into the fixed physical
 // records of the format. A user of this class announces the start of a new
-// logical record and the size of its payload. While writing the payload, the
-// physical records are created for the data. Possible fill bytes at the end of
-// a physical record are written automatically.
-class GOFFOstream : public raw_ostream {
+// logical record, and writes the full logical block. The physical records are
+// created while the content is written to the underlying stream. Possible fill
+// bytes at the end of a physical record are written automatically.
+// The implementation aims at simplicity, not speed.
+class GOFFOStream {
 public:
-  explicit GOFFOstream(raw_ostream &OS)
-      : OS(OS), LogicalRecords(0), RemainingSize(0), NewLogicalRecord(false) {
-    SetBufferSize(GOFF::PayloadLength);
-  }
-
-  ~GOFFOstream() { finalize(); }
+  explicit GOFFOStream(raw_ostream &OS)
+      : OS(OS), CurrentType(GOFF::RecordType(-1)) {}
 
-  void makeNewRecord(GOFF::RecordType Type, size_t Size) {
-    fillRecord();
-    CurrentType = Type;
-    RemainingSize = Size;
-    if (size_t Gap = (RemainingSize % GOFF::PayloadLength))
-      RemainingSize += GOFF::PayloadLength - Gap;
-    NewLogicalRecord = true;
-    ++LogicalRecords;
+  GOFFOStream &operator<<(StringRef Str) {
+    write(Str);
+    return *this;
   }
 
-  void finalize() { fillRecord(); }
-
-  uint32_t logicalRecords() { return LogicalRecords; }
+  void newRecord(GOFF::RecordType Type) { CurrentType = Type; }
 
 private:
   // The underlying raw_ostream.
   raw_ostream &OS;
 
-  // The number of logical records emitted so far.
-  uint32_t LogicalRecords;
-
-  // The remaining size of this logical record, including fill bytes.
-  size_t RemainingSize;
-
   // The type of the current (logical) record.
   GOFF::RecordType CurrentType;
 
-  // Signals start of new record.
-  bool NewLogicalRecord;
-
-  // Return the number of bytes left to write until next physical record.
-  // Please note that we maintain the total number of bytes left, not the
-  // written size.
-  size_t bytesToNextPhysicalRecord() {
-    size_t Bytes = RemainingSize % GOFF::PayloadLength;
-    return Bytes ? Bytes : GOFF::PayloadLength;
-  }
-
   // Write the record prefix of a physical record, using the current record
   // type.
-  static void writeRecordPrefix(raw_ostream &OS, GOFF::RecordType Type,
-                                size_t RemainingSize,
-                                uint8_t Flags = Rec_Continuation) {
-    uint8_t TypeAndFlags = Flags | (Type << 4);
-    if (RemainingSize > GOFF::RecordLength)
-      TypeAndFlags |= Rec_Continued;
-    OS << binaryBe(static_cast<unsigned char>(GOFF::PTVPrefix))
-       << binaryBe(static_cast<unsigned char>(TypeAndFlags))
-       << binaryBe(static_cast<unsigned char>(0));
-  }
+  void writeRecordPrefix(uint8_t Flags);
 
-  // Fill the last physical record of a logical record with zero bytes.
-  void fillRecord() {
-    assert((GetNumBytesInBuffer() <= RemainingSize) &&
-           "More bytes in buffer than expected");
-    size_t Remains = RemainingSize - GetNumBytesInBuffer();
-    if (Remains) {
-      assert((Remains < GOFF::RecordLength) &&
-             "Attempting to fill more than one physical record");
-      raw_ostream::write_zeros(Remains);
-    }
-    flush();
-    assert(RemainingSize == 0 && "Not fully flushed");
-    assert(GetNumBytesInBuffer() == 0 && "Buffer not fully empty");
-  }
+  // Write a logical record.
+  void write(StringRef Str);
+};
 
-  // See raw_ostream::write_impl.
-  void write_impl(const char *Ptr, size_t Size) override {
-    assert((RemainingSize >= Size) && "Attempt to write too much data");
-    assert(RemainingSize && "Logical record overflow");
-    if (!(RemainingSize % GOFF::PayloadLength)) {
-      writeRecordPrefix(OS, CurrentType, RemainingSize,
-                        NewLogicalRecord ? 0 : Rec_Continuation);
-      NewLogicalRecord = false;
-    }
-    assert(!NewLogicalRecord &&
-           "New logical record not on physical record boundary");
-
-    size_t Idx = 0;
-    while (Size > 0) {
-      size_t BytesToWrite = bytesToNextPhysicalRecord();
-      if (BytesToWrite > Size)
-        BytesToWrite = Size;
-      OS.write(Ptr + Idx, BytesToWrite);
-      Idx += BytesToWrite;
-      Size -= BytesToWrite;
-      RemainingSize -= BytesToWrite;
-      if (Size) {
-        writeRecordPrefix(OS, CurrentType, RemainingSize);
-      }
-    }
+void GOFFOStream::writeRecordPrefix(uint8_t Flags) {
+  uint8_t TypeAndFlags = Flags | (CurrentType << 4);
+  OS << binaryBe(static_cast<unsigned char>(GOFF::PTVPrefix))
+     << binaryBe(static_cast<unsigned char>(TypeAndFlags))
+     << binaryBe(static_cast<unsigned char>(0));
+}
+
+void GOFFOStream::write(StringRef Str) {
+  // The flags are determined by the flags of the prvious record, and by the
+  // remaining size of data.
+  uint8_t Flags = 0;
+  size_t Ptr = 0;
+  size_t Size = Str.size();
+  while (Size >= GOFF::RecordContentLength) {
+    if (Flags) {
+      Flags |= Rec_Continuation;
+      if (Size == GOFF::RecordContentLength)
+        Flags &= ~Rec_Continued;
+    } else
+      Flags |= (Size == GOFF::RecordContentLength) ? 0 : Rec_Continued;
+    writeRecordPrefix(Flags);
+    OS.write(&Str.data()[Ptr], GOFF::RecordContentLength);
+    Size -= GOFF::RecordContentLength;
+    Ptr += GOFF::RecordContentLength;
   }
+  if (Size) {
+    Flags &= ~Rec_Continued;
+    writeRecordPrefix(Flags);
+    OS.write(&Str.data()[Ptr], Size);
+    OS.write_zeros(GOFF::RecordContentLength - Size);
+  }
+}
+
+// A LogicalRecord buffers the data of a record.
+class LogicalRecord : public raw_svector_ostream {
+  GOFFOStream &OS;
+  SmallVector<char, 0> Buffer;
+
+  void anchor() override {};
 
-  // Return the current position within the stream, not counting the bytes
-  // currently in the buffer.
-  uint64_t current_pos() const override { return OS.tell(); }
+public:
+  LogicalRecord(GOFFOStream &OS) : raw_svector_ostream(Buffer), OS(OS) {}
+  ~LogicalRecord() override { OS << str(); }
+
+  LogicalRecord &operator<<(yaml::BinaryRef B) {
+    B.writeAsBinary(*this);
+    return *this;
+  }
 };
 
 class GOFFState {
-  void writeHeader(GOFFYAML::FileHeader &FileHdr);
-  void writeEnd();
+  void writeHeader(GOFFYAML::ModuleHeader &ModHdr);
+  void writeEnd(GOFFYAML::EndOfModule &EndMod);
 
   void reportError(const Twine &Msg) {
     ErrHandler(Msg);
@@ -185,8 +157,6 @@ class GOFFState {
             yaml::ErrorHandler ErrHandler)
       : GW(OS), Doc(Doc), ErrHandler(ErrHandler), HasError(false) {}
 
-  ~GOFFState() { GW.finalize(); }
-
   bool writeObject();
 
 public:
@@ -194,72 +164,61 @@ class GOFFState {
                         yaml::ErrorHandler ErrHandler);
 
 private:
-  GOFFOstream GW;
+  GOFFOStream GW;
   GOFFYAML::Object &Doc;
   yaml::ErrorHandler ErrHandler;
   bool HasError;
 };
 
-void GOFFState::writeHeader(GOFFYAML::FileHeader &FileHdr) {
-  SmallString<16> CCSIDName;
-  if (std::error_code EC =
-          ConverterEBCDIC::convertToEBCDIC(FileHdr.CharacterSetName, CCSIDName))
-    reportError("Conversion error on " + FileHdr.CharacterSetName);
-  if (CCSIDName.size() > 16) {
-    reportError("CharacterSetName too long");
-    CCSIDName.resize(16);
-  }
-  SmallString<16> LangProd;
-  if (std::error_code EC = ConverterEBCDIC::convertToEBCDIC(
-          FileHdr.LanguageProductIdentifier, LangProd))
-    reportError("Conversion error on " + FileHdr.LanguageProductIdentifier);
-  if (LangProd.size() > 16) {
-    reportError("LanguageProductIdentifier too long");
-    LangProd.resize(16);
-  }
-
-  GW.makeNewRecord(GOFF::RT_HDR, GOFF::PayloadLength);
-  GW << binaryBe(FileHdr.TargetEnvironment)     // TargetEnvironment
-     << binaryBe(FileHdr.TargetOperatingSystem) // TargetOperatingSystem
-     << zeros(2)                                // Reserved
-     << binaryBe(FileHdr.CCSID)                 // CCSID
-     << CCSIDName                               // CharacterSetName
-     << zeros(16 - CCSIDName.size())            // Fill bytes
-     << LangProd                                // LanguageProductIdentifier
-     << zeros(16 - LangProd.size())             // Fill bytes
-     << binaryBe(FileHdr.ArchitectureLevel);    // ArchitectureLevel
-  // The module propties are optional. Figure out if we need to write them.
-  uint16_t ModPropLen = 0;
-  if (FileHdr.TargetSoftwareEnvironment)
-    ModPropLen = 3;
-  else if (FileHdr.InternalCCSID)
-    ModPropLen = 2;
-  if (ModPropLen) {
-    GW << binaryBe(ModPropLen) << zeros(6);
-    if (ModPropLen >= 2)
-      GW << binaryBe(FileHdr.InternalCCSID ? *FileHdr.InternalCCSID : 0);
-    if (ModPropLen >= 3)
-      GW << binaryBe(FileHdr.TargetSoftwareEnvironment
-                         ? *FileHdr.TargetSoftwareEnvironment
-                         : 0);
-  }
+void GOFFState::writeHeader(GOFFYAML::ModuleHeader &ModHdr) {
+  GW.newRecord(GOFF::RT_HDR);
+  LogicalRecord LR(GW);
+  LR << zeros(45)                          // Reserved.
+     << binaryBe(ModHdr.ArchitectureLevel) // The architecture level.
+     << binaryBe(ModHdr.PropertiesLength)  // Length of module properties.
+     << zeros(6);                          // Reserved.
+  if (ModHdr.Properties)
+    LR << *ModHdr.Properties; // Module properties.
 }
 
-void GOFFState::writeEnd() {
-  GW.makeNewRecord(GOFF::RT_END, GOFF::PayloadLength);
-  GW << binaryBe(uint8_t(0)) // No entry point
-     << binaryBe(uint8_t(0)) // No AMODE
-     << zeros(3)             // Reserved
-     << binaryBe(GW.logicalRecords());
-  // No entry point yet. Automatically fill remaining space with zero bytes.
-  GW.finalize();
+void GOFFState::writeEnd(GOFFYAML::EndOfModule &EndMod) {
+  SmallString<16> EntryName;
+  if (std::error_code EC =
+          ConverterEBCDIC::convertToEBCDIC(EndMod.EntryName, EntryName))
+    reportError("Conversion error on " + EndMod.EntryName);
+
+  GW.newRecord(GOFF::RT_END);
+  LogicalRecord LR(GW);
+  LR << binaryBe(uint8_t(EndMod.Flags)) // The flags.
+     << binaryBe(uint8_t(EndMod.AMODE)) // The addressing mode.
+     << zeros(3)                        // Reserved.
+     << binaryBe(EndMod.RecordCount)    // The record count.
+     << binaryBe(EndMod.ESDID)          // ESDID of the entry point.
+     << zeros(4)                        // Reserved.
+     << binaryBe(EndMod.Offset)         // Offset of entry point.
+     << binaryBe(EndMod.NameLength)     // Length of external name.
+     << EntryName;                      // Name of the entry point.
 }
 
 bool GOFFState::writeObject() {
-  writeHeader(Doc.Header);
-  if (HasError)
-    return false;
-  writeEnd();
+  for (auto &RecPtr : Doc.Records) {
+    auto *Rec = RecPtr.get();
+    switch (Rec->getKind()) {
+    case GOFFYAML::RecordBase::RBK_ModuleHeader:
+      writeHeader(*static_cast<GOFFYAML::ModuleHeader *>(Rec));
+      break;
+    case GOFFYAML::RecordBase::RBK_EndOfModule:
+      writeEnd(*static_cast<GOFFYAML::EndOfModule *>(Rec));
+      break;
+    case GOFFYAML::RecordBase::RBK_RelocationDirectory:
+    case GOFFYAML::RecordBase::RBK_Symbol:
+    case GOFFYAML::RecordBase::RBK_Text:
+    case GOFFYAML::RecordBase::RBK_DeferredLength:
+      llvm_unreachable(("Not yet implemented"));
+    }
+    if (HasError)
+      return false;
+  }
   return true;
 }
 
diff --git a/llvm/lib/ObjectYAML/GOFFYAML.cpp b/llvm/lib/ObjectYAML/GOFFYAML.cpp
index ae857980a521b..cad61b759e01e 100644
--- a/llvm/lib/ObjectYAML/GOFFYAML.cpp
+++ b/llvm/lib/ObjectYAML/GOFFYAML.cpp
@@ -12,34 +12,92 @@
 
 #include "llvm/ObjectYAML/GOFFYAML.h"
 #include "llvm/BinaryFormat/GOFF.h"
-#include <string.h>
 
 namespace llvm {
-namespace GOFFYAML {
 
-Object::Object() {}
+namespace yaml {
 
-} // namespace GOFFYAML
+void ScalarEnumerationTraits<GOFFYAML::GOFF_AMODE>::enumeration(
+    IO &IO, GOFFYAML::GOFF_AMODE &Value) {
+#define ECase(X) IO.enumCase(Value, #X, GOFF::ESD_##X)
+  ECase(AMODE_None);
+  ECase(AMODE_24);
+  ECase(AMODE_31);
+  ECase(AMODE_ANY);
+  ECase(AMODE_64);
+  ECase(AMODE_MIN);
+#undef ECase
+  IO.enumFallback<Hex8>(Value);
+}
 
-namespace yaml {
+void ScalarEnumerationTraits<GOFFYAML::GOFF_ENDFLAGS>::enumeration(
+    IO &IO, GOFFYAML::GOFF_ENDFLAGS &Value) {
+#define ECase(X) IO.enumCase(Value, #X, unsigned(GOFF::END_##X) << 6)
+  ECase(EPR_None);
+  ECase(EPR_EsdidOffset);
+  ECase(EPR_ExternalName);
+  ECase(EPR_Reserved);
+#undef ECase
+  IO.enumFallback<Hex8>(Value);
+}
+
+void MappingTraits<GOFFYAML::ModuleHeader>::mapping(
+    IO &IO, GOFFYAML::ModuleHeader &ModHdr) {
+  IO.mapOptional("ArchitectureLevel", ModHdr.ArchitectureLevel, 0);
+  IO.mapOptional("PropertiesLength", ModHdr.PropertiesLength, 0);
+  IO.mapOptional("Properties", ModHdr.Properties);
+}
+
+void MappingTraits<GOFFYAML::EndOfModule>::mapping(IO &IO,
+                                                   GOFFYAML::EndOfModule &End) {
+  IO.mapOptional("Flags", End.Flags, 0);
+  IO.mapOptional("AMODE", End.AMODE, 0);
+  IO.mapOptional("RecordCount", End.RecordCount, 0);
+  IO.mapOptional("ESDID", End.ESDID, 0);
+  IO.mapOptional("Offset", End.Offset, 0);
+  IO.mapOptional("NameLength", End.NameLength, 0);
+  IO.mapOptional("EntryName", End.EntryName);
+}
+
+void CustomMappingTraits<GOFFYAML::RecordPtr>::inputOne(
+    IO &IO, StringRef Key, GOFFYAML::RecordPtr &Elem) {
+  if (Key == "ModuleHeader") {
+    GOFFYAML::ModuleHeader ModHdr;
+    IO.mapRequired("ModuleHeader", ModHdr);
+    Elem = std::make_unique<GOFFYAML::ModuleHeader>(std::move(ModHdr));
+  } else if (Key == "End") {
+    GOFFYAML::EndOfModule End;
+    IO.mapRequired("End", End);
+    Elem = std::make_unique<GOFFYAML::EndOfModule>(std::move(End));
+  } else if (Key == "RelocationDirectory" || Key == "Symbol" || Key == "Text" ||
+             Key == "Length")
+    IO.setError(Twine("Not yet implemented ").concat(Key));
+  else
+    IO.setError(Twine("Unknown record type name ").concat(Key));
+}
 
-void MappingTraits<GOFFYAML::FileHeader>::mapping(
-    IO &IO, GOFFYAML::FileHeader &FileHdr) {
-  IO.mapOptional("TargetEnvironment", FileHdr.TargetEnvironment, 0);
-  IO.mapOptional("TargetOperatingSystem", FileHdr.TargetOperatingSystem, 0);
-  IO.mapOptional("CCSID", FileHdr.CCSID, 0);
-  IO.mapOptional("CharacterSetName", FileHdr.CharacterSetName, "");
-  IO.mapOptional("LanguageProductIdentifier", FileHdr.LanguageProductIdentifier,
-                 "");
-  IO.mapOptional("ArchitectureLevel", FileHdr.ArchitectureLevel, 1);
-  IO.mapOptional("InternalCCSID", FileHdr.InternalCCSID);
-  IO.mapOptional("TargetSoftwareEnvironment",
-                 FileHdr.TargetSoftwareEnvironment);
+void CustomMappingTraits<GOFFYAML::RecordPtr>::output(
+    IO &IO, GOFFYAML::RecordPtr &Elem) {
+  switch (Elem->getKind()) {
+  case GOFFYAML::RecordBase::RBK_ModuleHeader:
+    IO.mapRequired("ModuleHeader",
+                   *static_cast<GOFFYAML::ModuleHeader *>(Elem.get()));
+    break;
+  case GOFFYAML::RecordBase::RBK_EndOfModule:
+    IO.mapRequired("End", *static_cast<GOFFYAML::EndOfModule *>(Elem.get()));
+    break;
+  case GOFFYAML::RecordBase::RBK_RelocationDirectory:
+  case GOFFYAML::RecordBase::RBK_Symbol:
+  case GOFFYAML::RecordBase::RBK_Text:
+  case GOFFYAML::RecordBase::RBK_DeferredLength:
+    llvm_unreachable(("Not yet implemented"));
+  }
 }
 
 void MappingTraits<GOFFYAML::Object>::mapping(IO &IO, GOFFYAML::Object &Obj) {
   IO.mapTag("!GOFF", true);
-  IO.mapRequired("FileHeader", Obj.Header);
+  EmptyContext Context;
+  yamlize(IO, Obj.Records, false, Context);
 }
 
 } // namespace yaml
diff --git a/llvm/test/tools/yaml2obj/GOFF/GOFF-header-settings.yaml b/llvm/test/tools/yaml2obj/GOFF/GOFF-header-settings.yaml
deleted file mode 100644
index 1971c407199fb..0000000000000
--- a/llvm/test/tools/yaml2obj/GOFF/GOFF-header-settings.yaml
+++ /dev/null
@@ -1,26 +0,0 @@
-# RUN: yaml2obj %s | od -v -An -tx1 | FileCheck --ignore-case %s
-
-## Verify that GOFF Header is correct.
-# CHECK:      03 f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 01 00 03 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-
-## Verify GOFF Module end.
-# CHECK-NEXT: 03 40 00 00 00 00 00 00 00 00 00 02 00 00 00 00
-# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-
---- !GOFF
-FileHeader:
-  TargetEnvironment: 0
-  TargetOperatingSystem: 0
-  CCSID: 0
-  CharacterSetName: ""
-  LanguageProductIdentifier: ""
-  ArchitectureLevel: 1
-  InternalCCSID: 0
-  TargetSoftwareEnvironment: 0 
diff --git a/llvm/test/tools/yaml2obj/GOFF/GOFF-no-header.yaml b/llvm/test/tools/yaml2obj/GOFF/GOFF-no-header.yaml
deleted file mode 100644
index 55f0efaacdf89..0000000000000
--- a/llvm/test/tools/yaml2obj/GOFF/GOFF-no-header.yaml
+++ /dev/null
@@ -1,7 +0,0 @@
-# RUN: not yaml2obj %s FileCheck --ignore-case %s
-
-# CHECK: yaml2obj: error: missing required key 'FileHeader'
---- !GOFF
-## X: [] is an extra field required as workaround for 
-## 'document of unknown type' error
-X: []
diff --git a/llvm/test/tools/yaml2obj/GOFF/end-amode-const.yaml b/llvm/test/tools/yaml2obj/GOFF/end-amode-const.yaml
new file mode 100644
index 0000000000000..48d8b8dfe6c16
--- /dev/null
+++ b/llvm/test/tools/yaml2obj/GOFF/end-amode-const.yaml
@@ -0,0 +1,15 @@
+# RUN: yaml2obj %s | od -v -An -tx1 | FileCheck --ignore-case %s
+
+## Verify that a constant can be used for field AMODE.
+
+# CHECK:      03 40 00 00 04 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
+# CHECK-EMPTY:
+
+--- !GOFF
+- End:
+    AMODE: 4
diff --git a/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml b/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
new file mode 100644
index 0000000000000..dfb20d7d6b7db
--- /dev/null
+++ b/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
@@ -0,0 +1,70 @@
+# RUN: sed -e "s/@FILL@/$(printf '%47s')/g" < %s | yaml2obj | \
+# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK1 %s
+# RUN: sed -e "s/@FILL@/$(printf '%48s')/g" < %s | yaml2obj | \
+# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK2 %s
+# RUN: sed -e "s/@FILL@/$(printf '%125s')/g" < %s | yaml2obj | \
+# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK3 %s
+# RUN: sed -e "s/@FILL@/$(printf '%160s')/g" < %s | yaml2obj | \
+# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK4 %s
+
+
+## Verify that the entry name is written correctly over multiple records.
+## The continued/continuation flags are in the lower nibble of the 2nd byte
+## of a record.
+
+## Last byte is a fill byte.
+# CHECK1:      03 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK1-NEXT: 00 00 00 00 00 00 00 00 00 00 c6 96 96 c2 81 99
+# CHECK1-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK1-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK1-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 00
+# CHECK1-EMPTY:
+
+## 1 record fully used.
+## Flags = 0 => no continuation, not continued.
+# CHECK2:      03 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK2-NEXT: 00 00 00 00 00 00 00 00 00 00 c6 96 96 c2 81 99
+# CHECK2-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK2-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK2-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK2-EMPTY:
+
+## 2 records fully used.
+## Flags = 1 => no continuation, but continued.
+# CHECK3:      03 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK3-NEXT: 00 00 00 00 00 00 00 00 00 00 c6 96 96 c2 81 99
+# CHECK3-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK3-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK3-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+## Flags = 2 => continuation, not continued.
+# CHECK3-NEXT: 03 42 00 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK3-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK3-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK3-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK3-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK3-EMPTY:
+
+## 3rd record used half.
+## Flags = 1 => no continuation, but continued.
+# CHECK4:      03 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK4-NEXT: 00 00 00 00 00 00 00 00 00 00 c6 96 96 c2 81 99
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+## Flags = 3 => continuation, and continued.
+# CHECK4-NEXT: 03 43 00 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+## Flags = 2 => continuation, not continued.
+# CHECK4-NEXT: 03 42 00 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40
+# CHECK4-NEXT: 40 40 40 40 40 40 00 00 00 00 00 00 00 00 00 00
+# CHECK4-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK4-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK4-EMPTY:
+
+--- !GOFF
+- End:
+    EntryName: "FooBar at FILL@"
diff --git a/llvm/test/tools/yaml2obj/GOFF/end-only.yaml b/llvm/test/tools/yaml2obj/GOFF/end-only.yaml
new file mode 100644
index 0000000000000..0f7af5530dc89
--- /dev/null
+++ b/llvm/test/tools/yaml2obj/GOFF/end-only.yaml
@@ -0,0 +1,22 @@
+# RUN: yaml2obj %s | od -v -An -tx1 | FileCheck --ignore-case %s
+
+## Verify that the GOFF end record is correct, with all fields having a value.
+## No other record is written.
+
+# CHECK:      03 40 00 01 04 00 00 00 00 00 00 01 00 00 00 2a
+# CHECK-NEXT: 00 00 00 00 00 00 02 01 00 06 c6 96 96 c2 81 99
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
+# CHECK-EMPTY:
+
+--- !GOFF
+- End:
+    Flags: 1
+    AMODE: AMODE_64
+    RecordCount: 1
+    ESDID: 42
+    Offset: 513
+    NameLength: 6
+    EntryName: FooBar
diff --git a/llvm/test/tools/yaml2obj/GOFF/GOFF-header-end.yaml b/llvm/test/tools/yaml2obj/GOFF/header-end.yaml
similarity index 69%
rename from llvm/test/tools/yaml2obj/GOFF/GOFF-header-end.yaml
rename to llvm/test/tools/yaml2obj/GOFF/header-end.yaml
index a5e99c2da2c49..242a11462539a 100644
--- a/llvm/test/tools/yaml2obj/GOFF/GOFF-header-end.yaml
+++ b/llvm/test/tools/yaml2obj/GOFF/header-end.yaml
@@ -1,20 +1,22 @@
 # RUN: yaml2obj %s | od -v -An -tx1 | FileCheck --ignore-case %s
 
-## Verify that GOFF Header is correct.
+## Verify that the GOFF Header and end record are written with the default
+## values. No other record is written.
+
 # CHECK:      03 f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 # CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 # CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
-# CHECK-NEXT: 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 # CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 
-## Verify GOFF Module end.
-# CHECK-NEXT: 03 40 00 00 00 00 00 00 00 00 00 02 00 00 00 00
+# CHECK-NEXT: 03 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 # CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 # CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 # CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 # CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
 # CHECK-EMPTY:
 
 --- !GOFF
-FileHeader:
-  ArchitectureLevel: 1
+- ModuleHeader:
+- End:
diff --git a/llvm/test/tools/yaml2obj/GOFF/header-only.yaml b/llvm/test/tools/yaml2obj/GOFF/header-only.yaml
new file mode 100644
index 0000000000000..a4f121d9c8593
--- /dev/null
+++ b/llvm/test/tools/yaml2obj/GOFF/header-only.yaml
@@ -0,0 +1,18 @@
+# RUN: yaml2obj %s | od -v -An -tx1 | FileCheck --ignore-case %s
+
+## Verify that the GOFF header record is correct, with all fields having a
+## value. No other record is written.
+
+# CHECK:      03 f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# CHECK-NEXT: 00 00 00 01 00 03 00 00 00 00 00 00 03 33 ff 00
+# CHECK-NEXT: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
+# CHECK-EMPTY:
+
+--- !GOFF
+- ModuleHeader:
+    ArchitectureLevel: 1
+    PropertiesLength: 3
+    Properties: 0333ff
diff --git a/llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml b/llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml
new file mode 100644
index 0000000000000..5b1d2b588d76b
--- /dev/null
+++ b/llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml
@@ -0,0 +1,7 @@
+# RUN: not yaml2obj %s 2>&1 | FileCheck %s
+
+# CHECK: YAML:{{[0-9]*}}:{{[0-9]*}}: error: Unknown record type name Foo
+# CHECk: yaml2obj: error: failed to parse YAML input: Invalid argument
+
+--- !GOFF
+- Foo:

>From d850899fdfdb66d70cdadf1299c2ea4a53bece4d Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Fri, 31 May 2024 10:50:53 -0400
Subject: [PATCH 2/4] Replace combination of sed and bash expression with awk
 in LIT test

---
 .../tools/yaml2obj/GOFF/end-long-name.yaml    | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml b/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
index dfb20d7d6b7db..c48ba28d8c42b 100644
--- a/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
+++ b/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
@@ -1,11 +1,15 @@
-# RUN: sed -e "s/@FILL@/$(printf '%47s')/g" < %s | yaml2obj | \
-# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK1 %s
-# RUN: sed -e "s/@FILL@/$(printf '%48s')/g" < %s | yaml2obj | \
-# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK2 %s
-# RUN: sed -e "s/@FILL@/$(printf '%125s')/g" < %s | yaml2obj | \
-# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK3 %s
-# RUN: sed -e "s/@FILL@/$(printf '%160s')/g" < %s | yaml2obj | \
-# RUN:   od -v -An -tx1 | FileCheck --ignore-case --check-prefix CHECK4 %s
+# RUN: awk -e 'BEGIN {str = sprintf("%47s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN:   yaml2obj | od -v -An -tx1 | \
+# RUN:   FileCheck --ignore-case --check-prefix CHECK1 %s
+# RUN: awk -e 'BEGIN {str = sprintf("%48s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN:   yaml2obj | od -v -An -tx1 | \
+# RUN:   FileCheck --ignore-case --check-prefix CHECK2 %s
+# RUN: awk -e 'BEGIN {str = sprintf("%125s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN:   yaml2obj | od -v -An -tx1 | \
+# RUN:   FileCheck --ignore-case --check-prefix CHECK3 %s
+# RUN: awk -e 'BEGIN {str = sprintf("%160s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN:   yaml2obj | od -v -An -tx1 | \
+# RUN:   FileCheck --ignore-case --check-prefix CHECK4 %s
 
 
 ## Verify that the entry name is written correctly over multiple records.

>From ebd2444a34ab7affe48c7e17a89f7e065b1f356d Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Fri, 31 May 2024 14:13:24 -0400
Subject: [PATCH 3/4] Change awk statement  in LIT test again

---
 llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml b/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
index c48ba28d8c42b..47f413c74480b 100644
--- a/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
+++ b/llvm/test/tools/yaml2obj/GOFF/end-long-name.yaml
@@ -1,13 +1,13 @@
-# RUN: awk -e 'BEGIN {str = sprintf("%47s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN: awk 'BEGIN {str = sprintf("%47s", "")} {gsub("@FILL@", str); print}' < %s | \
 # RUN:   yaml2obj | od -v -An -tx1 | \
 # RUN:   FileCheck --ignore-case --check-prefix CHECK1 %s
-# RUN: awk -e 'BEGIN {str = sprintf("%48s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN: awk 'BEGIN {str = sprintf("%48s", "")} {gsub("@FILL@", str); print}' < %s | \
 # RUN:   yaml2obj | od -v -An -tx1 | \
 # RUN:   FileCheck --ignore-case --check-prefix CHECK2 %s
-# RUN: awk -e 'BEGIN {str = sprintf("%125s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN: awk 'BEGIN {str = sprintf("%125s", "")} {gsub("@FILL@", str); print}' < %s | \
 # RUN:   yaml2obj | od -v -An -tx1 | \
 # RUN:   FileCheck --ignore-case --check-prefix CHECK3 %s
-# RUN: awk -e 'BEGIN {str = sprintf("%160s", "")} {gsub("@FILL@", str); print}' < %s | \
+# RUN: awk 'BEGIN {str = sprintf("%160s", "")} {gsub("@FILL@", str); print}' < %s | \
 # RUN:   yaml2obj | od -v -An -tx1 | \
 # RUN:   FileCheck --ignore-case --check-prefix CHECK4 %s
 

>From a778e25388943b14b289d89a39513ca6116d89e9 Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Fri, 7 Jun 2024 13:14:45 -0400
Subject: [PATCH 4/4] Changes based on reviewer comments.

---
 llvm/include/llvm/ObjectYAML/GOFFYAML.h       | 28 +++----
 llvm/lib/ObjectYAML/GOFFEmitter.cpp           | 82 ++++++++++---------
 llvm/lib/ObjectYAML/GOFFYAML.cpp              | 18 ++--
 .../tools/yaml2obj/GOFF/invalid-record.yaml   |  2 +-
 4 files changed, 67 insertions(+), 63 deletions(-)

diff --git a/llvm/include/llvm/ObjectYAML/GOFFYAML.h b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
index 16219058e969b..0b3e1d6f04396 100644
--- a/llvm/include/llvm/ObjectYAML/GOFFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/GOFFYAML.h
@@ -33,40 +33,40 @@ LLVM_YAML_STRONG_TYPEDEF(uint8_t, GOFF_ENDFLAGS)
 // forward references to records, etc.). However, to be able to specify invalid
 // GOFF files, we treat all records the same way.
 struct RecordBase {
-  enum RecordBaseKind {
-    RBK_ModuleHeader,
-    RBK_RelocationDirectory,
-    RBK_Symbol,
-    RBK_Text,
-    RBK_DeferredLength,
-    RBK_EndOfModule
+  enum class Kind {
+    ModuleHeader,
+    RelocationDirectory,
+    Symbol,
+    Text,
+    DeferredLength,
+    EndOfModule
   };
 
 private:
-  const RecordBaseKind Kind;
+  const Kind RecordKind;
 
 protected:
-  RecordBase(RecordBaseKind Kind) : Kind(Kind) {}
+  RecordBase(Kind RecordKind) : RecordKind(RecordKind) {}
 
 public:
-  RecordBaseKind getKind() const { return Kind; }
+  Kind getKind() const { return RecordKind; }
 };
 using RecordPtr = std::unique_ptr<RecordBase>;
 
 struct ModuleHeader : public RecordBase {
-  ModuleHeader() : RecordBase(RBK_ModuleHeader) {}
+  ModuleHeader() : RecordBase(Kind::ModuleHeader) {}
 
   uint32_t ArchitectureLevel;
   uint16_t PropertiesLength;
   std::optional<yaml::BinaryRef> Properties;
 
   static bool classof(const RecordBase *S) {
-    return S->getKind() == RBK_ModuleHeader;
+    return S->getKind() == Kind::ModuleHeader;
   }
 };
 
 struct EndOfModule : public RecordBase {
-  EndOfModule() : RecordBase(RBK_EndOfModule) {}
+  EndOfModule() : RecordBase(Kind::EndOfModule) {}
 
   GOFF_ENDFLAGS Flags;
   GOFF_AMODE AMODE;
@@ -77,7 +77,7 @@ struct EndOfModule : public RecordBase {
   StringRef EntryName;
 
   static bool classof(const RecordBase *S) {
-    return S->getKind() == RBK_EndOfModule;
+    return S->getKind() == Kind::EndOfModule;
   }
 };
 
diff --git a/llvm/lib/ObjectYAML/GOFFEmitter.cpp b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
index a702aefd5076d..90fb588dfe0c7 100644
--- a/llvm/lib/ObjectYAML/GOFFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/GOFFEmitter.cpp
@@ -69,8 +69,7 @@ ZerosImpl zeros(const size_t NumBytes) { return ZerosImpl{NumBytes}; }
 // The implementation aims at simplicity, not speed.
 class GOFFOStream {
 public:
-  explicit GOFFOStream(raw_ostream &OS)
-      : OS(OS), CurrentType(GOFF::RecordType(-1)) {}
+  explicit GOFFOStream(raw_ostream &OS) : OS(OS), CurrentType() {}
 
   GOFFOStream &operator<<(StringRef Str) {
     write(Str);
@@ -95,35 +94,37 @@ class GOFFOStream {
 };
 
 void GOFFOStream::writeRecordPrefix(uint8_t Flags) {
+  // See https://www.ibm.com/docs/en/zos/3.1.0?topic=conventions-record-prefix.
   uint8_t TypeAndFlags = Flags | (CurrentType << 4);
-  OS << binaryBe(static_cast<unsigned char>(GOFF::PTVPrefix))
-     << binaryBe(static_cast<unsigned char>(TypeAndFlags))
-     << binaryBe(static_cast<unsigned char>(0));
+  OS << binaryBe(uint8_t(GOFF::PTVPrefix)) // The prefix value.
+     << binaryBe(uint8_t(TypeAndFlags))    // The record type and the flags.
+     << binaryBe(uint8_t(0));              // The version.
 }
 
 void GOFFOStream::write(StringRef Str) {
   // The flags are determined by the flags of the prvious record, and by the
-  // remaining size of data.
-  uint8_t Flags = 0;
-  size_t Ptr = 0;
+  // size of the remaining data.
+  size_t Pos = 0;
   size_t Size = Str.size();
-  while (Size >= GOFF::RecordContentLength) {
-    if (Flags) {
+  bool Continuation = false;
+  while (Size > 0) {
+    uint8_t Flags = 0;
+    if (Continuation)
       Flags |= Rec_Continuation;
-      if (Size == GOFF::RecordContentLength)
-        Flags &= ~Rec_Continued;
-    } else
-      Flags |= (Size == GOFF::RecordContentLength) ? 0 : Rec_Continued;
-    writeRecordPrefix(Flags);
-    OS.write(&Str.data()[Ptr], GOFF::RecordContentLength);
-    Size -= GOFF::RecordContentLength;
-    Ptr += GOFF::RecordContentLength;
-  }
-  if (Size) {
-    Flags &= ~Rec_Continued;
+    if (Size > GOFF::RecordContentLength) {
+      Flags |= Rec_Continued;
+      Continuation = true;
+    }
     writeRecordPrefix(Flags);
-    OS.write(&Str.data()[Ptr], Size);
-    OS.write_zeros(GOFF::RecordContentLength - Size);
+    if (Size < GOFF::RecordContentLength) {
+      OS.write(&Str.data()[Pos], Size);
+      OS.write_zeros(GOFF::RecordContentLength - Size);
+      Size = 0;
+    } else {
+      OS.write(&Str.data()[Pos], GOFF::RecordContentLength);
+      Size -= GOFF::RecordContentLength;
+    }
+    Pos += GOFF::RecordContentLength;
   }
 }
 
@@ -145,8 +146,8 @@ class LogicalRecord : public raw_svector_ostream {
 };
 
 class GOFFState {
-  void writeHeader(GOFFYAML::ModuleHeader &ModHdr);
-  void writeEnd(GOFFYAML::EndOfModule &EndMod);
+  void writeHeader(const GOFFYAML::ModuleHeader &ModHdr);
+  void writeEnd(const GOFFYAML::EndOfModule &EndMod);
 
   void reportError(const Twine &Msg) {
     ErrHandler(Msg);
@@ -170,7 +171,9 @@ class GOFFState {
   bool HasError;
 };
 
-void GOFFState::writeHeader(GOFFYAML::ModuleHeader &ModHdr) {
+void GOFFState::writeHeader(const GOFFYAML::ModuleHeader &ModHdr) {
+  // See
+  // https://www.ibm.com/docs/en/zos/3.1.0?topic=formats-module-header-record.
   GW.newRecord(GOFF::RT_HDR);
   LogicalRecord LR(GW);
   LR << zeros(45)                          // Reserved.
@@ -181,11 +184,12 @@ void GOFFState::writeHeader(GOFFYAML::ModuleHeader &ModHdr) {
     LR << *ModHdr.Properties; // Module properties.
 }
 
-void GOFFState::writeEnd(GOFFYAML::EndOfModule &EndMod) {
+void GOFFState::writeEnd(const GOFFYAML::EndOfModule &EndMod) {
+  // See https://www.ibm.com/docs/en/zos/3.1.0?topic=formats-end-module-record.
   SmallString<16> EntryName;
   if (std::error_code EC =
           ConverterEBCDIC::convertToEBCDIC(EndMod.EntryName, EntryName))
-    reportError("Conversion error on " + EndMod.EntryName);
+    reportError("conversion to EBCDIC 1047 failed on " + EndMod.EntryName);
 
   GW.newRecord(GOFF::RT_END);
   LogicalRecord LR(GW);
@@ -201,20 +205,20 @@ void GOFFState::writeEnd(GOFFYAML::EndOfModule &EndMod) {
 }
 
 bool GOFFState::writeObject() {
-  for (auto &RecPtr : Doc.Records) {
-    auto *Rec = RecPtr.get();
+  for (const std::unique_ptr<GOFFYAML::RecordBase> &RecPtr : Doc.Records) {
+    const GOFFYAML::RecordBase *Rec = RecPtr.get();
     switch (Rec->getKind()) {
-    case GOFFYAML::RecordBase::RBK_ModuleHeader:
-      writeHeader(*static_cast<GOFFYAML::ModuleHeader *>(Rec));
+    case GOFFYAML::RecordBase::Kind::ModuleHeader:
+      writeHeader(*static_cast<const GOFFYAML::ModuleHeader *>(Rec));
       break;
-    case GOFFYAML::RecordBase::RBK_EndOfModule:
-      writeEnd(*static_cast<GOFFYAML::EndOfModule *>(Rec));
+    case GOFFYAML::RecordBase::Kind::EndOfModule:
+      writeEnd(*static_cast<const GOFFYAML::EndOfModule *>(Rec));
       break;
-    case GOFFYAML::RecordBase::RBK_RelocationDirectory:
-    case GOFFYAML::RecordBase::RBK_Symbol:
-    case GOFFYAML::RecordBase::RBK_Text:
-    case GOFFYAML::RecordBase::RBK_DeferredLength:
-      llvm_unreachable(("Not yet implemented"));
+    case GOFFYAML::RecordBase::Kind::RelocationDirectory:
+    case GOFFYAML::RecordBase::Kind::Symbol:
+    case GOFFYAML::RecordBase::Kind::Text:
+    case GOFFYAML::RecordBase::Kind::DeferredLength:
+      llvm_unreachable("not yet implemented");
     }
     if (HasError)
       return false;
diff --git a/llvm/lib/ObjectYAML/GOFFYAML.cpp b/llvm/lib/ObjectYAML/GOFFYAML.cpp
index cad61b759e01e..f36cae78238b8 100644
--- a/llvm/lib/ObjectYAML/GOFFYAML.cpp
+++ b/llvm/lib/ObjectYAML/GOFFYAML.cpp
@@ -71,26 +71,26 @@ void CustomMappingTraits<GOFFYAML::RecordPtr>::inputOne(
     Elem = std::make_unique<GOFFYAML::EndOfModule>(std::move(End));
   } else if (Key == "RelocationDirectory" || Key == "Symbol" || Key == "Text" ||
              Key == "Length")
-    IO.setError(Twine("Not yet implemented ").concat(Key));
+    IO.setError(Twine("not yet implemented ").concat(Key));
   else
-    IO.setError(Twine("Unknown record type name ").concat(Key));
+    IO.setError(Twine("unknown record type name ").concat(Key));
 }
 
 void CustomMappingTraits<GOFFYAML::RecordPtr>::output(
     IO &IO, GOFFYAML::RecordPtr &Elem) {
   switch (Elem->getKind()) {
-  case GOFFYAML::RecordBase::RBK_ModuleHeader:
+  case GOFFYAML::RecordBase::Kind::ModuleHeader:
     IO.mapRequired("ModuleHeader",
                    *static_cast<GOFFYAML::ModuleHeader *>(Elem.get()));
     break;
-  case GOFFYAML::RecordBase::RBK_EndOfModule:
+  case GOFFYAML::RecordBase::Kind::EndOfModule:
     IO.mapRequired("End", *static_cast<GOFFYAML::EndOfModule *>(Elem.get()));
     break;
-  case GOFFYAML::RecordBase::RBK_RelocationDirectory:
-  case GOFFYAML::RecordBase::RBK_Symbol:
-  case GOFFYAML::RecordBase::RBK_Text:
-  case GOFFYAML::RecordBase::RBK_DeferredLength:
-    llvm_unreachable(("Not yet implemented"));
+  case GOFFYAML::RecordBase::Kind::RelocationDirectory:
+  case GOFFYAML::RecordBase::Kind::Symbol:
+  case GOFFYAML::RecordBase::Kind::Text:
+  case GOFFYAML::RecordBase::Kind::DeferredLength:
+    llvm_unreachable(("not yet implemented"));
   }
 }
 
diff --git a/llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml b/llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml
index 5b1d2b588d76b..d8a25f4aade30 100644
--- a/llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml
+++ b/llvm/test/tools/yaml2obj/GOFF/invalid-record.yaml
@@ -1,6 +1,6 @@
 # RUN: not yaml2obj %s 2>&1 | FileCheck %s
 
-# CHECK: YAML:{{[0-9]*}}:{{[0-9]*}}: error: Unknown record type name Foo
+# CHECK: YAML:{{[0-9]*}}:{{[0-9]*}}: error: unknown record type name Foo
 # CHECk: yaml2obj: error: failed to parse YAML input: Invalid argument
 
 --- !GOFF



More information about the llvm-commits mailing list