Fix function pointer tail calls in armv8-M.base

[pbarrio]

Summary

The compiler fails with the following error message:

fatal error: error in backend: ran out of registers during
register allocation

Tail call optimization for Armv8-M.base fails to meet all the required
constraints when handling calls to function pointers where the
arguments take up r0-r3. This is because the pointer to the
function to be called can only be stored in r0-r3, but these are
all occupied by arguments. This patch makes sure that tail call
optimization does not try to handle this type of calls.

[olista01]

Could you also add a test for the (i32, i64) case? We can currently emit the tail call for that (because r1 is not used).

lib/Target/ARM/ARMISelLowering.cpp

2289 ↗

(On Diff #125089)

Why does it matter that the callee comes from a load? This code also triggers the error, without needing to load the function pointer:

define void @_Z3fooPFviiiiE(void (i32, i32, i32, i32)* nocapture %bar) local_unnamed_addr #0 {
entry:
  tail call void %bar(i32 0, i32 0, i32 0, i32 0)
  ret void
}

test/CodeGen/ARM/v8m-tail-call.ll

55 ↗

(On Diff #125089)

These tests should check that a blx is being emitted.

[pbarrio]

Use !isa<GlobalAddressSDNode> to check if the call is through a function pointer

A non-pointer function call will always use a GlobalAddressSDNode; checking this
guarantees that the condition is met for all function pointers. Using
isa<LoadSDNode> only works when the pointer is dereferenced right before doing
the call, which only fixed part of the problem.

[pbarrio]

I have also updated the existing tests and added a couple more.

[efriedma]

Why are we running out of registers here? We should copy the function pointer to r12, like we do for Thumb2 or ARM.

[efriedma]

On a sort-of-related note, we currently miscompile the following:

void a(int (*p)(int,int,int,int)) {
  register int (*x)(int,int,int,int) asm("r0") = p;
  asm("ldr r0, =g":"=r"(x));
  x(1,2,3,4);
}

[efriedma]

Looked a bit more; we're running out of registers because we don't consider r12 an allocatable register in Thumb1 mode. Changing that would be a project way outside the scope of this bugfix, so this is fine as-is for now. LGTM.

[pbarrio]

@efriedma , @olista01 , thank you for the reviews. I will commit upstream now.

llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

ARMTargetLowering::IsEligibleForTailCallOptimization(SDValue Callee,
                                     const SmallVectorImpl<ISD::InputArg> &Ins,
                                                      SelectionDAG& DAG) const {
   MachineFunction &MF = DAG.getMachineFunction();
   const Function *CallerF = MF.getFunction();
   CallingConv::ID CallerCC = CallerF->getCallingConv();
 
   assert(Subtarget->supportsTailCall());
 
+  // Tail calls to function pointers cannot be optimized for Thumb1 if the args
+  // to the call take up r0-r3. The reason is that there are no legal registers
+  // left to hold the pointer to the function to be called.
+  if (Subtarget->isThumb1Only() && Outs.size() >= 4 &&
+      !isa<GlobalAddressSDNode>(Callee.getNode()))
+      return false;
+
   // Look for obvious safe cases to perform tail call optimization that do not
   // require ABI changes. This is what gcc calls sibcall.
 
   // Exception-handling functions need a special set of instructions to indicate
   // a return to the hardware. Tail-calling another function would probably
   // break this.
   if (CallerF->hasFnAttribute("interrupt"))
     return false;

llvm/trunk/test/CodeGen/ARM/v8m-tail-call.ll

define hidden i32 @f2(i32, i32, i32, i32, i32) {
   ret i32 %11
 ; CHECK-LABEL: f2
 ; CHECK:      ldr     [[POP:r[4567]]], [sp, #12]
 ; CHECK-NEXT: mov     lr, [[POP]]
 ; CHECK-NEXT: pop     {{.*}}[[POP]]
 ; CHECK-NEXT: add     sp, #4
 ; CHECK-NEXT: b       h2
 }
+
+; Make sure that tail calls to function pointers that require r0-r3 for argument
+; passing do not break the compiler.
+@fnptr = global i32 (i32, i32, i32, i32)* null
+define i32 @test3() {
+; CHECK-LABEL: test3:
+; CHECK: blx {{r[0-9]+}}
+  %1 = load i32 (i32, i32, i32, i32)*, i32 (i32, i32, i32, i32)** @fnptr
+  %2 = tail call i32 %1(i32 1, i32 2, i32 3, i32 4)
+  ret i32 %2
+}
+
+@fnptr2 = global i32 (i32, i32, i64)* null
+define i32 @test4() {
+; CHECK-LABEL: test4:
+; CHECK: blx {{r[0-9]+}}
+  %1 = load i32 (i32, i32, i64)*, i32 (i32, i32, i64)** @fnptr2
+  %2 = tail call i32 %1(i32 1, i32 2, i64 3)
+  ret i32 %2
+}
+
+; Check that tail calls to function pointers where not all of r0-r3 are used for
+; parameter passing are tail-call optimized.
+; test5: params in r0, r1. r2 & r3 are free.
+@fnptr3 = global i32 (i32, i32)* null
+define i32 @test5() {
+; CHECK-LABEL: test5:
+; CHECK: ldr [[REG:r[0-9]+]]
+; CHECK: bx [[REG]]
+; CHECK-NOT: blx [[REG]]
+  %1 = load i32 (i32, i32)*, i32 (i32, i32)** @fnptr3
+  %2 = tail call i32 %1(i32 1, i32 2)
+  ret i32 %2
+}
+
+; test6: params in r0 and r2-r3. r1 is free.
+@fnptr4 = global i32 (i32, i64)* null
+define i32 @test6() {
+; CHECK-LABEL: test6:
+; CHECK: ldr [[REG:r[0-9]+]]
+; CHECK: bx [[REG]]
+; CHECK-NOT: blx [[REG]]
+  %1 = load i32 (i32, i64)*, i32 (i32, i64)** @fnptr4
+  %2 = tail call i32 %1(i32 1, i64 2)
+  ret i32 %2
+}
+
+; Check that tail calls to functions other than function pointers are
+; tail-call optimized.
+define i32 @test7() {
+; CHECK-LABEL: test7:
+; CHECK: b bar
+; CHECK-NOT: bl bar
+  %tail = tail call i32 @bar(i32 1, i32 2, i32 3, i32 4)
+  ret i32 %tail
+}
+
+declare i32 @bar(i32, i32, i32, i32)