[PATCH] D78929: [AIX][XCOFF]emit extern linkage for the llvm intrinsic symbol

Fri May 29 07:34:26 PDT 2020

hubert.reinterpretcast added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1734
+  case PPC::BL_NOP:
+  case PPC::BL_TLS:
+  case PPC::BL8_TLS:
----------------
DiggerLin wrote:
> sfertile wrote:
> > We haven't implemented TLS on AIX yet, and I'm not sure which if any of these call pseudos will be used. How about adding them  as a separate case with a report_fatal_error which can be revisited as part of TLS implementation on AIX.
> I will delete the TLS related  opcode.
Can we have an update on why the TLS call pseudos were simply removed instead of being tracked by `report_fatal_error` as @sfertile suggested? I think that leaving some breadcrumbs in the code would be useful.

================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1763
+bool PPCAIXAsmPrinter::doFinalization(Module &M) {
+  bool Ret = AsmPrinter::doFinalization(M);
+  for (MCSymbol *Sym : ExtSymSDNodeSymbols)
----------------
Please use the immediate base class name to refer to the base class implementation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929