[PATCH] D71144: [AIX] Use csect reference for function address constants

[Digger via Phabricator via llvm-commits]

DiggerLin marked an inline comment as done.
DiggerLin added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1767
+            TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GO);
+        GOSym->setStorageClass(SC);
+        MCSectionXCOFF *Csect = OutStreamer->getContext().getXCOFFSection(
----------------
DiggerLin wrote:
> hubert.reinterpretcast wrote:
> > DiggerLin wrote:
> > > Xiangling_L wrote:
> > > > Since this is the case where one MCSectionXCOFF has only one MCSymbolXCOFF, when we want to get `SC`, we can do `MCSymbolXCOFF->getContainingCsect()`? (Please correct me if I am wrong) If so, do we still want to bother to set `SC` for MCSymbolXCOFF in this case?
> > > we will output GOSym into symboltable in the XCOFFObjectWriter::writeSymbolTableEntryForCsectMemberLabel()
> > > it get the storageclass directly 
> > >  W.write<uint8_t>(SymbolRef.getStorageClass()); 
> > Thanks for point this out, @DiggerLin. It sounds like this change may introduce a difference how the symbol table entries for the csect and the label are produced. Please look into adding appropriate testing. Note that the status quo in `llvm/test/CodeGen/PowerPC/aix-func-dsc-gen.ll` does not use the same storage class for the csect and the label. Note also that it is not generally okay to use the same storage class for the csect and the label (https://reviews.llvm.org/D71125?id=232583#inline-646944).
> > 
> > I believe @daltenty can elaborate on the refactor he mentions (https://reviews.llvm.org/D71144?id=232631#inline-644323). If I understand correctly, said refactor looks to be unavoidable. If this patch is indeed stuck, please indicate it. I think the "Plan Changes" option in Phabricator is a good way to do so.
> the patch is only for the assembly,  not for the xcoff objectfile output.
> 
> because we do not invoke a getAssembler().registerSection(*Section) and 
> getAssembler().registerSymbol(*Symbol)  in the  patch.
> it can not output the  foo (extern function) symbol entry in the symbol table in xcoff objectfile.
> and  it also can not output the  csect in the symbol table(it is not necessary need csect symbol output here ).
> 
> I will create a new patch for to output the  foo (extern function) symbol entry in the symbol table in xcoff objectfile later.
> 
> And I will change the storage class for SC to HIDDEN  here.
> 
>   
sorry , change the storage class from SC to HIDDEN will have problem.  since we look external symbol as csect. and we will use  writeSymbolTableEntryForControlSection to write the external function symbol


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71144