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

[EsmeYi via Phabricator via llvm-commits]

Esme added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:67
+bool XCOFFWriter::nameShouldBeInStringTable(StringRef SymbolName) {
+  return (SymbolName.size() > XCOFF::NameSize);
+}
----------------
jhenderson wrote:
> You can drop the parentheses. Do you need to allow space for a null terminator, or does `NameSize` take that into account?
Well, it seems that NameSize should have taken this into account.


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml:11
+  - Name:          .text
+    Flags:         0x20
+    SectionData:   "9061FFF8808200008064000038630001906400008061FFF8386300019061FFF88061FFF88082000480840000888400007C6322149061FFF88061FFF84E8000200000000000092040800001010000000000000040000466756E310000600000007C0802A693E1FFFC900100089421FFB07C3F0B7838600000907F004880620008907F0044808300003884000190830000806300004BFFFF6D60000000807F0044806300004BFFFF5D60000000382100508001000883E1FFFC7C0803A64E8000200000000000092261800100010000006000046D61696E1F006162636400000000"
----------------
jhenderson wrote:
> Is there an enum you could use to represent this set of flags? It would be preferable to be able write either of the following (flag values are placeholders):
> ```
> Flags: Exec
> ```
> or probably
> ```
> Flags: [Exec, Alloc]
> ```
> although `Flags: 0x20` (or possibly `Flags: [0x20]`) should probably still be permitted.
Yes, we have the enum SectionTypeFlags. How about marking this as a TODO for follow-up work? Because this will have an impact on other tools, like obj2yaml.
```
enum SectionTypeFlags : int32_t {
  STYP_PAD = 0x0008,
  STYP_DWARF = 0x0010,
  STYP_TEXT = 0x0020,
  STYP_DATA = 0x0040,
  STYP_BSS = 0x0080,
  STYP_EXCEPT = 0x0100,
  STYP_INFO = 0x0200,
...
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95505