[PATCH] D71125: [AIX] Avoid unset csect assert for functions defined after their use in TOC

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 9 16:34:41 PST 2019


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1740
       CurrentFnDescSym->getName(), XCOFF::XMC_DS, XCOFF::XTY_SD,
-      XCOFF::C_HIDEXT, SectionKind::getData());
+      XCOFF::C_EXT, SectionKind::getData());
   cast<MCSymbolXCOFF>(CurrentFnDescSym)->setContainingCsect(FnDescSec);
----------------
Xiangling_L wrote:
> DiggerLin wrote:
> > for global ,I checked the gcc generated Functions Description as HIDDEN.
> > for static function, I checked both gcc and xlc generated Functions Description as HIDDEN.
> For static function, Digger is right, function descriptor symbol should have internal storage type. 
> 
> But for external linkage function, I think what Digger mentioned `I checked the gcc generated Functions Description as HIDDEN` should be interpreted as `GCC generates two symbols for function descriptor, one is .csect foo[DS]`, the `SD` symbol has `C_HIDEXT` storage type, the other one is `foo` as a label within the csect which has `C_EXT` SC?  And the label one plays the important role in GCC case to expose foo's function descriptor when the address of function is taken.
> 
> So in LLVM,  since we use the same symbol to represent the `foo` in `.csect foo[DS]` and the `foo` as label, I think the storage class here should follow the linkage type of function symbol, like:
> 
> 
> ```
> const XCOFF::StorageClass SC =
>             TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GO);
> 
>  MCSectionXCOFF *FnDescSec = OutStreamer->getContext().getXCOFFSection(
>       CurrentFnDescSym->getName(), XCOFF::XMC_DS, XCOFF::XTY_SD,
>       SC, SectionKind::getData());
> ```
I agree with @Xiangling_L. The storage class should match that of the function symbol itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71125





More information about the llvm-commits mailing list