X86ISelLowering: Support tail calls to/from thiscall functions

[hans]

Summary

This saves 16.5 KB on a Clang bootstrap. I'm hoping for more on Chromium, but haven't measured yet.

[hans]

(In a Chrome official build, this saves 124 KB over chrome.exe, chrome.dll and chrome_child.dll.)

[rnk]

Can we generalize this logic to all conventions that are isCalleePop? Then it will cover stdcall, fastcall, and vectorcall.

[hans]

Thanks for landing r251137!

Updated the patch to support the other calling conventions too.

[rnk]

lgtm

lib/Target/X86/X86ISelLowering.cpp

3712

This doesn't need an extra blank line

3779–3781

I think factoring out the negation will make this more readable:

bool CalleePopMatches = CalleeWillPop && BytesToPop == StackArgSize;
if (!CalleePopMatches)
  return false;

test/CodeGen/X86/tailcall-msvc-conventions.ll

146 ↗

(On Diff #38274)

s/aruments/arguments/

lib/Target/X86/X86ISelLowering.cpp

 bool X86TargetLowering::mayBeEmittedAsTailCall(CallInst *CI) const {
   auto Attr =
       CI->getParent()->getParent()->getFnAttribute("disable-tail-calls");
   if (!CI->isTailCall() || Attr.getValueAsString() == "true")
     return false;
 
   CallSite CS(CI);
   CallingConv::ID CalleeCC = CS.getCallingConv();
-  if (!IsTailCallConvention(CalleeCC) && !IsCCallConvention(CalleeCC))
+  if (!IsTailCallConvention(CalleeCC) && !IsCCallConvention(CalleeCC) &&
+      CalleeCC != CallingConv::X86_ThisCall)
     return false;
 
   return true;
 }
 
 /// Return true if the function is being made into
 /// a tailcall target by changing its ABI.
 static bool FuncIsMadeTailCallSafe(CallingConv::ID CC,

 /// Check whether the call is eligible for tail call optimization. Targets
 /// that want to do tail call optimization should implement this function.
 bool X86TargetLowering::IsEligibleForTailCallOptimization(
     SDValue Callee, CallingConv::ID CalleeCC, bool isVarArg,
     bool isCalleeStructRet, bool isCallerStructRet, Type *RetTy,
     const SmallVectorImpl<ISD::OutputArg> &Outs,
     const SmallVectorImpl<SDValue> &OutVals,
     const SmallVectorImpl<ISD::InputArg> &Ins, SelectionDAG &DAG) const {
-  if (!IsTailCallConvention(CalleeCC) && !IsCCallConvention(CalleeCC))
+  if (!IsTailCallConvention(CalleeCC) && !IsCCallConvention(CalleeCC) &&
+      CalleeCC != CallingConv::X86_ThisCall)
     return false;
 
   // If -tailcallopt is specified, make fastcc functions tail-callable.
   const MachineFunction &MF = DAG.getMachineFunction();
   const Function *CallerF = MF.getFunction();
 
   // If the function return type is x86_fp80 and the callee return type is not,
   // then the FP_EXTEND of the call result is not a nop. It's not safe to
bool X86TargetLowering::IsEligibleForTailCallOptimization(
 
   // Also avoid sibcall optimization if either caller or callee uses struct
   // return semantics.
   if (isCalleeStructRet || isCallerStructRet)
     return false;
 
   // An stdcall/thiscall caller is expected to clean up its arguments; the
   // callee isn't going to do that.
-  // FIXME: this is more restrictive than needed. We could produce a tailcall
-  // when the stack adjustment matches. For example, with a thiscall that takes
-  // only one argument.
   if (!CCMatch && (CallerCC == CallingConv::X86_StdCall ||
-                   CallerCC == CallingConv::X86_ThisCall))
+                   CallerCC == CallingConv::X86_ThisCall)) {
+    if (MF.getInfo<X86MachineFunctionInfo>()->getBytesToPopOnReturn())
       return false;
+  }
 
   // Do not sibcall optimize vararg calls unless all arguments are passed via
   // registers.
   if (isVarArg && !Outs.empty()) {
 
     // Optimizing for varargs on Win64 is unlikely to be safe without
     // additional testing.
     if (IsCalleeWin64 || IsCallerWin64)
for (unsigned i = 0, e = RVLocs1.size(); i != e; ++i) {
           return false;
       } else {
         if (RVLocs1[i].getLocMemOffset() != RVLocs2[i].getLocMemOffset())
           return false;
       }
     }
   }
 
+  unsigned StackArgsSize = 0;
+
   // If the callee takes no arguments then go on to check the results of the
   // call.
   if (!Outs.empty()) {
     // Check if stack adjustment is needed. For now, do not do this if any
     // argument is passed on the stack.
     SmallVector<CCValAssign, 16> ArgLocs;
     CCState CCInfo(CalleeCC, isVarArg, DAG.getMachineFunction(), ArgLocs,
                    *DAG.getContext());
 
+
     // Allocate shadow area for Win64
     if (IsCalleeWin64)
       CCInfo.AllocateStack(32, 8);
 
     CCInfo.AnalyzeCallOperands(Outs, CC_X86);
+    StackArgsSize = CCInfo.getNextStackOffset();
+
     if (CCInfo.getNextStackOffset()) {
       MachineFunction &MF = DAG.getMachineFunction();
-      if (MF.getInfo<X86MachineFunctionInfo>()->getBytesToPopOnReturn())
-        return false;
 
       // Check if the arguments are already laid out in the right way as
       // the caller's fixed stack objects.
       MachineFrameInfo *MFI = MF.getFrameInfo();
       const MachineRegisterInfo *MRI = &MF.getRegInfo();
       const X86InstrInfo *TII = Subtarget->getInstrInfo();
       for (unsigned i = 0, e = ArgLocs.size(); i != e; ++i) {
         CCValAssign &VA = ArgLocs[i];
if (!Subtarget->is64Bit() &&
           if (++NumInRegs == MaxInRegs)
             return false;
           break;
         }
       }
     }
   }
 
+  if (unsigned BytesToPop =
+          MF.getInfo<X86MachineFunctionInfo>()->getBytesToPopOnReturn()) {
+    // If we have bytes to pop, the callee must pop them for tail call to be
+    // possible.
+    assert(CCMatch && "CCs must be equal for a callee pop caller.");
+    if (BytesToPop != StackArgsSize)
+      return false;
+  } else {
+    // If we don't have bytes to pop, make sure the callee doesn't pop any.
+    if (X86::isCalleePop(CalleeCC, Subtarget->is64Bit(), isVarArg,
+                         MF.getTarget().Options.GuaranteedTailCallOpt) &&
+        StackArgsSize > 0)
+      return false;
+  }
+
   return true;
 }
 
 FastISel *
 X86TargetLowering::createFastISel(FunctionLoweringInfo &funcInfo,
                                   const TargetLibraryInfo *libInfo) const {
   return X86::createFastISel(funcInfo, libInfo);
 }

test/CodeGen/X86/tailcall-thiscall.ll

+; RUN: llc -mtriple=i686-unknown-linux -O1 %s -o - | FileCheck %s
+
+declare x86_thiscallcc void @no_args_method(i8*)
+declare x86_thiscallcc void @one_arg_method(i8*, i32)
+declare x86_thiscallcc void @two_args_method(i8*, i32, i32)
+declare void @ccall_func()
+declare void @ccall_func1(i32)
+
+
+
+define x86_thiscallcc void @test1(i8* %this) {
+entry:
+  tail call x86_thiscallcc void @no_args_method(i8* %this)
+  ret void
+
+; CHECK-LABEL: test1:
+; CHECK: jmp no_args_method
+}
+
+
+define x86_thiscallcc void @test2(i8* %this, i32 %a, i32 %b) {
+entry:
+  tail call x86_thiscallcc void @two_args_method(i8* %this, i32 %a, i32 %b)
+  ret void
+
+; @two_args_method will take care of popping %a and %b from the stack for us.
+;
+; CHECK-LABEL: test2:
+; CHECK: jmp two_args_method
+}
+
+
+define x86_thiscallcc void @test3(i8* %this, i32 %a, i32 %b, i32 %x) {
+entry:
+  tail call x86_thiscallcc void @two_args_method(i8* %this, i32 %a, i32 %b)
+  ret void
+
+; This would have been a tail call (%a and %b have the right positions in the
+; stack frame), but we must also pop %x from our stack, and @two_args_method
+; will not do that for us.
+;
+; CHECK-LABEL: test3:
+; CHECK: calll two_args_method
+; CHECK: retl $12
+}
+
+
+define x86_thiscallcc void @test4(i8* %this, i32 %a) {
+entry:
+  tail call x86_thiscallcc void @no_args_method(i8* %this)
+  ret void
+
+; There are no arguments in the call to match against our stack frame, but we
+; still can't tail call here because @no_args_method would not pop %x for us.
+;
+; CHECK-LABEL: test4:
+; CHECK: calll no_args_method
+; CHECK: retl $4
+}
+
+
+define void @test5(i8* %x) {
+entry:
+  tail call x86_thiscallcc void @no_args_method(i8* %x)
+  ret void
+
+; There is no restriction against tail calling from a ccall to thiscall
+; function in general.
+;
+; CHECK-LABEL: test5:
+; CHECK: jmp no_args_method
+}
+
+
+define void @test6(i8* %x, i32 %y) {
+entry:
+  tail call x86_thiscallcc void @one_arg_method(i8* %x, i32 %y);
+  ret void
+
+; No tail call: @test6 is ccall and @one_arg_method would pop %y off the stack.
+;
+; CHECK-LABEL: test6:
+; CHECK: calll one_arg_method
+}
+
+
+define x86_thiscallcc void @test7(i8* %this) {
+entry:
+  tail call void @ccall_func()
+  ret void
+
+; Tail call from thiscall to ccall works if no arguments need popping.
+;
+; CHECK-LABEL: test7:
+; CHECK: jmp ccall_func
+}
+
+
+define x86_thiscallcc void @test8(i8* %this, i32 %x) {
+entry:
+  tail call void @ccall_func1(i32 %x)
+  ret void
+
+; No tail call: %x needs to be popped by @test8.
+;
+; CHECK-LABEL: test8:
+; CHECK: calll ccall_func1
+; CHECK: retl $4
+}
+
+
+%S = type { i32 (...)** }
+define x86_thiscallcc void @test9(%S* %this, i32 %a) {
+entry:
+  %0 = bitcast %S* %this to void (%S*, i32)***
+  %vtable = load void (%S*, i32)**, void (%S*, i32)*** %0
+  %1 = load void (%S*, i32)*, void (%S*, i32)** %vtable
+  tail call x86_thiscallcc void %1(%S* %this, i32 %a)
+  ret void
+
+; Tail calling works through function pointers too.
+;
+; CHECK-LABEL: test9:
+; CHECK: movl (%ecx), %eax
+; CHECK: jmpl *(%eax)
+}