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

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:202
+                         ? Obj.Header.NumberOfSymTableEntries
+                         : InitFileHdr.NumberOfSymTableEntries);
+  } else {
----------------
number of entries in symbol table should be after `SymbolTableOffset`. This should align with XCOFF32?


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:308
   }
   if (PaddingSize > 0)
     W.OS.write_zeros(PaddingSize);
----------------
delete the check to align with others


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:338
       // storage class (n_sclass) and type (n_type) of the symbol table entry.
       W.OS.write_zeros(18);
     }
----------------
Would it be more sense to use the defined const `SymbolTableEntrySize` for 18?


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml:73
+# CHECK64-NEXT:     Index: 3
+# CHECK64-NEXT:     Name: .data
+# CHECK64-NEXT:     PhysicalAddress: 0x0
----------------
The physicaladdress and the virtualaddress should be not 0 as the previous section's size is 8?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100375