[llvm-commits] [llvm] r151123 - in /llvm/trunk: lib/Target/X86/X86CallingConv.td test/CodeGen/X86/thiscall-struct-return.ll

[Timur Iskhodzhanov]

2013/3/29 Aaron Ballman <aaron at aaronballman.com>:
> On Thu, Mar 28, 2013 at 9:45 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> 2012/2/21 Aaron Ballman <aaron at aaronballman.com>:
>>> Author: aaronballman
>>> Date: Tue Feb 21 21:04:40 2012
>>> New Revision: 151123
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=151123&view=rev
>>> Log:
>>> Adding support for Microsoft's thiscall calling convention.  LLVM side of the patch.
>>>
>>> Added:
>>>     llvm/trunk/test/CodeGen/X86/thiscall-struct-return.ll
>>> Modified:
>>>     llvm/trunk/lib/Target/X86/X86CallingConv.td
>>>
>>> Modified: llvm/trunk/lib/Target/X86/X86CallingConv.td
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86CallingConv.td?rev=151123&r1=151122&r2=151123&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/X86/X86CallingConv.td (original)
>>> +++ llvm/trunk/lib/Target/X86/X86CallingConv.td Tue Feb 21 21:04:40 2012
>>> @@ -331,8 +331,8 @@
>>>    // Promote i8/i16 arguments to i32.
>>>    CCIfType<[i8, i16], CCPromoteToType<i32>>,
>>>
>>> -  // The 'nest' parameter, if any, is passed in EAX.
>>> -  CCIfNest<CCAssignToReg<[EAX]>>,
>>> +  // Pass sret arguments indirectly through EAX
>>> +  CCIfSRet<CCAssignToReg<[EAX]>>,
>>>
>>>    // The first integer argument is passed in ECX
>>>    CCIfType<[i32], CCAssignToReg<[ECX]>>,
>>>
>>> Added: llvm/trunk/test/CodeGen/X86/thiscall-struct-return.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/thiscall-struct-return.ll?rev=151123&view=auto
>>> ==============================================================================
>>> --- llvm/trunk/test/CodeGen/X86/thiscall-struct-return.ll (added)
>>> +++ llvm/trunk/test/CodeGen/X86/thiscall-struct-return.ll Tue Feb 21 21:04:40 2012
>>> @@ -0,0 +1,47 @@
>> Hi Aaron,
>>
>> I was working on sret vs thiscall and I thought I've fixed it locally
>> but then noticed this test (which I find wrong, see below) still
>> passes for me.
>>
>>> +; RUN: llc < %s -mtriple=i386-PC-Win32 | FileCheck %s
>> I'm afraid triples are case-sensitive :(
>> As least I get different |md5sum when running my local build with
>> -mtriple=i386-PC-Win32 and -mtriple=i386-pc-win32.
>
> Good catch!  I never noticed that!
>
>>
>>> +%class.C = type { i8 }
>>> +%struct.S = type { i32 }
>>> +%struct.M = type { i32, i32 }
>>> +
>>> +declare void @_ZN1CC1Ev(%class.C* %this) unnamed_addr nounwind align 2
>>> +declare x86_thiscallcc void @_ZNK1C5SmallEv(%struct.S* noalias sret %agg.result, %class.C* %this) nounwind align 2
>>> +declare x86_thiscallcc void @_ZNK1C6MediumEv(%struct.M* noalias sret %agg.result, %class.C* %this) nounwind align 2
>>> +
>>> +define void @testv() nounwind {
>>> +; CHECK: testv:
>>> +; CHECK: leal
>>> +; CHECK-NEXT: movl     %esi, (%esp)
>>> +; CHECK-NEXT: calll _ZN1CC1Ev
>>> +; CHECK: leal 8(%esp), %eax
>>> +; CHECK-NEXT: movl %esi, %ecx
>> I believe this is wrong, the address of the structure should be passed
>> via stack.
>
> I seem to recall observing it being passed "both ways" in that it was
> allocated on the stack, but that the function would use EAX to
> determine where on the stack to find it.
I don't think so.

The first thing that happens inside any thiscall/sret methods I saw at -O2 is
  mov eax, ***[esp]
i.e. the eax is ignored.
A similar thing happens after the frame pointer prolog at -O0.
I never observed eax being read by a thiscall/sret method.

What often happens in the caller of a thiscall is
  lea eax, ***[esp/ebp]
  push eax
  lea ecx, ***[esp/ebp]
  call ...
i.e. eax is used just as a temporary register to push a pointer onto stack.
This could as well be ecx though.

>>> +; CHECK-NEXT: calll _ZNK1C5SmallEv
>>> +entry:
>>> +  %c = alloca %class.C, align 1
>>> +  %tmp = alloca %struct.S, align 4
>>> +  call void @_ZN1CC1Ev(%class.C* %c)
>>> +  ; This call should put the return structure as a pointer
>>> +  ; into EAX instead of returning directly in EAX.
>> AFAICT, the value of EAX value after the call should match the top of
>> the stack before the call.
>
> It's been a while since I've looked into this, but I recall thiscall
> being an odd-man-out with how it handled structure returns in that it
> would put the pointer to the structure into EAX instead of the single
> four-byte value.
That depends on whether the return type is a POD or not.

> So yes, it makes sense that it should match the top
> of the stack before the call (accounting for the allocated space).
>
> ~Aaron