PowerPC: Mark super regs of reserved regs reserved.

[MatzeB]

Summary

When a register like R1 is reserved, X1 should be reserved as well. This
was already done "manually" when 64bit code was enabled, however using
the markSuperRegs() function on the base register is more convenient and
allows to use the checksAllSuperRegsMarked() function even in 32bit mode
to avoid accidental breakage in the future. markSuperRegs() is used even
on registers without superregs for consistency.

And more importantly for me: It allows me to make progress in
https://reviews.llvm.org/D28881

[hfinkel]

LGTM

[nemanjai]

llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp

254 ↗

(On Diff #85499)

I think it would be nice to have a function to do this just like there's a function to do the reverse. Maybe unmarkSuperRegs()? That way we can accomplish both setting the bits and resetting them for all the super registers with just a single call.

Is there a specific reason such a function was not added when markSuperRegs() was added?

[MatzeB]

llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp

254 ↗

(On Diff #85499)

There simply is no other user for this functionality yet. And actually you even could rewrite the PowerPC code so it doesn't first set and later clear the reserved bits (IMO even looks slightly cleaner than the current code):

diff --git a/lib/Target/PowerPC/PPCRegisterInfo.cpp b/lib/Target/PowerPC/PPCRegisterInfo.cpp
index 5afe412..aad9139 100644
--- a/lib/Target/PowerPC/PPCRegisterInfo.cpp
+++ b/lib/Target/PowerPC/PPCRegisterInfo.cpp
@@ -234,30 +234,21 @@ BitVector PPCRegisterInfo::getReservedRegs(const MachineFunction &MF) const {

   // The SVR4 ABI reserves r2 and r13
   if (Subtarget.isSVR4ABI()) {
-    markSuperRegs(Reserved, PPC::R2);  // System-reserved register
+    // We only reserve r2 if we need to use the TOC pointer. If we have no
+    // explicit uses of the TOC pointer (meaning we're a leaf function with
+    // no constant-pool loads, etc.) and we have no potential uses inside an
+    // inline asm block, then we can treat r2 has an ordinary callee-saved
+    // register.
+    const PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>();
+    if (!TM.isPPC64() || FuncInfo->usesTOCBasePtr() || MF.hasInlineAsm())
+      markSuperRegs(Reserved, PPC::R2);  // System-reserved register
     markSuperRegs(Reserved, PPC::R13); // Small Data Area pointer register
   }

-  if (TM.isPPC64()) {
-    // On PPC64, r13 is the thread pointer. Never allocate this register.
+  // On PPC64, r13 is the thread pointer. Never allocate this register.
+  if (TM.isPPC64())
     markSuperRegs(Reserved, PPC::R13);

-    // The 64-bit SVR4 ABI reserves r2 for the TOC pointer.
-    if (Subtarget.isSVR4ABI()) {
-      // We only reserve r2 if we need to use the TOC pointer. If we have no
-      // explicit uses of the TOC pointer (meaning we're a leaf function with
-      // no constant-pool loads, etc.) and we have no potential uses inside an
-      // inline asm block, then we can treat r2 has an ordinary callee-saved
-      // register.
-      const PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>();
-      if (!FuncInfo->usesTOCBasePtr() && !MF.hasInlineAsm()) {
-        for (MCSuperRegIterator Super(PPC::R2, this, true); Super.isValid();
-             ++Super)
-          Reserved.reset(*Super);
-      }
-    }
-  }
-
   if (TFI->needsFP(MF))
     markSuperRegs(Reserved, PPC::R31);

[hfinkel]

llvm/trunk/lib/Target/PowerPC/PPCRegisterInfo.cpp

254 ↗

(On Diff #85499)

I agree, this is nicer (LGTM).

lib/Target/PowerPC/PPCRegisterInfo.cpp

 
 BitVector PPCRegisterInfo::getReservedRegs(const MachineFunction &MF) const {
   BitVector Reserved(getNumRegs());
   const PPCSubtarget &Subtarget = MF.getSubtarget<PPCSubtarget>();
   const PPCFrameLowering *TFI = getFrameLowering(MF);
 
   // The ZERO register is not really a register, but the representation of r0
   // when used in instructions that treat r0 as the constant 0.
-  Reserved.set(PPC::ZERO);
-  Reserved.set(PPC::ZERO8);
+  markSuperRegs(Reserved, PPC::ZERO);
 
   // The FP register is also not really a register, but is the representation
   // of the frame pointer register used by ISD::FRAMEADDR.
-  Reserved.set(PPC::FP);
-  Reserved.set(PPC::FP8);
+  markSuperRegs(Reserved, PPC::FP);
 
   // The BP register is also not really a register, but is the representation
   // of the base pointer register used by setjmp.
-  Reserved.set(PPC::BP);
-  Reserved.set(PPC::BP8);
+  markSuperRegs(Reserved, PPC::BP);
 
   // The counter registers must be reserved so that counter-based loops can
   // be correctly formed (and the mtctr instructions are not DCE'd).
-  Reserved.set(PPC::CTR);
-  Reserved.set(PPC::CTR8);
+  markSuperRegs(Reserved, PPC::CTR);
+  markSuperRegs(Reserved, PPC::CTR8);
 
-  Reserved.set(PPC::R1);
-  Reserved.set(PPC::LR);
-  Reserved.set(PPC::LR8);
-  Reserved.set(PPC::RM);
+  markSuperRegs(Reserved, PPC::R1);
+  markSuperRegs(Reserved, PPC::LR);
+  markSuperRegs(Reserved, PPC::LR8);
+  markSuperRegs(Reserved, PPC::RM);
 
   if (!Subtarget.isDarwinABI() || !Subtarget.hasAltivec())
-    Reserved.set(PPC::VRSAVE);
+    markSuperRegs(Reserved, PPC::VRSAVE);
 
   // The SVR4 ABI reserves r2 and r13
   if (Subtarget.isSVR4ABI()) {
-    Reserved.set(PPC::R2);  // System-reserved register
-    Reserved.set(PPC::R13); // Small Data Area pointer register
+    markSuperRegs(Reserved, PPC::R2);  // System-reserved register
+    markSuperRegs(Reserved, PPC::R13); // Small Data Area pointer register
   }
 
-  // On PPC64, r13 is the thread pointer. Never allocate this register.
   if (TM.isPPC64()) {
-    Reserved.set(PPC::R13);
-
-    Reserved.set(PPC::X1);
-    Reserved.set(PPC::X13);
-
-    if (TFI->needsFP(MF))
-      Reserved.set(PPC::X31);
-
-    if (hasBasePointer(MF))
-      Reserved.set(PPC::X30);
+    // On PPC64, r13 is the thread pointer. Never allocate this register.
+    markSuperRegs(Reserved, PPC::R13);
 
     // The 64-bit SVR4 ABI reserves r2 for the TOC pointer.
     if (Subtarget.isSVR4ABI()) {
       // We only reserve r2 if we need to use the TOC pointer. If we have no
       // explicit uses of the TOC pointer (meaning we're a leaf function with
       // no constant-pool loads, etc.) and we have no potential uses inside an
       // inline asm block, then we can treat r2 has an ordinary callee-saved
       // register.
       const PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>();
-      if (FuncInfo->usesTOCBasePtr() || MF.hasInlineAsm())
-        Reserved.set(PPC::X2);
-      else
-        Reserved.reset(PPC::R2);
+      if (!FuncInfo->usesTOCBasePtr() && !MF.hasInlineAsm()) {
+        for (MCSuperRegIterator Super(PPC::R2, this, true); Super.isValid();
+             ++Super)
+          Reserved.reset(*Super);
+      }
     }
   }
 
   if (TFI->needsFP(MF))
-    Reserved.set(PPC::R31);
+    markSuperRegs(Reserved, PPC::R31);
 
   bool IsPositionIndependent = TM.isPositionIndependent();
   if (hasBasePointer(MF)) {
     if (Subtarget.isSVR4ABI() && !TM.isPPC64() && IsPositionIndependent)
-      Reserved.set(PPC::R29);
+      markSuperRegs(Reserved, PPC::R29);
     else
-      Reserved.set(PPC::R30);
+      markSuperRegs(Reserved, PPC::R30);
   }
 
   if (Subtarget.isSVR4ABI() && !TM.isPPC64() && IsPositionIndependent)
-    Reserved.set(PPC::R30);
+    markSuperRegs(Reserved, PPC::R30);
 
   // Reserve Altivec registers when Altivec is unavailable.
   if (!Subtarget.hasAltivec())
     for (TargetRegisterClass::iterator I = PPC::VRRCRegClass.begin(),
          IE = PPC::VRRCRegClass.end(); I != IE; ++I)
-      Reserved.set(*I);
+      markSuperRegs(Reserved, *I);
 
+  assert(checkAllSuperRegsMarked(Reserved));
   return Reserved;
 }
 
 unsigned PPCRegisterInfo::getRegPressureLimit(const TargetRegisterClass *RC,
                                               MachineFunction &MF) const {
   const PPCFrameLowering *TFI = getFrameLowering(MF);
   const unsigned DefaultSafety = 1;