[PATCH] D72347: [NFC][XCOFF] Refactor Csect creation into TargetLoweringObjectFile

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/Target/TargetLoweringObjectFile.h:215
 
+  /// On targets that support function descriptors, return a section for the
+  /// descriptor given it's symbol.
----------------
This is for targets that use separate function descriptor symbols, not for targets that support function descriptors in general.


================
Comment at: llvm/include/llvm/Target/TargetLoweringObjectFile.h:221
+
+  /// On targets that support TOC entries, return a section for the entry given
+  /// the symbol it refers to.
----------------
https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html mentions TOC entries. Did we implement this function for the corresponding LLVM implementation?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1803
+  // External global variables are already handled.
+  if (!GV->hasInitializer())
+    return;
----------------
daltenty wrote:
> DiggerLin wrote:
> > If I understand correctly, I do not think we need to change the 
> >   if (!GV->hasInitializer())  to here .
> > 
> > the reason is  if the extern variable is declared in the source code. but never be used, it the declare extern variable do not to show on the xcoff object file even in the symbol table or in the asm file.
> > 
> > if the extern variable is  declared and be used as  initiator , the MCSection will be created in the PPCAIXAsmPrinter::lowerConstant() 
> There are other cases than will be handled by lowerConstant(). We may refer to the external global in a function body, in which case we will need to setup the symbols containing CSect here.
Please add this rationale to the comment about creating the containing csect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72347