[PATCH] D66969: Output XCOFF object text section header and symbol entry for program code

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added a comment.

Just some minor comments. I think this is almost ready.



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:150
 
+  bool nameShouldBeInStringTable(const StringRef &);
+  void writeSymbolName(const StringRef &);
----------------
This should be a static member function or a non-member function.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:361
+
+  W.write<uint32_t>(CSectionRef.Address + SymbolOffset);
+  W.write<int16_t>(SectionIndex);
----------------
Maybe check for overflow here.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:481
+      writeSymbolTableEntryForCsectMemberLabel(
+          Sym, Csect, Text.Index, Layout.getSymbolOffset(*(Sym.MCSym)));
+    }
----------------
Please remove the excess parentheses.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll:84
 ; SYMS-NEXT:   Symbol {
-; SYMS-NEXT:     Index: [[#Index:]]
-; SYMS-NEXT:     Name: a
+; SYMS:          Index: [[#Index:]]{{[[:space:]] *}}Name: a
 ; SYMS-NEXT:     Value (RelocatableAddress): 0x0
----------------
Can this be merged with the previous line?
```
SYMS:        Symbol {{[{][[:space:]] *}}Index: [[#Index:]]{{[[:space:]] *}}Name: a{{$}}
```



================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-lcomm.ll:36
+; OBJ:        Section {
+; OBJ:          Index: 2
 ; OBJ-NEXT:     Name: .bss
----------------
Same comment re: merging with the previous line.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-lcomm.ll:56
 ; SYMS-NEXT:   Symbol {
-; SYMS-NEXT:     Index: [[#Index:]]
-; SYMS-NEXT:     Name: a
+; SYMS:          Index: [[#Index:]]{{[[:space:]] *}}Name: a
 ; SYMS-NEXT:     Value (RelocatableAddress): 0x0
----------------
Same comment re: merging with the previous line.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66969