[ARM] Relax restriction on variadic functions for tailcall optimization

[pbarrio]

Summary

Variadic functions can be treated in the same way as normal functions
with respect to the number and types of parameters.

[pbarrio]

Hello @rengolin , @olista01 ,

Could you have a look at this patch? I see no reason to restrict the tail call optimizations to variadic functions: the AAPCS restrictions seem to be already checked for non-variadic functions. The LLVM regression tests passed, and I have also tested with a few benchmarks and they all seem to work fine.

Please shout if I overlooked something. New test suggestions will be also very welcome.

Thanks,
Pablo

[rengolin]

This was added by Jim in 2010, so I'm assuming it's something related to Darwin.

Adding Tim and Jim as reviewers.

[t.p.northover]

Jim's change seems to have been a whitespace tidy-up. It actually seems to hail from the very first tailcall commit in r105413 so it was probably just paranoia.

I can't think of any reason why it should be banned either so I say go for it.

[olista01]

The change itself looks fine, I have a few suggestions for testing:

The test is checking the APCS ABI, could you add some extra RUN lines to check the newer AAPCS ABI? In particular, v_caller_ints2 will not be tail-callable in AAPCS because i64s must be in an even-odd register pair, so the last argument will go on the stack.

I'd also suggest adding some tests using doubles, and using the arm_aapcs_vfpcc calling convention, as doubles get treated differently for variadic and non-variadic calls.

[rengolin]

In D26748#597420, @t.p.northover wrote:

Jim's change seems to have been a whitespace tidy-up.

Oh, apologies for the lack of thoroughness on my research. :)

[pbarrio]

More testing: add tests for AAPCS and for floating-point arguments to variadic/non-variadic functions.

[rengolin]

Small comment on the tests, otherwise, LGTM. Thanks!

test/CodeGen/ARM/tail-call.ll

43 ↗

(On Diff #78243)

You can make all of them use --check-prefix=CHECK and then have only one CHECK-LABEL.

[pbarrio]

Cleaned up tests as per Renato's comments.

[pbarrio]

Thanks all for the reviews!

[rengolin]

LGTM, thanks!

test/CodeGen/ARM/tail-call.ll

30 ↗

(On Diff #78339)

Heh, this was being completely ignored before. :)

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

ARMTargetLowering::IsEligibleForTailCallOptimization(SDValue Callee,
   const Function *CallerF = MF.getFunction();
   CallingConv::ID CallerCC = CallerF->getCallingConv();
 
   assert(Subtarget->supportsTailCall());
 
   // Look for obvious safe cases to perform tail call optimization that do not
   // require ABI changes. This is what gcc calls sibcall.
 
-  // Do not sibcall optimize vararg calls unless the call site is not passing
-  // any arguments.
-  if (isVarArg && !Outs.empty())
-    return false;
-
   // 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;
 
   // Also avoid sibcall optimization if either caller or callee uses struct
   // return semantics.

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

+; RUN: llc -mtriple armv7 -target-abi aapcs -float-abi soft -O0 -o - < %s \
+; RUN:   | FileCheck %s -check-prefix CHECK-SOFT -check-prefix CHECK
+; RUN: llc -mtriple armv7 -target-abi aapcs -float-abi hard -O0 -o - < %s \
+; RUN:   | FileCheck %s -check-prefix CHECK-HARD -check-prefix CHECK
+
+; Tests for passing floating-point regs. Variadic functions will always use
+; general-purpose registers. Standard functions will use the floating-point
+; registers if there is hardware FP available.
+
+declare i1 @non_variadic(float, float, float, float)
+declare i1 @non_variadic_big(float, float, float, float, float, float)
+declare i1 @variadic(float, ...)
+
+define void @non_variadic_fp(float %x, float %y) {
+; CHECK-LABEL: non_variadic_fp:
+; CHECK: b non_variadic
+entry:
+  %call = tail call i1 (float, float, float, float) @non_variadic(float %y, float %x, float %x, float %y)
+  ret void
+}
+
+define void @variadic_fp(float %x, float %y) {
+; CHECK-LABEL: variadic_fp:
+; CHECK: b variadic
+entry:
+  %call = tail call i1 (float, ...) @variadic(float %y, float %x, float %x, float %y)
+  ret void
+}
+
+; With soft-float, general-purpose registers are used and there are not enough
+; of them to handle the 6 arguments. With hard-float, we have plenty of regs
+; (s0-s15) to pass FP arguments.
+define void @non_variadic_fp_big(float %x, float %y) {
+; CHECK-LABEL: non_variadic_fp_big:
+; CHECK-SOFT: bl non_variadic_big
+; CHECK-HARD: b non_variadic_big
+entry:
+  %call = tail call i1 (float, float, float, float, float, float) @non_variadic_big(float %y, float %x, float %x, float %y, float %x, float %y)
+  ret void
+}
+
+; Variadic functions cannot use FP regs to pass arguments; only GP regs.
+define void @variadic_fp_big(float %x, float %y) {
+; CHECK-LABEL: variadic_fp_big:
+; CHECK: bl variadic
+entry:
+  %call = tail call i1 (float, ...) @variadic(float %y, float %x, float %x, float %y, float %x, float %y)
+  ret void
+}

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

 ; RUN: llc -mtriple armv7 -target-abi apcs -O0 -o - < %s \
-; RUN:   | FileCheck %s -check-prefix CHECK-TAIL
+; RUN:   | FileCheck %s -check-prefix CHECK-TAIL -check-prefix CHECK
 ; RUN: llc -mtriple armv7 -target-abi apcs -O0 -disable-tail-calls -o - < %s \
-; RUN:   | FileCheck %s -check-prefix CHECK-NO-TAIL
+; RUN:   | FileCheck %s -check-prefix CHECK-NO-TAIL -check-prefix CHECK
+; RUN: llc -mtriple armv7 -target-abi aapcs -O0 -o - < %s \
+; RUN:   | FileCheck %s -check-prefix CHECK-TAIL-AAPCS -check-prefix CHECK
 
 declare i32 @callee(i32 %i)
 declare extern_weak fastcc void @callee_weak()
 
 define i32 @caller(i32 %i) {
 entry:
   %r = tail call i32 @callee(i32 %i)
   ret i32 %r

 ; Weakly-referenced extern functions cannot be tail-called, as AAELF does
 ; not define the behaviour of branch instructions to undefined weak symbols.
 define fastcc void @caller_weak() {
 ; CHECK-LABEL: caller_weak:
 ; CHECK: bl callee_weak
   tail call void @callee_weak()
   ret void
 }
+
+; A tail call can be optimized if all the arguments can be passed in registers
+; R0-R3, or the remaining arguments are already in the caller's parameter area
+; in the stack. Variadic functions are no different.
+declare i32 @variadic(i32, ...)
+
+; e.g. four integers
+define void @v_caller_ints1(i32 %a, i32 %b) {
+; CHECK-LABEL: v_caller_ints1:
+; CHECK-TAIL: b variadic
+; CHECK-TAIL-AAPCS: b variadic
+; CHECK-NO-TAIL: bl variadic
+entry:
+  %call = tail call i32 (i32, ...) @variadic(i32 %a, i32 %b, i32 %b, i32 %a)
+  ret void
+}
+
+; e.g. two 32-bit integers, one 64-bit integer (needs to span two regs)
+define void @v_caller_ints2(i32 %y, i64 %z) {
+; CHECK-LABEL: v_caller_ints2:
+; CHECK-TAIL: b variadic
+; CHECK-TAIL-AAPCS: b variadic
+; CHECK-NO-TAIL: bl variadic
+entry:
+  %call = tail call i32 (i32, ...) @variadic(i32 %y, i32 %y, i64 %z)
+  ret void
+}
+
+; e.g. two 32-bit integers, one 64-bit integer (needs to span two regs). Notice
+; that %z is passed in r1-r2 if APCS is used, contrary to AAPCS where r2-r3
+; would be used (since double-word types must start at an even register). In the
+; latter case, the third argument needs to be passed through the stack.
+define void @v_caller_ints3(i32 %y, i64 %z) {
+; CHECK-LABEL: v_caller_ints3:
+; CHECK-TAIL: b variadic
+; CHECK-TAIL-AAPCS: bl variadic
+; CHECK-NO-TAIL: bl variadic
+entry:
+  %call = tail call i32 (i32, ...) @variadic(i32 %y, i64 %z, i32 %y)
+  ret void
+}
+
+; e.g. two 32-bit integers, one 64-bit integer and another 64-bit integer that
+; doesn't fit in r0-r3 but comes from the caller argument list and is in the
+; same position.
+define void @v_caller_ints4(i64 %a, i32 %b, i32 %c, i64 %d) {
+; CHECK-LABEL: v_caller_ints4:
+; CHECK-TAIL: b variadic
+; CHECK-TAIL-AAPCS: b variadic
+; CHECK-NO-TAIL: bl variadic
+entry:
+  %call = tail call i32 (i32, ...) @variadic(i32 %b, i32 %c, i64 %a, i64 %d)
+  ret void
+}
+
+; If the arguments do not fit in r0-r3 and the existing parameters cannot be
+; taken from the caller's parameter region, the optimization is not supported.
+
+; e.g. one 32-bit integer, two 64-bit integers
+define void @v_caller_ints_fail(i32 %y, i64 %z) {
+; CHECK-LABEL: v_caller_ints_fail:
+; CHECK: bl variadic
+entry:
+  %call = tail call i32 (i32, ...) @variadic(i32 %y, i64 %z, i64 %z)
+  ret void
+}