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

EsmeYi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 18 20:29:44 PDT 2021


Esme added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/XCOFF.h:269
 
+struct FileHeader64 {
+  uint16_t Magic;
----------------
Higuoxing wrote:
> 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;
> };
> ```
It seems we could remove these structs and define size constants directly.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:231
+      W.write<int32_t>(YamlSec.Flags);
+      W.OS.write_zeros(4);
+    } else {
----------------
Higuoxing wrote:
> It looks that `char Padding[4];` isn't used here?
Here we padded 4 bytes in the end of SectionsHeader64, which is a reserved field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100375



More information about the llvm-commits mailing list