[PATCH] D95505: [yaml2obj] Initial support for 32-bit XCOFF in yaml2obj.

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:241
+      }
+      W.OS.write_zeros(PaddingSize);
+      for (const XCOFFYAML::Relocation &YamlRel : YamlSec.Relocations) {
----------------
We don't have to call `write_zeros` if `PaddingSize` is 0?


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:264
+    W.write<uint32_t>(YamlSym.Value);
+    W.write<int16_t>(SectionIndexMap[YamlSym.SectionName]);
+    W.write<uint16_t>(YamlSym.Type);
----------------
What about a symbol that does not have belonged sections? For example undefined external symbols?


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml:19
+  - Flags:           [ STYP_BSS ]
+  - Name:            .dwabrev
+    Address:         0x0
----------------
use another section name that is not DWARF specific?


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml:16
+    Relocations:
+      - Address:     0x3A
+        Type:        0x02
----------------
Is it ok that the relocation address is not in the range of `.data` section?


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/full-contents.yaml:36
+    Relocations:
+      - Address:         0x80
+        Symbol:          0x21
----------------
Same as above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95505