[PATCH] D78929: [AIX][XCOFF]emit extern linkage for the llvm intrinsic symbol
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 5 01:02:24 PDT 2020
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
More information about the llvm-commits
mailing list