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

[Digger via Phabricator via llvm-commits]

DiggerLin marked 6 inline comments as done.
DiggerLin added inline comments.


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


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


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


================
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
----------------
hubert.reinterpretcast wrote:
> Can this be merged with the previous line?
> ```
> SYMS:        Symbol {{[{][[:space:]] *}}Index: [[#Index:]]{{[[:space:]] *}}Name: a{{$}}
> ```
> 
yes ,your suggestion is more reasonable.


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


================
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
----------------
hubert.reinterpretcast wrote:
> Same comment re: merging with the previous line.
changed as suggestion.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66969