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

[Zarko Todorovski via Phabricator via llvm-commits]

ZarkoCA marked 2 inline comments as done.
ZarkoCA added a comment.

In D96351#2580598 <https://reviews.llvm.org/D96351#2580598>, @nemanjai wrote:

> Furthermore, I think you should have a test case where you clobber all FPR's and VSR's **except** the ones in the `vr20 - vr31` range in functions that perform FP/vector computations with checks that show the affected registers are not allocated.

I added `all_fprs_and_vecregs` and then checked to see that vr20-31 are not allocated.  The check is only in MIR since there is no way to distinguish float, vector, and gprs in assembly on AIX by register number alone (we don't have ppc-full-reg-names on AIX yet).



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:15165-15166
+    if (R.first >= (PPC::V0 + 20) && R.second == &PPC::VSRCRegClass)
+      errs() << "warning: vector registers 20 to 32 are reserved in the "
+                "default AIX AltiVec ABI and cannot be used\n";
+  }
----------------
hubert.reinterpretcast wrote:
> Is there a lot of precedent for "raw output" to stderr for warnings from the back-end? Can we diagnose this closer to where `err_asm_unknown_register_name` is handled in the front-end?
I've seen some cases in other targets but, no, there is not a lot of precedent for this in  PPC subtarget. 

I investigated with making the suggested change where `err_asm_unknown_register_name` is handled in clang but it seems to me making the changes there is at least a patch of its own.  I wasn't able to find an efficient way to do it for now using that method.  

If you feel strongly about this, I could remove the warning here and then make a follow on patch to add the warning the front end. My own feeling is that the warning should come from the frond end and this is a comprise for now.


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:348
+
+      markSuperRegs(Reserved, CSR_Altivec_SaveList[I]);
+    }
----------------
nemanjai wrote:
> Since this effectively shrinks the register class by 12 registers, it is worth double checking that marking these registers as reserved without providing an alternate allocation order does not adversely affect register pressure estimates.
Is the way to test this to check the number of spills we get when using the different Altivec ABIs? In the test case `all_fprs_and_vecregs` I just added in `llvm/test/CodeGen/PowerPC/aix-csr-vector.ll` I didn't notice a difference in the number of registers spills.  Would adding a check for that be sufficient? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96351