[ARM] Fix PR35481

[chill]

Summary

The issue is when restoring LR when there are no available low registers after popping the callee-saves and we try to find a temporary register before the pop. In the testcase, we are in front of a pop {r7}, however r7 is not in the pop-friendly set (since it's used as a frame pointer), whereas the code assumes that at least one register in the pop is in that set.

This patch allows r7 to be used, regardless of its use as a frame pointer, and also just falls back to using a high register if, for some reason,
we weren't able to find a suitable temporary reg among the popped ones.

Fixes https://bugs.llvm.org/show_bug.cgi?id=35481

[chill]

Ping?

[chill]

Ping?

[efriedma]

lib/Target/ARM/Thumb1FrameLowering.cpp

618

If you want exactly the set r0...r7, shouldn't you just set that, rather than getting the set of allocatable registers and adding back specific registers? That would simplify the code a bit because findTemporariesForLR would be guaranteed to succeed.

[chill]

lib/Target/ARM/Thumb1FrameLowering.cpp

618

I'd prefer to not depend (or depends less) on which registers are considered allocatable and contain this decision in ARMBaseRegisterInfo::getReservedRegs - to deal with platform specific conventions, possible future options like GCC's -ffixed-reg-N, etc.
That said, I'd like to add back in a similar manner r6/BasePtr.

[chill]

lib/Target/ARM/Thumb1FrameLowering.cpp

618

That said, I'd like to add back in a similar manner r6/BasePtr.

I wasn't able to produce a testcase which would take advantage of having a reserved r6 in that set, so I'll
leave it as is.

[efriedma]

LGTM.

lib/Target/ARM/Thumb1FrameLowering.cpp

618

Okay.

lib/Target/ARM/Thumb1FrameLowering.cpp

bool Thumb1FrameLowering::emitPopSpecialFixUp(MachineBasicBlock &MBB,
   }
 
   // Look for a register that can be directly use in the POP.
   unsigned PopReg = 0;
   // And some temporary register, just in case.
   unsigned TemporaryReg = 0;
   BitVector PopFriendly =
       TRI.getAllocatableSet(MF, TRI.getRegClass(ARM::tGPRRegClassID));
+  // R7 may be used as a frame pointer, hence marked as not generally
+  // allocatable, however there's no reason to not use it as a temporary for
+  // restoring LR.
+  if (STI.useR7AsFramePointer())
+    PopFriendly.set(ARM::R7);
+
   assert(PopFriendly.any() && "No allocatable pop-friendly register?!");
   // 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);
 
-  // If we couldn't find a pop-friendly register, restore LR before popping the
-  // other callee-saved registers, so we can use one of them as a temporary.
+  // 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) {
-      MBBI = PrevMBBI;
-      UsedRegs.stepBackward(*MBBI);
+      UsedRegs.stepBackward(*PrevMBBI);
       findTemporariesForLR(GPRsNoLRSP, PopFriendly, UsedRegs, PopReg, TemporaryReg);
+      if (PopReg) {
+        MBBI = PrevMBBI;
         UseLDRSP = true;
       }
     }
+  }
 
   if (!DoIt && !PopReg && !TemporaryReg)
     return false;
 
   assert((PopReg || TemporaryReg) && "Cannot get LR");
 
   if (UseLDRSP) {
     assert(PopReg && "Do not know how to get LR");

test/CodeGen/Thumb/PR35481.ll

+; RUN: llc -mtriple thumbv4t-eabi    < %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-V4T
+; RUN: llc -mtriple armv8m.base-eabi < %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-V8M
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+
+; Function Attrs: nounwind
+define <4 x i32> @f() local_unnamed_addr #0 {
+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
+; CHECK: ldr r7, [sp, #4]
+; CHECK-NEXT: mov lr, r7
+; CHECK-NEXT: pop {r7}
+; CHECK-NEXT: add sp, #4
+; CHECK-V47: bx lr
+; CHECK-V8M: b g
+}
+
+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" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" }