Teach fast isel about thiscall (and callee-pop) calls.

[thakis]

Summary

Testing for this is a bit ugly: -fast-isel-abort=1 doesn't abort on calls, and -fast-isel-abort=2 aborts both on calls and on parameters (which fail to be lowered), so it can't be used. Instead, look at the output of -fast-isel-verbose.

Depends on http://reviews.llvm.org/D22314, else test/CodeGen/X86/win32_sret.ll fails due to (harmless) changes in the generated code.

[rnk]

lib/Target/X86/X86FastISel.cpp

3028

Maybe toss stdcall on here, since that's easier than thiscall.

3117

"getAlignedCallFrameSize" makes me suspicious

3337

I don't think this is true if stack realignment is active. NumBytes is the argument size rounded up to stack alignment. Make a test case with alloca align 32 to see if there's a bug.

[thakis]

Thanks for the review!

lib/Target/X86/X86FastISel.cpp

3028

I wanted to do that in a separate change (here and in X86SelectRet()) since it feels like a separate thing.

3337

It seems to work (if I understood your example request correctly – this is with a locally-hacked up version that supports stdcall in fast isel as well):

Nicos-MBP:llvm-build thakis$ cat test.ll
source_filename = "fastisel.cc"
target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
target triple = "i386-pc-windows-msvc18.0.0"

define void @myfun() #0 {
entry:
  %i = alloca i32, align 32
  store i32 43, i32* %i, align 4
  %0 = load i32, i32* %i, align 4
  call x86_stdcallcc void @stdcallfun(i32 %0)
  ret void
}

declare x86_stdcallcc void @stdcallfun(i32) #1
Nicos-MBP:llvm-build thakis$ bin/llc -no-x86-call-frame-opt  -O0 -fast-isel-verbose < test.ll
...
_myfun:                                 # @myfun
# BB#0:                                 # %entry
	pushl	%ebp
	movl	%esp, %ebp
	andl	$-32, %esp
	subl	$64, %esp
	movl	$43, 32(%esp)
	movl	32(%esp), %eax
	movl	%eax, (%esp)
	calll	_stdcallfun@4
	movl	%ebp, %esp
	popl	%ebp
	retl

This matches what X86ISelLowering.cpp does, and I built a few test binaries with this that worked too. Intuitively, I think it works because the call frame is aligned as much as the parameters are, not as much as the arguments are – and the callee-pop pops the parameter size.

[rnk]

lib/Target/X86/X86FastISel.cpp

3337

I see, CCInfo.getAlignedCallFrameSize() doesn't round up to stack alignment, it rounds up to the alignment of the parameters. I was concerned about what when the outgoing stack alignment is 16 instead of 4, and we need to emit cleanup adjustments. This probably only happens on Linux, so this is something like the test case I was thinking about:

$ cat t.ll
target triple = "i686-linux"
declare x86_stdcallcc void @stdcallfun(i32*)
define void @myfun(i32 %n) {
entry:
  %i = alloca i32
  call x86_stdcallcc void @stdcallfun(i32* %i)
  %j = alloca i32, i32 %n
  call x86_stdcallcc void @stdcallfun(i32* %j)
  store volatile i32 0, i32* %i
  ret void
}

$ llc t.ll -o - -mtriple=i686-linux -no-x86-call-frame-opt
...
        subl    $16, %esp
        leal    -8(%ebp), %eax
        movl    %eax, (%esp)
        calll   stdcallfun
        addl    $12, %esp

        movl    %esp, %eax
        leal    15(,%esi,4), %ecx
        andl    $-16, %ecx
        subl    %ecx, %eax
        movl    %eax, %esp

        subl    $16, %esp  # ADJCALLSTACKDOWN uses 16 bytes
        movl    %eax, (%esp)
        calll   stdcallfun
        addl    $12, %esp    # The callee only pops 4 bytes, not 16, so we need to add 12 to esp
...

3338

This computeBytesPoppedByCallee function only handles popping hidden sret parameters, which has to be one of the weirdest 32-bit ABI corner cases ever. Maybe rename it computeBytesPoppedByCalleeForSRet?

test/CodeGen/X86/fast-isel-call.ll

59

We should check for correct code

64

If you add a store to an alloca after the call, it will ensure that the call sequence end stack adjustment isn't eliminated as dead code.

66

I would suggest testing with dynamic allocas, but fastisel can't handle them. =/

[thakis]

address comments

test/CodeGen/X86/fast-isel-call.ll

59

Done. (e.g. win32_sret.ll already checked for that, but having more testing doesn't hurt of course.)

64

Does that happen in fast isel mode? As far as I can tell, it doesn't.

[rnk]

lgtm

test/CodeGen/X86/fast-isel-call.ll

64

Nevermind, this comment isn't relevant to the actual code sequence you're checking.

[thakis]

r275360, thanks!

lib/Target/X86/X86FastISel.cpp

for (auto const &Arg : F->args()) {
     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
             TII.get(TargetOpcode::COPY), ResultReg)
       .addReg(DstReg, getKillRegState(true));
     updateValueMap(&Arg, ResultReg);
   }
   return true;
 }
 
-static unsigned computeBytesPoppedByCallee(const X86Subtarget *Subtarget,
+static unsigned computeBytesPoppedByCalleeForSRet(const X86Subtarget *Subtarget,
                                                   CallingConv::ID CC,
                                                   ImmutableCallSite *CS) {
   if (Subtarget->is64Bit())
     return 0;
   if (Subtarget->getTargetTriple().isOSMSVCRT())
     return 0;
   if (CC == CallingConv::Fast || CC == CallingConv::GHC ||
       CC == CallingConv::HiPE)
     return 0;
 
bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
   // Handle only C, fastcc, and webkit_js calling conventions for now.
   switch (CC) {
   default: return false;
   case CallingConv::C:
   case CallingConv::Fast:
   case CallingConv::WebKit_JS:
   case CallingConv::Swift:
   case CallingConv::X86_FastCall:
+  case CallingConv::X86_ThisCall:
   case CallingConv::X86_64_Win64:
   case CallingConv::X86_64_SysV:
     break;
   }
 
   // Allow SelectionDAG isel to handle tail calls.
   if (IsTailCall)
     return false;
bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
   // Don't know about inalloca yet.
   if (CLI.CS && CLI.CS->hasInAllocaArgument())
     return false;
 
   for (auto Flag : CLI.OutFlags)
     if (Flag.isSwiftError())
       return false;
 
-  // Fast-isel doesn't know about callee-pop yet.
-  if (X86::isCalleePop(CC, Subtarget->is64Bit(), IsVarArg,
-                       TM.Options.GuaranteedTailCallOpt))
-    return false;
-
   SmallVector<MVT, 16> OutVTs;
   SmallVector<unsigned, 16> ArgRegs;
 
   // If this is a constant i1/i8/i16 argument, promote to i32 to avoid an extra
   // instruction. This is safe because it is common to all FastISel supported
   // calling conventions on x86.
   for (int i = 0, e = OutVals.size(); i != e; ++i) {
     Value *&Val = OutVals[i];
bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
 
   // Allocate shadow area for Win64
   if (IsWin64)
     CCInfo.AllocateStack(32, 8);
 
   CCInfo.AnalyzeCallOperands(OutVTs, OutFlags, CC_X86);
 
   // Get a count of how many bytes are to be pushed on the stack.
   unsigned NumBytes = CCInfo.getAlignedCallFrameSize();
 
   // Issue CALLSEQ_START
   unsigned AdjStackDown = TII.getCallFrameSetupOpcode();
   BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(AdjStackDown))
     .addImm(NumBytes).addImm(0);
 
   // Walk the register/memloc assignments, inserting copies/loads.
   const X86RegisterInfo *RegInfo = Subtarget->getRegisterInfo();
if (Is64Bit && IsVarArg && !IsWin64)
     MIB.addReg(X86::AL, RegState::Implicit);
 
   // Add implicit physical register uses to the call.
   for (auto Reg : OutRegs)
     MIB.addReg(Reg, RegState::Implicit);
 
   // Issue CALLSEQ_END
   unsigned NumBytesForCalleeToPop =
-    computeBytesPoppedByCallee(Subtarget, CC, CLI.CS);
+      X86::isCalleePop(CC, Subtarget->is64Bit(), IsVarArg,
+                       TM.Options.GuaranteedTailCallOpt)
+          ? NumBytes // Callee pops everything.
+          : computeBytesPoppedByCalleeForSRet(Subtarget, CC, CLI.CS);
   unsigned AdjStackUp = TII.getCallFrameDestroyOpcode();
   BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(AdjStackUp))
     .addImm(NumBytes).addImm(NumBytesForCalleeToPop);
 
   // Now handle call return values.
   SmallVector<CCValAssign, 16> RVLocs;
   CCState CCRetInfo(CC, IsVarArg, *FuncInfo.MF, RVLocs,
                     CLI.RetTy->getContext());

test/CodeGen/X86/fast-isel-call.ll

-; RUN: llc < %s -O0 -fast-isel-abort=1 -march=x86 -mtriple=i686-apple-darwin8 | FileCheck %s
+; RUN: llc < %s -O0 -fast-isel-abort=1 -march=x86 -mtriple=i686-apple-darwin8 2>/dev/null | FileCheck %s
+; RUN: llc < %s -O0 -fast-isel-abort=1 -march=x86 -mtriple=i686-apple-darwin8 2>&1 >/dev/null | FileCheck -check-prefix=STDERR -allow-empty %s
 
 %struct.s = type {i32, i32, i32}
 
 define i32 @test1() nounwind {
 tak:
   %tmp = call i1 @foo()
   br i1 %tmp, label %BB1, label %BB2
 BB1:
define void @test4(i8* %a, i8* %b) {
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* %a, i8* %b, i32 100, i32 1, i1 false)
   ret void
 ; CHECK-LABEL: test4:
 ; CHECK:   movl {{.*}}, (%esp)
 ; CHECK:   movl {{.*}}, 4(%esp)
 ; CHECK:   movl $100, 8(%esp)
 ; CHECK:   calll {{.*}}memcpy
 }
+
+; STDERR-NOT: FastISel missed call:   call x86_thiscallcc void @thiscallfun
+%struct.S = type { i8 }
+define void @test5() #0 {
+entry:
+  %s = alloca %struct.S, align 1
+; CHECK-LABEL: test5:
+; CHECK: subl $12, %esp
+; CHECK: leal 8(%esp), %ecx
+; CHECK: movl $43, (%esp)
+; CHECK: calll {{.*}}thiscallfun
+; CHECK: addl $8, %esp
+  call x86_thiscallcc void @thiscallfun(%struct.S* %s, i32 43)
+  ret void
+}
+declare x86_thiscallcc void @thiscallfun(%struct.S*, i32) #1