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

[Sean Fertile via Phabricator via llvm-commits]

sfertile accepted this revision.
sfertile added a comment.
This revision is now accepted and ready to land.

Hey Digger, sorry for letting this sit for so long. Feel free to ping reviews periodically to get peoples attention back on them.

Other then a few minor nits, LGTM.



================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:152
 private:
+  /// Symbols lowered from ExternalSymbolSDNodes.
+  SmallPtrSet<MCSymbol *, 8> ExtSymSDNodeSymbols;
----------------
 Minor nit: Mention that we will need the emit extern linkage for them in the comment.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1800
+    if (MI->getOperand(0).isSymbol())
+      report_fatal_error("Tail call for extern symbol not yet implemented.");
+    break;
----------------
nit: I would instead say 'not supported' or 'not expected'. The AIX ABI makes a tail-call to an external symbol really unexpected.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-byval-mem.ll:91
 ; 32BIT-DAG:      $r5 = COPY %2
-; 32BIT-NEXT:     BL_NOP <mcsymbol .memcpy>, csr_aix32, implicit-def dead $lr, implicit $rm, implicit $r3, implicit $r4, implicit $r5, implicit $r2, implicit-def $r1, implicit-def $r3
+; 32BIT-NEXT:     BL_NOP &.memcpy, csr_aix32, implicit-def dead $lr, implicit $rm, implicit $r3, implicit $r4, implicit $r5, implicit $r2, implicit-def $r1, implicit-def $r3
 ; 32BIT-NEXT:     ADJCALLSTACKUP 56, 0, implicit-def dead $r1, implicit $r1
----------------
hubert.reinterpretcast wrote:
> sfertile wrote:
> > Xiangling_L wrote:
> > > Though unrelated to this patch, I am wondering if we can replace `&` to something more meaningful like `extsym` etc. like what we have for MCSymbol as `mcsymbol`?
> > Good suggestion, send a quick email to the llvm-dev list and see if there interest for this, or any objections.
> @sfertile, do we have further news on this?
Not sure, I suggested @Xiangling_L send an email to the dev list to discuss, but never followed whether it happened or not. Either way I think its outside the scope of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929