[PATCH] D96415: [PowerPC][AIX] Support passing vector arguments on the stack.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6402-6403
     else {
-      report_fatal_error(
-          "passing vector parameters to the stack is unimplemented for AIX");
+      const unsigned VecSize = 16;
+      const unsigned Offset = State.AllocateStack(VecSize, Align(VecSize));
+      State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset, LocVT, LocInfo));
----------------
ZarkoCA wrote:
> Do we need the `VecSize` variable here instead of just using `16`?
While it would still be obvious what the 16 represented if used directly in the allocate stack call I think its still better to name it.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-vec-arg-spills-mir.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=+altivec -vec-extabi \
----------------
ZarkoCA wrote:
> Does the script work with AIX now? It would make things a lot easier. 
It did for this test. if you hit any cases where it fails to work let me know and we can fix it.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-vector-stack.ll:1
-; RUN: not --crash llc < %s -verify-machineinstrs -mcpu=pwr7 -mattr=+altivec \
-; RUN:   -vec-extabi -mtriple powerpc-ibm-aix-xcoff 2>&1 | \
-; RUN: FileCheck %s --check-prefix=AIX-ERROR
+; RUN: llc -verify-machineinstrs -mcpu=pwr7 -mattr=+altivec \
+; RUN:     -vec-extabi -mtriple powerpc-ibm-aix-xcoff < %s | \
----------------
ZarkoCA wrote:
> I think we can remove this test? The ones you added do a much better job of showing what the ABI does. 
I think its worthwhile to keep this test around too. The other ones are complex and take a long time to read and understand where everything gets mapped and why. This one shows exactly how we spill the vectors in a simple as possible way. If something breaks looking at the output of this test is much easier than having to wade through the other more thorough tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96415