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

Mon May 25 14:28:59 PDT 2020

DiggerLin marked 4 inline comments as done.
DiggerLin added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1733
+  case PPC::BL8_NOP:
+  case PPC::BL_NOP:
+    const MachineOperand &MO = MI->getOperand(0);
----------------
sfertile wrote:
> sfertile wrote:
> > What is the state of tail-calls on AIX? I know we can't tail call in many of the situations where we can on Linux, however if they aren't completely disabled we might need to add the tail-call opcodes here for similar transformation.
> You have added `PPC::TAILB` so I am assuming that means we do support tail-calls on AIX. Why do we not need to add any of the other tail call pseudos. At the very least i would expect we need `PPC::TAILB8` for 64-bit support if we expect TAILB for 32-bit. What about `TAILBA[8]` or `TAILBCTR[8]`?
as discussed with Sean offline, since wo do not support tail call for the extern symbol now . I added a report_fatal_error("Tail call for extern symbol not yet implemented.");

================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:153
+  /// Symbols lowered from ExternalSymbolSDNodes.
+  SmallPtrSet<MCSymbol *, 8> IntrinsicSymbols;
+
----------------
sfertile wrote:
> Not all intrinsics get handled this way, and not all of these symbols necessarily come from lowering an intrinsic: so we shouldn't be using 'Intrinsic' in the naming. I suggested 'ExternalSymbols' above but that sprobably  too generic.
thanks . change to "ExtSymSDNodeSymbols" as you discussed on the slack

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929