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

[Aaron Ballman]

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.

>> +; 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.  So yes, it makes sense that it should match the top
of the stack before the call (accounting for the allocated space).

~Aaron