[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

Tue Oct 6 13:05:49 PDT 2020

ZarkoCA marked 6 inline comments as done.
ZarkoCA added inline comments.

================
Comment at: llvm/test/CodeGen/PowerPC/aix-csr-vector.ll:2
+; RUN: llc -mtriple=powerpc-unknown-aix-xcoff -verify-machineinstrs \
+; RUN: -mcpu=pwr7 -mattr=+altivec -stop-after=prologepilog < %s | \
+; RUN: FileCheck --check-prefix=MIR32 %s
----------------
Xiangling_L wrote:
> sfertile wrote:
> > Minor nit: align this with the first argument in the preceeding line.
> The ABI mentioned AIX5.3 is the first AIX release to enable vector programming, and there are arch like pwr4 is not compatible with altivec. Since this is our first altivec patch, it looks it's the right place to add `report_fatal_error` for arch level which doesn't support altivec.
While I think that's a good suggestion, none of the other PPC targets do anything similar.  If you choose an arch that doesn't support altivec while selecting a CPU that doesn't support it they quietly don't generate the altivec instructions.  

Also, as things are, we do have a report fatal error when ever someone tries using vector types in the front end and in the back end.  

================
Comment at: llvm/test/CodeGen/PowerPC/aix-csr-vector.ll:3
+; RUN: -mcpu=pwr7 -mattr=+altivec -stop-after=prologepilog < %s | \
+; RUN: FileCheck --check-prefix=MIR32 %s
+
----------------
Xiangling_L wrote:
> sfertile wrote:
> >  I'm not sure the formatting we have adopted for the tests, but I think subsequent commands were to be indented by 2? @Xiangling_L or @DiggerLin  might know better, I think it was on one of their reviews where I saw it mentioned.
> The original formatting comment is here: https://reviews.llvm.org/D78929#inline-740137
Thanks for info, I updated the RUN steps.  

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88676