[ARM] Change a couple of instances of LiveRegs.contains to !LiveRegs.available

[dmgreen]

Summary

This changes a couple of calls to LiveRegs.contains to !LiveRegs.available, one in Thumb1FrameLoweringInfo (which modifies a test to look more correct to me, given r7 should be the frame pointer so is not available), and another in the ARMLoadStoreOptimizer, that I don't have a test for, it was just found by inspection.

[SjoerdMeijer]

llvm/lib/Target/ARM/Thumb1FrameLowering.cpp

588

I have not looked very long at this (and the context how this is used), but if we want to find an available temporary, then the check:

"the live physical registers UsedRegs does not contain Reg"

sounds about right to me.

So can you perhaps explain the problem and why this is wrong?

[dmgreen]

llvm/lib/Target/ARM/Thumb1FrameLowering.cpp

588

The docs of contains() say it all:

/// Note: Returns false if just some sub registers are live, use available()
/// when searching a free register.

available() will also check for reserved regs, like r7 under the thumb1 test. The reason I was looking at this was because of the subreg liveness. RDA needed a number of fixes to make it correct. This case possibly does not matter as much, with GPR regs not having subregs, but available() seems like the more correct function to use.

[SjoerdMeijer]

llvm/lib/Target/ARM/Thumb1FrameLowering.cpp

588

Ah, thanks, makes sense!

Looks like a good fix to me!

llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp

 /// Return the first register of class \p RegClass that is not in \p Regs.
 unsigned ARMLoadStoreOpt::findFreeReg(const TargetRegisterClass &RegClass) {
   if (!RegClassInfoValid) {
     RegClassInfo.runOnMachineFunction(*MF);
     RegClassInfoValid = true;
   }
 
   for (unsigned Reg : RegClassInfo.getOrder(&RegClass))
-    if (!LiveRegs.contains(Reg))
+    if (LiveRegs.available(MF->getRegInfo(), Reg))
       return Reg;
   return 0;
 }
 
 /// Compute live registers just before instruction \p Before (in normal schedule
 /// direction). Computes backwards so multiple queries in the same block must
 /// come in reverse order.
 void ARMLoadStoreOpt::moveLiveRegsBefore(const MachineBasicBlock &MBB,

llvm/lib/Target/ARM/Thumb1FrameLowering.cpp

if (CSI.getReg() == ARM::LR)
       return true;
 
   return false;
 }
 
 static void findTemporariesForLR(const BitVector &GPRsNoLRSP,
                                  const BitVector &PopFriendly,
                                  const LivePhysRegs &UsedRegs, unsigned &PopReg,
-                                 unsigned &TmpReg) {
+                                 unsigned &TmpReg, MachineRegisterInfo &MRI) {
   PopReg = TmpReg = 0;
   for (auto Reg : GPRsNoLRSP.set_bits()) {
-    if (!UsedRegs.contains(Reg)) {
+    if (UsedRegs.available(MRI, Reg)) {
       // Remember the first pop-friendly register and exit.
       if (PopFriendly.test(Reg)) {
         PopReg = Reg;
         TmpReg = 0;
         break;
       }
       // Otherwise, remember that the register will be available to
       // save a pop-friendly register.
bool Thumb1FrameLowering::emitPopSpecialFixUp(MachineBasicBlock &MBB,
   // Rebuild the GPRs from the high registers because they are removed
   // form the GPR reg class for thumb1.
   BitVector GPRsNoLRSP =
       TRI.getAllocatableSet(MF, TRI.getRegClass(ARM::hGPRRegClassID));
   GPRsNoLRSP |= PopFriendly;
   GPRsNoLRSP.reset(ARM::LR);
   GPRsNoLRSP.reset(ARM::SP);
   GPRsNoLRSP.reset(ARM::PC);
-  findTemporariesForLR(GPRsNoLRSP, PopFriendly, UsedRegs, PopReg, TemporaryReg);
+  findTemporariesForLR(GPRsNoLRSP, PopFriendly, UsedRegs, PopReg, TemporaryReg,
+                       MF.getRegInfo());
 
   // If we couldn't find a pop-friendly register, try restoring LR before
   // popping the other callee-saved registers, so we could use one of them as a
   // temporary.
   bool UseLDRSP = false;
   if (!PopReg && MBBI != MBB.begin()) {
     auto PrevMBBI = MBBI;
     PrevMBBI--;
     if (PrevMBBI->getOpcode() == ARM::tPOP) {
       UsedRegs.stepBackward(*PrevMBBI);
-      findTemporariesForLR(GPRsNoLRSP, PopFriendly, UsedRegs, PopReg, TemporaryReg);
+      findTemporariesForLR(GPRsNoLRSP, PopFriendly, UsedRegs, PopReg,
+                           TemporaryReg, MF.getRegInfo());
       if (PopReg) {
         MBBI = PrevMBBI;
         UseLDRSP = true;
       }
     }
   }
 
   if (!DoIt && !PopReg && !TemporaryReg)

llvm/test/CodeGen/Thumb/PR35481.ll

 ; CHECK-V4T-NEXT:    .setfp r7, sp
 ; CHECK-V4T-NEXT:    add r7, sp, #0
 ; CHECK-V4T-NEXT:    movs r0, #1
 ; CHECK-V4T-NEXT:    bl h
 ; CHECK-V4T-NEXT:    movs r1, #2
 ; CHECK-V4T-NEXT:    movs r2, #3
 ; CHECK-V4T-NEXT:    movs r3, #4
 ; CHECK-V4T-NEXT:    bl g
-; CHECK-V4T-NEXT:    ldr r7, [sp, #4]
-; CHECK-V4T-NEXT:    mov lr, r7
 ; CHECK-V4T-NEXT:    pop {r7}
-; CHECK-V4T-NEXT:    add sp, #4
+; CHECK-V4T-NEXT:    mov r12, r0
+; CHECK-V4T-NEXT:    pop {r0}
+; CHECK-V4T-NEXT:    mov lr, r0
+; CHECK-V4T-NEXT:    mov r0, r12
 ; CHECK-V4T-NEXT:    bx lr
 ;
 ; CHECK-V8M-LABEL: f:
 ; CHECK-V8M:       @ %bb.0: @ %entry
 ; CHECK-V8M-NEXT:    .save {r7, lr}
 ; CHECK-V8M-NEXT:    push {r7, lr}
 ; CHECK-V8M-NEXT:    .setfp r7, sp
 ; CHECK-V8M-NEXT:    add r7, sp, #0
 ; CHECK-V8M-NEXT:    movs r0, #1
 ; CHECK-V8M-NEXT:    bl h
 ; CHECK-V8M-NEXT:    movs r1, #2
 ; CHECK-V8M-NEXT:    movs r2, #3
 ; CHECK-V8M-NEXT:    movs r3, #4
-; CHECK-V8M-NEXT:    ldr r7, [sp, #4]
-; CHECK-V8M-NEXT:    mov lr, r7
 ; CHECK-V8M-NEXT:    pop {r7}
-; CHECK-V8M-NEXT:    add sp, #4
+; CHECK-V8M-NEXT:    mov r12, r0
+; CHECK-V8M-NEXT:    pop {r0}
+; CHECK-V8M-NEXT:    mov lr, r0
+; CHECK-V8M-NEXT:    mov r0, r12
 ; CHECK-V8M-NEXT:    b g
 entry:
   %call = tail call i32 @h(i32 1)
   %call1 = tail call <4 x i32> @g(i32 %call, i32 2, i32 3, i32 4)
   ret <4 x i32> %call1
 }
 
 declare <4 x i32> @g(i32, i32, i32, i32) local_unnamed_addr
 
 declare i32 @h(i32) local_unnamed_addr
 
 attributes #0 = { "disable-tail-calls"="false" "frame-pointer"="all" }