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

[Digger via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1756
+  bool Ret = AsmPrinter::doFinalization(M);
+  for (auto Sym : IntrinsicSymbols)
+    OutStreamer->emitSymbolAttribute(Sym, MCSA_Extern);
----------------
jhenderson wrote:
> Don't use `auto` here. Also, should this really be a copy and not a const reference?
it store the a pointer of MCSymbol in the IntrinsicSymbols. I think return a copy of pointer is same efficient as const reference. 

and the 

```
bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym,
                                          MCSymbolAttr Attribute)
```


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5284
+      auto &Context = DAG.getMachineFunction().getMMI().getContext();
+      MCSymbolXCOFF *Sym = cast<MCSymbolXCOFF>(
+          Context.getOrCreateSymbol(Twine(".") + Twine(SymName)));
----------------
jhenderson wrote:
> Should this be `const MCSymbolXCOFF *Sym`?
thanks


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:2
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec < %s | FileCheck --check-prefixes=COMMON,BIT32 %s
+
----------------
jhenderson wrote:
> Please don't break the lines up like this. I'd put -mattr=altivec on the same line as the rest of the llc line, but if you do want to break it onto a new line, split it roughly halfway through the arguments, and indent the second line, then put the FileCheck line on a new line. Same goes below.
thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929