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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Please reupload the latest patch with context.



================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:152
 private:
+  /// LLVM extern intrinsic xcoff symbols used for emitting the linkage of these
+  /// symbols.
----------------
Should it be spelt XCOFF?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1756
+  bool Ret = AsmPrinter::doFinalization(M);
+  for (auto Sym : IntrinsicSymbols)
+    OutStreamer->emitSymbolAttribute(Sym, MCSA_Extern);
----------------
Don't use `auto` here. Also, should this really be a copy and not a const reference?


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


================
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
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929