[PATCH] D100375: [yaml2obj] Enable support for parsing 64-bit XCOFF.

[Xing GUO via Phabricator via llvm-commits]

Higuoxing added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:269
 
+struct FileHeader64 {
+  uint16_t Magic;
----------------
It looks that `FileHeader32` and `FileHeader64` are very similar. Instead of creating a seperate struct, can we change the type of `SymbolTableFileOffset` to `uint64_t` in `FileHeader32`, and rename it to `FileHeader`?

```
struct FileHeader {
  uint16_t Magic;
  uint16_t NumberOfSections;
  int32_t TimeStamp;
  uint64_t SymbolTableFileOffset;
  int32_t NumberOfSymbolTableEntries;
  uint16_t AuxiliaryHeaderSize;
  uint16_t Flags;
};
```


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:292
 
+struct SectionHeader64 {
+  char Name[XCOFF::NameSize];
----------------
Ditto.

```
struct SectionHeader {
  char Name[XCOFF::NameSize];
  uint64_t PhysicalAddress;
  uint64_t VirtualAddress;
  uint64_t Size;
  uint64_t FileOffsetToData;
  uint64_t FileOffsetToRelocations;
  uint64_t FileOffsetToLineNumbers;
  uint32_t NumberOfRelocations;
  uint32_t NumberOfLineNumbers;
  int32_t Flags;
  char Padding[4];
};
```


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:40
 private:
+  bool nameShouldBeInStringTable(const StringRef &);
   bool initFileHeader(uint64_t CurrentOffset);
----------------
> StringRef is small and pervasive enough in LLVM that it should always be passed by value. (https://bcain-llvm.readthedocs.io/projects/llvm/en/latest/ProgrammersManual/#the-stringref-class)


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:231
+      W.write<int32_t>(YamlSec.Flags);
+      W.OS.write_zeros(4);
+    } else {
----------------
It looks that `char Padding[4];` isn't used here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100375/new/

https://reviews.llvm.org/D100375