[PATCH] D66969: Output XCOFF object text section

[Digger via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:343
+  writeSymbolName(SymbolRef.getName());
+  W.write<uint32_t>(SymbolOffset);
+  W.write<int16_t>(SectionRef.Index);
----------------
jasonliu wrote:
> The symbol Address should be: CSectionRef.Address + SymbolOffset
 good find , thanks, I will fix it


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:348
+  W.write<uint32_t>(SymbolOffset);
+  W.write<int16_t>(SectionRef.Index);
+  // Basic/Derived type. See the description of the n_type field for symbol
----------------
jasonliu wrote:
> DiggerLin wrote:
> > jasonliu wrote:
> > > Do we need to pass in a reference to SectionRef if all we want is an 'Index' from it?
> > I think pass an SectionRef reference is same efficient as passing index 
> It's not about efficiency. I think it's overkill to pass in a reference to SectionRef if all we need is 'Section Index'.
changed as suggestion.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll:18
 @f = common local_unnamed_addr global float 0.000000e+00, align 4
+ at comm = common local_unnamed_addr global double 0.000000e+00, align 8
 
----------------
jasonliu wrote:
> DiggerLin wrote:
> > jasonliu wrote:
> > > Not sure how this change is related to this patch.
> > I think it is not matter here, If you strong want me to delete it , I can delete it.
> I prefer to have the patch reflect exactly what we need to change or test to achieve certain functionality. The extra bit here confuses people. 
deleted it as suggestion


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66969