[PATCH] D66969: Output XCOFF object text section

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:340
+    const Symbol &SymbolRef, const ControlSection &CSectionRef,
+    const int16_t Index, uint64_t SymbolOffset) {
+  // n_name, n_zeros, n_offset
----------------
jasonliu wrote:
> jasonliu wrote:
> > top level const is not necessary for the Index parameter.
> > Should it be uint16_t?
> > Nit: Index -> SectionIndex 
> > 
> sorry, it should be int16_t.
Top-level `const` still not fixed here.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:338
+
+void XCOFFObjectWriter::writeSymbolTableEntryForSymbol(
+    const Symbol &SymbolRef, const ControlSection &CSectionRef,
----------------
This function is misnamed. It neither handles all variations of the LLVM concept of "symbol" nor the same for the XCOFF concept of "symbol". Maybe use `writeSymbolTableEntryForCsectMemberLabel`.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:373
+void XCOFFObjectWriter::writeSymbolTableEntryForControlSection(
+    const ControlSection &CSectionRef, const int16_t SectionIndex,
+    XCOFF::StorageClass StorageClass) {
----------------
Ditto re: top-level `const`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-return55.ll:7
+  ret i32 55
+; CHECK-LABEL: foo
+; CHECK: li 3, 55
----------------
`foo:` with the colon. `.foo:` if possible.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66969