[PATCH] D22315: Teach fast isel about thiscall (and callee-pop) calls.

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 13 16:18:05 PDT 2016


thakis added a comment.

Thanks for the review!


================
Comment at: lib/Target/X86/X86FastISel.cpp:3028
@@ -3027,2 +3027,3 @@
   case CallingConv::X86_FastCall:
+  case CallingConv::X86_ThisCall:
   case CallingConv::X86_64_Win64:
----------------
rnk wrote:
> Maybe toss stdcall on here, since that's easier than thiscall.
I wanted to do that in a separate change (here and in X86SelectRet()) since it feels like a separate thing.

================
Comment at: lib/Target/X86/X86FastISel.cpp:3337
@@ +3336,3 @@
+                       TM.Options.GuaranteedTailCallOpt)
+          ? NumBytes // Callee pops everything.
+          : computeBytesPoppedByCallee(Subtarget, CC, CLI.CS);
----------------
rnk wrote:
> 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.
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 at 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.


http://reviews.llvm.org/D22315





More information about the llvm-commits mailing list