[PATCH] D96351: [AIX] Enable the default AltiVec ABI on AIX

[Zarko Todorovski via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:199
   if (TM.isPPC64()) {
-    if (Subtarget.hasAltivec())
-      return SaveR2 ? CSR_PPC64_R2_Altivec_SaveList
-                    : CSR_PPC64_Altivec_SaveList;
+    if (Subtarget.hasAltivec()) {
+      if ((Subtarget.isAIXABI() && TM.getAIXExtendedAltivecABI()) ||
----------------
sfertile wrote:
> Can we instead structure it this way?
> 
> ```
>   if (TM.isPPC64()) {
>     if (Subtarget.hasAltivec() &&
>         (!Subtarget.isAIXABI() || TM.getAIXExtendedAltivecABI())) {
>       return SaveR2 ? CSR_PPC64_R2_Altivec_SaveList
>                     : CSR_PPC64_Altivec_SaveList;
>     }
>     return SaveR2 ? CSR_PPC64_R2_SaveList : CSR_PPC64_SaveList;
>   }
> ```
I think your suggestion is simpler. Thanks, I was looking for ways to make it cleaner. 


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:348
+    // reserved and cannot be used.
+    for (unsigned I = 0;
+         const unsigned E = array_lengthof(CSR_Altivec_SaveList); ++I) {
----------------
sfertile wrote:
> Use a range based for loop.
Thanks for catching this oversight by me. 


================
Comment at: llvm/test/CodeGen/PowerPC/aix-csr-vector.ll:215
+; MIR32-DFL:         fixedStack:
+; MIR32-DFL-NEXT:    - { id: 0, type: spill-slot, offset: -8, size: 8, alignment: 8, stack-id: default, 
+; MIR32-DFL-NEXT:        callee-saved-register: '$f31', callee-saved-restored: true, debug-info-variable: '', 
----------------
sfertile wrote:
> The patch adds a lot of whitespace errors. I'm betting its from this mir output. Please highlight trailing whitespace and fix it up.
They should hopefully be gone now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96351