[PATCH] D69101: [AIX] Refactor AIX Call Lowering to use CCState. NFCI.

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm marked 2 inline comments as done.
cebowleratibm added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6727
+  // Arguments always reserve parameter save area.
+  switch (LocVT.SimpleTy) {
+  default:
----------------
jasonliu wrote:
> Nit: I know in the caller side of CC functions, we always pass in the same argument for both LocVT and ValVT. But to  be more pedantic and for both paramaters intended usage, should we check the ValVT here instead?
It's more intuitive to the reader, though on AIX ABI LocVT and ValVT will always come in with the same value.  I'll make the suggested update.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-byval-param.ll:1
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
----------------
ZarkoCA wrote:
> jasonliu wrote:
> > add -verify-machine-instr, -mcpu to all the llc command?
> For the calling conventions we've been using -stop-after=machine-cp, so no need for -verify-machine-instr etc at this point. 
> 
> That said, I guess these testcase are here for to generate the error but it would be useful if the RUN command was reflected what we usually do, that way we make sure the tests are uniform if/when we enable these. 
I'm not adding -verify-machine-instr because of -stop-after=machine-cp

These CC tests are not sensitive to cpu level because the ABI must be consistent across all AIX supported cpu.  One thought is to explicitly run the CC tests in all -mcpu, however, I think that's overkill.


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

https://reviews.llvm.org/D69101