[AIX] Remove unused vector registers from allocation order in the default AltiVec ABI

[ZarkoCA]

Summary

The previous implementation of the default AltiVec ABI marked registers V20-V31
as reserved. This failed to prevent reserved VFRC registers being allocated.
In this patch instead of marking the registers reserved we remove unallowed
registers from the allocation order completely.

This is a slight rework of an implementation by @nemanjai

[jsji]

What is the fundamental reason of not fixing the bug in getReservedRegs but doing that by changing allocation order instead ?

[ZarkoCA]

In D100050#2676761, @jsji wrote:

What is the fundamental reason of not fixing the bug in getReservedRegs but doing that by changing allocation order instead ?

We had an offline discussion about this and I decided to go with this approach here for the following reasons:

1. Applying the fix in getReservedRegs should work, however, removing from the allocation order is technically more correct since they are not actually reserved for any use. Instead they cannot be used at all.
2. I am concerned about the different register subclasses that we can accidentally omit using the getReservedRegs
3. In our offline discussion we didn't come to conclusion to which approach is "better" and maybe continuing the discussion here would be good.

[jsji]

Thanks.

1. Applying the fix in getReservedRegs should work, however, removing from the allocation order is technically more correct since they are not actually reserved for any use. Instead they cannot be used at all.

I am not sure why it is *technically more correct* to change allocation order instead of *reservering reserved reg in designed API*.

I believe these regs are exactly what *Reserved Register* defines.

According to AIX ABI: https://www.ibm.com/docs/en/aix/7.2?topic=concepts-aix-vector-programming

VR20:31	Reserved (default mode)
Nonvolatile (extended ABI mode)

When the default Vector enabled mode is used, these registers are reserved and must not be used.
In the extended ABI vector enabled mode, these registers are nonvolatile and their values are preserved across function calls

And the getReservedRegs API is:

/// Returns a bitset indexed by physical register number indicating if a
/// register is a special register that has particular uses and should be
/// considered unavailable at all times, e.g. stack pointer, return address.
/// A reserved register:
/// - is not allocatable
/// - is considered always live
/// - is ignored by liveness tracking
/// It is often necessary to reserve the super registers of a reserved
/// register as well, to avoid them getting allocated indirectly. You may use
/// markSuperRegs() and checkAllSuperRegsMarked() in this case.
virtual BitVector getReservedRegs(const MachineFunction &MF) const = 0;

My understanding of has particular uses is that compiler will work well if there is some special (or unexpected) use of these regs,
but it does not mean that you *HAVE* to have some use in compiler...

2. I am concerned about the different register subclasses that we can accidentally omit using the getReservedRegs

I don't think this is a good reason. We should be able to reserve what we want here. If there is bug preventing us to do so, we should fix it.
For missing VFRC in original implementation, that is a bug , we should be able to fix it by marking all alias regs as reserved.

[ZarkoCA]

In D100050#2677005, @jsji wrote:

Thanks.

1. Applying the fix in getReservedRegs should work, however, removing from the allocation order is technically more correct since they are not actually reserved for any use. Instead they cannot be used at all.

I am not sure why it is *technically more correct* to change allocation order instead of *reservering reserved reg in designed API*.

I believe these regs are exactly what *Reserved Register* defines.

According to AIX ABI: https://www.ibm.com/docs/en/aix/7.2?topic=concepts-aix-vector-programming

VR20:31	Reserved (default mode)
Nonvolatile (extended ABI mode)

When the default Vector enabled mode is used, these registers are reserved and must not be used.
In the extended ABI vector enabled mode, these registers are nonvolatile and their values are preserved across function calls

And the getReservedRegs API is:

/// Returns a bitset indexed by physical register number indicating if a
/// register is a special register that has particular uses and should be
/// considered unavailable at all times, e.g. stack pointer, return address.
/// A reserved register:
/// - is not allocatable
/// - is considered always live
/// - is ignored by liveness tracking
/// It is often necessary to reserve the super registers of a reserved
/// register as well, to avoid them getting allocated indirectly. You may use
/// markSuperRegs() and checkAllSuperRegsMarked() in this case.
virtual BitVector getReservedRegs(const MachineFunction &MF) const = 0;

My understanding of has particular uses is that compiler will work well if there is some special (or unexpected) use of these regs,
but it does not mean that you *HAVE* to have some use in compiler...

I understand what you mean. And a further point toward your argument that you made in our offline discussions is that this is how we treat all AltiVec registers for a target that does not support it, that is, we mark all of them as reserved.

[ZarkoCA]

-use getReservedRegs instead
-add correct callee saved regs for the default Altivec ABI in the AllReg calling convention case

[jsji]

LGTM.

llvm/lib/Target/PowerPC/PPCCallingConv.td

 def CSR_64_AllRegs: CalleeSavedRegs<(add X0, (sequence "X%u", 3, 10),
                                              (sequence "X%u", 14, 31),
                                              (sequence "F%u", 0, 31),
                                              (sequence "CR%u", 0, 7))>;
 
 def CSR_64_AllRegs_Altivec : CalleeSavedRegs<(add CSR_64_AllRegs,
                                              (sequence "V%u", 0, 31))>;
 
+def CSR_64_AllRegs_AIX_Dflt_Altivec : CalleeSavedRegs<(add CSR_64_AllRegs,
+                                             (sequence "V%u", 0, 19))>;
+
 def CSR_64_AllRegs_VSX : CalleeSavedRegs<(add CSR_64_AllRegs_Altivec,
                                          (sequence "VSL%u", 0, 31))>;
 
+def CSR_64_AllRegs_AIX_Dflt_VSX : CalleeSavedRegs<(add CSR_64_AllRegs_Altivec,
+                                         (sequence "VSL%u", 0, 19))>;

llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp

 }
 
 const MCPhysReg*
 PPCRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
   const PPCSubtarget &Subtarget = MF->getSubtarget<PPCSubtarget>();
   if (MF->getFunction().getCallingConv() == CallingConv::AnyReg) {
     if (!TM.isPPC64() && Subtarget.isAIXABI())
       report_fatal_error("AnyReg unimplemented on 32-bit AIX.");
-    if (Subtarget.hasVSX())
+    if (Subtarget.hasVSX()) {
+      if (Subtarget.isAIXABI() && !TM.getAIXExtendedAltivecABI())
+        return CSR_64_AllRegs_AIX_Dflt_VSX_SaveList;
       return CSR_64_AllRegs_VSX_SaveList;
-    if (Subtarget.hasAltivec())
+    }
+    if (Subtarget.hasAltivec()) {
+      if (Subtarget.isAIXABI() && !TM.getAIXExtendedAltivecABI())
+        return CSR_64_AllRegs_AIX_Dflt_Altivec_SaveList;
       return CSR_64_AllRegs_Altivec_SaveList;
+    }
     return CSR_64_AllRegs_SaveList;
   }
 
   // On PPC64, we might need to save r2 (but only if it is not reserved).
   // We do not need to treat R2 as callee-saved when using PC-Relative calls
   // because any direct uses of R2 will cause it to be reserved. If the function
   // is a leaf or the only uses of R2 are implicit uses for calls, the calls
   // will use the @notoc relocation which will cause this function to set the
PPCRegisterInfo::getCalleeSavedRegs(const MachineFunction *MF) const {
   return CSR_SVR432_SaveList;
 }
 
 const uint32_t *
 PPCRegisterInfo::getCallPreservedMask(const MachineFunction &MF,
                                       CallingConv::ID CC) const {
   const PPCSubtarget &Subtarget = MF.getSubtarget<PPCSubtarget>();
   if (CC == CallingConv::AnyReg) {
-    if (Subtarget.hasVSX())
+    if (Subtarget.hasVSX()) {
+      if (Subtarget.isAIXABI() && !TM.getAIXExtendedAltivecABI())
+        return CSR_64_AllRegs_AIX_Dflt_VSX_RegMask;
       return CSR_64_AllRegs_VSX_RegMask;
-    if (Subtarget.hasAltivec())
+    }
+    if (Subtarget.hasAltivec()) {
+      if (Subtarget.isAIXABI() && !TM.getAIXExtendedAltivecABI())
+        return CSR_64_AllRegs_AIX_Dflt_Altivec_RegMask;
       return CSR_64_AllRegs_Altivec_RegMask;
+    }
     return CSR_64_AllRegs_RegMask;
   }
 
   if (Subtarget.isAIXABI()) {
     return TM.isPPC64()
                ? ((Subtarget.hasAltivec() && TM.getAIXExtendedAltivecABI())
                       ? CSR_PPC64_Altivec_RegMask
                       : CSR_PPC64_RegMask)
BitVector PPCRegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   if (Subtarget.isAIXABI() && Subtarget.hasAltivec() &&
       !TM.getAIXExtendedAltivecABI()) {
     //  In the AIX default Altivec ABI, vector registers VR20-VR31 are reserved
     //  and cannot be used.
     for (auto Reg : CSR_Altivec_SaveList) {
       if (Reg == 0)
         break;
       markSuperRegs(Reserved, Reg);
+      for (MCRegAliasIterator AS(Reg, this, true); AS.isValid(); ++AS) {
+        Reserved.set(*AS);
+      }
     }
   }
 
   assert(checkAllSuperRegsMarked(Reserved));
   return Reserved;
 }
 
 bool PPCRegisterInfo::requiresFrameIndexScavenging(const MachineFunction &MF) const {

llvm/test/CodeGen/PowerPC/aix-dfltabi-rsrvd-reg.ll

+;; Test to ensure that we are not using any of the aliased reserved registers
+;; under the Extended Altivec ABI on AIX.
+; RUN: llc -verify-machineinstrs -mcpu=pwr8 -mattr=+altivec \
+; RUN:     -stop-after=machine-cp -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN:   FileCheck %s --check-prefix=DFLABI
+; RUN: llc -verify-machineinstrs -mcpu=pwr8 -mattr=+altivec -vec-extabi\
+; RUN:     -stop-after=machine-cp -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN:   FileCheck %s --check-prefix=EXTABI
+
+define double @dbl_test(double %a, double* %b) local_unnamed_addr {
+entry:
+  %0 = load volatile double, double* %b, align 4
+  %add = fadd double %0, %a
+  store volatile double %add, double* %b, align 4
+  ;; Clobbered all vector and floating point registers. In the default Altivec
+  ;; ABI this forces a register spill since no registers are free to use.
+  tail call void asm sideeffect "nop", "~{v19},~{v18},~{v17},~{v16},~{v15},~{v14},~{v13},~{v12},~{v11},~{v10},~{v9},~{v8},~{v7},~{v6},~{v5},~{v4},~{v3},~{v2},~{v1},~{v0},~{f0},~{f1},~{f2},~{f3},~{f4},~{f5},~{f6},~{f7},~{f8},~{f9},~{f10},~{f11},~{f12},~{f13},~{f14},~{f15},~{f16},~{f17},~{f18},~{f19},~{f20},~{f21},~{f22},~{f23},~{f24},~{f25},~{f26},~{f27},~{f28},~{f29},~{f30},~{f31}"()
+  %mul = fmul double %a, %a
+  %1 = load volatile double, double* %b, align 4
+  %add1 = fadd double %mul, %1
+  store volatile double %add1, double* %b, align 4
+  %2 = load volatile double, double* %b, align 4
+  ret double %2
+}
+
+define <4 x i32> @vec_test(<4 x i32> %a,  <4 x i32>* %b) local_unnamed_addr {
+entry:
+  %0 = load volatile <4 x i32>, <4 x i32>* %b, align 4
+  %add = add <4 x i32> %0, %a
+  store volatile <4 x i32> %add, <4 x i32>* %b, align 4
+  tail call void asm sideeffect "nop", "~{v19},~{v18},~{v17},~{v16},~{v15},~{v14},~{v13},~{v12},~{v11},~{v10},~{v9},~{v8},~{v7},~{v6},~{v5},~{v4},~{v3},~{v2},~{v1},~{v0},~{f0},~{f1},~{f2},~{f3},~{f4},~{f5},~{f6},~{f7},~{f8},~{f9},~{f10},~{f11},~{f12},~{f13},~{f14},~{f15},~{f16},~{f17},~{f18},~{f19},~{f20},~{f21},~{f22},~{f23},~{f24},~{f25},~{f26},~{f27},~{f28},~{f29},~{f30},~{f31}"()
+  %mul = mul <4 x i32> %a, %a
+  %1 = load volatile <4 x i32>, <4 x i32>* %b, align 4
+  %add1 = add <4 x i32> %mul, %1
+  store volatile <4 x i32> %add1, <4 x i32>* %b, align 4
+  %2 = load volatile <4 x i32>, <4 x i32>* %b, align 4
+  ret <4 x i32> %2
+}
+
+; DFLABI-LABEL:   dbl_test
+
+; DFLABI-NOT:     $v20
+; DFLABI-NOT:     $v21
+; DFLABI-NOT:     $v22
+; DFLABI-NOT:     $v23
+; DFLABI-NOT:     $v24
+; DFLABI-NOT:     $v25
+; DFLABI-NOT:     $v26
+; DFLABI-NOT:     $v27
+; DFLABI-NOT:     $v28
+; DFLABI-NOT:     $v29
+; DFLABI-NOT:     $v30
+; DFLABI-NOT:     $v31
+
+; DFLABI-NOT:     $vf20
+; DFLABI-NOT:     $vf21
+; DFLABI-NOT:     $vf22
+; DFLABI-NOT:     $vf23
+; DFLABI-NOT:     $vf24
+; DFLABI-NOT:     $vf25
+; DFLABI-NOT:     $vf26
+; DFLABI-NOT:     $vf27
+; DFLABI-NOT:     $vf28
+; DFLABI-NOT:     $vf29
+; DFLABI-NOT:     $vf30
+; DFLABI-NOT:     $vf31
+
+; DFLABI-NOT:     $vs20
+; DFLABI-NOT:     $vs21
+; DFLABI-NOT:     $vs22
+; DFLABI-NOT:     $vs23
+; DFLABI-NOT:     $vs24
+; DFLABI-NOT:     $vs25
+; DFLABI-NOT:     $vs26
+; DFLABI-NOT:     $vs27
+; DFLABI-NOT:     $vs28
+; DFLABI-NOT:     $vs29
+; DFLABI-NOT:     $vs30
+; DFLABI-NOT:     $vs31
+
+; EXTABI-LABEL:   vec_test
+; EXTABI:         liveins:
+; EXTABI-NEXT:     - { reg: '$f1', virtual-reg: '' }
+; EXTABI-NEXT:     - { reg: '$x4', virtual-reg: '' }
+; EXTABI:         body:             |
+; EXTABI:         bb.0.entry:
+; EXTABI:         liveins: $f1, $x4
+; EXTABI-DAG:     renamable $f0 = XFLOADf64 $zero8, renamable $x4 :: (volatile load 8 from %ir.b, align 4)
+; EXTABI-DAG:     renamable $f0 = nofpexcept XSADDDP killed renamable $f0, renamable $f1, implicit $rm
+; EXTABI-DAG:     renamable $vf31 = nofpexcept XSMULDP killed renamable $f1, renamable $f1, implicit $rm
+; EXTABI:         XFSTOREf64 killed renamable $f0, $zero8, renamable $x4 :: (volatile store 8 into %ir.b, align 4)
+; EXTABI-LABEL:   INLINEASM
+; EXTABI-DAG:     renamable $f0 = XFLOADf64 $zero8, renamable $x4 :: (volatile load 8 from %ir.b, align 4)
+; EXTABI-DAG:     renamable $f0 = nofpexcept XSADDDP killed renamable $vf31, killed renamable $f0, implicit $rm
+; EXTABI-DAG:     XFSTOREf64 killed renamable $f0, $zero8, renamable $x4 :: (volatile store 8 into %ir.b, align 4)
+; EXTABI:         renamable $f1 = XFLOADf64 $zero8, killed renamable $x4 :: (volatile load 8 from %ir.b, align 4)
+
+; DFLABI-LABEL:   vec_test
+
+; DFLABI-NOT:     $v20
+; DFLABI-NOT:     $v21
+; DFLABI-NOT:     $v22
+; DFLABI-NOT:     $v23
+; DFLABI-NOT:     $v24
+; DFLABI-NOT:     $v25
+; DFLABI-NOT:     $v26
+; DFLABI-NOT:     $v27
+; DFLABI-NOT:     $v28
+; DFLABI-NOT:     $v29
+; DFLABI-NOT:     $v30
+; DFLABI-NOT:     $v31
+
+; DFLABI-NOT:     $vf20
+; DFLABI-NOT:     $vf21
+; DFLABI-NOT:     $vf22
+; DFLABI-NOT:     $vf23
+; DFLABI-NOT:     $vf24
+; DFLABI-NOT:     $vf25
+; DFLABI-NOT:     $vf26
+; DFLABI-NOT:     $vf27
+; DFLABI-NOT:     $vf28
+; DFLABI-NOT:     $vf29
+; DFLABI-NOT:     $vf30
+; DFLABI-NOT:     $vf31
+
+; DFLABI-NOT:     $vs20
+; DFLABI-NOT:     $vs21
+; DFLABI-NOT:     $vs22
+; DFLABI-NOT:     $vs23
+; DFLABI-NOT:     $vs24
+; DFLABI-NOT:     $vs25
+; DFLABI-NOT:     $vs26
+; DFLABI-NOT:     $vs27
+; DFLABI-NOT:     $vs28
+; DFLABI-NOT:     $vs29
+; DFLABI-NOT:     $vs30
+; DFLABI-NOT:     $vs31
+
+; EXTABI-LABEL:   vec_test
+
+; EXTABI:         liveins:
+; EXTABI-NEXT:     - { reg: '$v2', virtual-reg: '' }
+; EXTABI-NEXT:     - { reg: '$x3', virtual-reg: '' }
+; EXTABI:         body:             |
+; EXTABI-DAG:     bb.0.entry:
+; EXTABI-DAG:     liveins: $v2, $x3
+; EXTABI-DAG:     renamable $v3 = LXVW4X $zero8, renamable $x3 :: (volatile load 16 from %ir.b, align 4)
+; EXTABI-DAG:     renamable $v31 = COPY $v2
+; EXTABI-DAG:     renamable $v2 = VADDUWM killed renamable $v3, $v2
+; EXTABI-LABEL:   INLINEASM    
+; EXTABI-DAG:     renamable $v2 = LXVW4X $zero8, renamable $x3 :: (volatile load 16 from %ir.b, align 4)
+; EXTABI-DAG:     renamable $v3 = VMULUWM killed renamable $v31, renamable $v31
+; EXTABI-DAG:     renamable $v2 = VADDUWM killed renamable $v3, killed renamable $v2
+; EXTABI-DAG:     STXVW4X killed renamable $v2, $zero8, renamable $x3 :: (volatile store 16 into %ir.b, align 4)
+; EXTABI:         renamable $v2 = LXVW4X $zero8, killed renamable $x3 :: (volatile load 16 from %ir.b, align 4)