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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5244
+    return cast<MCSymbolXCOFF>(
+        Context.getOrCreateSymbol(Twine(".") + Twine(SymName)));
+  };
----------------
Should the name be adjusted to match that of the function introduced by https://reviews.llvm.org/D80402? @jasonliu?


================
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
----------------
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?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:8
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN:   -mattr=-altivec -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj --symbols %t.o | FileCheck --check-prefix=CHECKSYM %s
----------------
We've been indenting options to line up with options on the previous line. That usually avoids indenting by only two spaces (which we may want to reserve for indentation after things like shell operators).


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:12
+; RUN: not --crash llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc64-ibm-aix-xcoff \
+; RUN:   -mattr=-altivec -filetype=obj -o %t.o 2>&1 < %s | \ 
+; RUN:   FileCheck --check-prefix=XCOFF64 %s
----------------
Same comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929