Support for sret the msvc++ way.

[oscarfv]

Bugzilla Link	5058
Resolution	WORKSFORME
Resolved on	Jun 18, 2012 20:00
Version	trunk
OS	Windows XP
Blocks	llvm/llvm-bugzilla-archive#12477
CC	@AaronBallman,@asl,@tritao

Extended Description

Contrary to gcc, on msvc++ is the caller the responsible of freeing the stack space used by the sret pointer parameter.

Right now if LLVM code calls a function compiled with msvc++ that returns a struct, the application will crash due to the unbalanced stack (if the signature of the external function contains sret. If no sret is used and the address of the return struct is passed as a regular parameter, the call works).

There are other differences on sret handling among gcc and msvc++. Those are listed on page 20 of

http://www.agner.org/optimize/calling_conventions.pdf

[AaronBallman]

I believe this issue is resolved on ToT. From what I can tell, the various calling conventions have all implemented matching behavior to MSVC.

Please test again against the trunk, and if you find an example where this is not the case, please reopen.

[oscarfv]

Sorry, but you close the bug with WORKSFORME because you believe that it is fixed? Without checking the fact at all? That's not what WORKSFORME means.

Reopening. For closing this bug please indicate the commit numbers on svn or test cases in the test suite that supports your action. Thanks.

[AaronBallman]

Sorry, I wasn't clear. I believe it is fixed because all of the test cases (thiscall-struct-return.cpp, thiscall-struct-return.ll) claim it is so, and my own testing shows it to be so. I said "believe" simply because I've learned that my testing != other's testing and so I'm more cautious about assertions.

r151122, r151123 are both sret related (with thiscall) and AFAIK were the last outstanding issue.

Feel free to test for yourself to verify, however. I verified: 4, 8, 16, 3, 7, 15 byte structures with packing sizes 1, 2, 4, 8, and 16 with cdecl, stdcall, fastcall and thiscall.

[oscarfv]

Thanks for the detailed reply.

But the feature request is not restricted to thiscall. The default C calling convention on MSVC++ also leaves to the caller the responsibility of removing the stack space allocated for passing the address of the returning struct.

I haven't checked if that is also fixed but, contrary to X86_ThisCall, I don't see any mention on the comments of CallingConv.h about special casing the C calling convention on Windows.

Anyways this would be easily fixed, as my initial report hints (just ignore the sret attribute on Windows). The hard part is on what is mentioned on the referenced pdf document about non-POD C++ objects: when passing temporary values, g++ pushes pointers to those temporaries, while MSVC++ "pushes" by constructing them on the right place of stack. That's the hard part because it requires to invoke constructors, copiers and destructors. This is discussed on http://llvm.org/bugs/show_bug.cgi?id=5064

IMO, if any of the above points are still unresolved this bug should remain open.

[AaronBallman]

Thanks for the detailed reply.

But the feature request is not restricted to thiscall. The default C calling
convention on MSVC++ also leaves to the caller the responsibility of removing
the stack space allocated for passing the address of the returning struct.

Not always -- it depends on the size of the structure. For instance, 4 byte structures are returned via EAX, 8 byte via EAX/EDX, but odd byte + packed structures do the hidden parameter trick. MS doesn't document the complete ABI, unfortunately.

I haven't checked if that is also fixed but, contrary to X86_ThisCall, I don't
see any mention on the comments of CallingConv.h about special casing the C
calling convention on Windows.

You are correct, the comments don't make any mention of special cases for Windows. It'd be reasonable to modify them.

Anyways this would be easily fixed, as my initial report hints (just ignore the
sret attribute on Windows). The hard part is on what is mentioned on the
referenced pdf document about non-POD C++ objects: when passing temporary
values, g++ pushes pointers to those temporaries, while MSVC++ "pushes" by
constructing them on the right place of stack. That's the hard part because it
requires to invoke constructors, copiers and destructors. This is discussed on
http://llvm.org/bugs/show_bug.cgi?id=5064

IMO, if any of the above points are still unresolved this bug should remain
open.

I think it is better to create a new report with specific test cases to resolve, if there are any. For instance, non-POD return types with constructors is quite specific and without test cases would be a lot of guesswork.