[PATCH] D73115: [AIX] Minor cleanup in AsmPrinter. [NFC]

Sean Fertile via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 22 10:05:15 PST 2020


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


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:671
+  // If the functions linkage is not internal then the function descriptor
+  // symbol must have same linkage as the function symbol.
   if (MAI->needsFunctionDescriptors() &&
----------------
Xiangling_L wrote:
> I kinda feel this comment is a little misleading. The linkage of function descriptor symbol and function entry point symbol must be same all the time, no matter the linkage is internal or not. This comment here seems imply that `If the function linkage is internal, then the function descriptor symbol can have different linkage as the function symbol.`? And I vaguely recall that why we choose to omit `.lglobl foo{DS}` when foo is internal is because by default, for csect name, it would be `.lglobl` linkage if we don't explicitly set `'.globl` as assembler manual state:
> 
> 
> > The assembler automatically generates the symbol table entry for any csect name with a class of C_HIDEXT unless there is an
> > explicit .globl pseudo-op applied to the csect name.
> 
> Please correct me if I am wrong.
Thanks, You are right. If we can always emit the linkage why not do so? I understand its redundant because the default is correct anyway, but it slightly simplifies the code. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73115





More information about the llvm-commits mailing list