Hello Aaron, Anton, LLVM-dev,
While working on http://llvm.org/PR13676#c6
I found out that whenever I compile code with class methods returning
structures it get generated incompatible with MSVC.
Looking at lib/Target/X86/X86CallingConv.td, I found out that
CC_X86_32_ThisCall maps SRet to EAX but in fact it should push
the address of the return temp on stack.
The following patch fixes the issue on Windows:
---------------------------------
Index: lib/Target/X86/X86CallingConv.td
==================================================================---
lib/Target/X86/X86CallingConv.td (revision 164763)
+++ lib/Target/X86/X86CallingConv.td (working copy)
@@ -335,7 +335,8 @@
CCIfType<[i8, i16], CCPromoteToType<i32>>,
// Pass sret arguments indirectly through EAX
- CCIfSRet<CCAssignToReg<[EAX]>>,
+ CCIfSRet<CCAssignToStack<4, 4>>,
// The first integer argument is passed in ECX
CCIfType<[i32], CCAssignToReg<[ECX]>>,
---------------------------------
[hope this doesn't get wrapped in your email client]
Unfortunately, this patch also changes how SRet/ThisCall behaves
non-Windows systems too (right?).
I'd like to ask for advice:
a) Is it OK to change the SRet/ThisCall behaviour on non-Windows platforms?
[I suppose no]
b) Should I be altering CC_X86_32_ThisCall
OR should I introduce CC_X86_Win32_ThisCall instead?
[Answer not clear to me - is there any platform besides Windows
that uses thiscall?]
Probably I need to create CC_X86_Win32_ThisCall (and maybe
CC_X86_Win32_C later) by copying CC_X86_32_ThisCall similar to how
CC_X86_Win64_C is done - does that sound right?
Hints and suggestions on fixing this are welcome!
--
Thanks,
Timur Iskhodzhanov,
Google Russia
Timur Iskhodzhanov <timurrrr at google.com> writes:> Hello Aaron, Anton, LLVM-dev, > > While working on http://llvm.org/PR13676#c6 > I found out that whenever I compile code with class methods returning > structures it get generated incompatible with MSVC.http://llvm.org/bugs/show_bug.cgi?id=5058 [snip]> Hints and suggestions on fixing this are welcome!LLVM supports the g++ C++ ABI. Your change breaks that. Please note that this is not a Windows issue, it is a MSVC++ issue, as g++ works fine on Windows too. There are other issues with paramenter passing and the MSVC++ ABI, see for instance http://llvm.org/bugs/show_bug.cgi?id=5064 IIRC LLVM needs susbstantial modifications for supporting function calls compatible with MSVC++, or the frontend that uses LLVM has to do quite a bit of hacking for working around LLVM's limitations.
Hello Timur,> I'd like to ask for advice: > a) Is it OK to change the SRet/ThisCall behaviour on non-Windows platforms? > [I suppose no]no> > b) Should I be altering CC_X86_32_ThisCall > OR should I introduce CC_X86_Win32_ThisCall instead? > [Answer not clear to me - is there any platform besides Windows > that uses thiscall?]no It seems for me that you're trying to solve the problem from the wrong end. As far as I remember, there is a difference - "simple" (probable POD-like stuff) are returned in the regs, while classes with non-trivial ctors, etc. are passed / returned on stack. It's frontend responsibility to emit proper IR in this case. See http://llvm.org/bugs/show_bug.cgi?id=5064 and around. This seems to be the correct description of what's going on. -- With best regards, Anton Korobeynikov Faculty of Mathematics and Mechanics, Saint Petersburg State University
Something to keep in mind...when I was researching it, thiscall was a bit odd (at least to me) in MSVC. It pushes the structure onto the stack from the caller, but it returns the pointer to the structure in EAX and cleans up the pushed stack. The caller then either reloads the structure from its local stack, or it uses EAX to access it. The latter is especially true if it involves a temp. ~Aaron On Tue, Oct 2, 2012 at 10:34 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:> Hello Aaron, Anton, LLVM-dev, > > While working on http://llvm.org/PR13676#c6 > I found out that whenever I compile code with class methods returning > structures it get generated incompatible with MSVC. > > Looking at lib/Target/X86/X86CallingConv.td, I found out that > CC_X86_32_ThisCall maps SRet to EAX but in fact it should push > the address of the return temp on stack. > > The following patch fixes the issue on Windows: > --------------------------------- > Index: lib/Target/X86/X86CallingConv.td > ==================================================================> --- lib/Target/X86/X86CallingConv.td (revision 164763) > +++ lib/Target/X86/X86CallingConv.td (working copy) > @@ -335,7 +335,8 @@ > CCIfType<[i8, i16], CCPromoteToType<i32>>, > > // Pass sret arguments indirectly through EAX > - CCIfSRet<CCAssignToReg<[EAX]>>, > + CCIfSRet<CCAssignToStack<4, 4>>, > > // The first integer argument is passed in ECX > CCIfType<[i32], CCAssignToReg<[ECX]>>, > > --------------------------------- > [hope this doesn't get wrapped in your email client] > > Unfortunately, this patch also changes how SRet/ThisCall behaves > non-Windows systems too (right?). > > I'd like to ask for advice: > a) Is it OK to change the SRet/ThisCall behaviour on non-Windows platforms? > [I suppose no] > > b) Should I be altering CC_X86_32_ThisCall > OR should I introduce CC_X86_Win32_ThisCall instead? > [Answer not clear to me - is there any platform besides Windows > that uses thiscall?] > > Probably I need to create CC_X86_Win32_ThisCall (and maybe > CC_X86_Win32_C later) by copying CC_X86_32_ThisCall similar to how > CC_X86_Win64_C is done - does that sound right? > > Hints and suggestions on fixing this are welcome! > > -- > Thanks, > Timur Iskhodzhanov, > Google Russia
On Tue, Oct 2, 2012 at 7:03 PM, Anton Korobeynikov <asl at math.spbu.ru> wrote:> Hello Timur, > >> I'd like to ask for advice: >> a) Is it OK to change the SRet/ThisCall behaviour on non-Windows platforms? >> [I suppose no] > no > >> >> b) Should I be altering CC_X86_32_ThisCall >> OR should I introduce CC_X86_Win32_ThisCall instead? >> [Answer not clear to me - is there any platform besides Windows >> that uses thiscall?] > noCan you please clarify which question you've answered here? Sorry for making the ambiguous questions in the first place :)> It seems for me that you're trying to solve the problem from the wrong > end. As far as I remember, there is a difference - "simple" (probable > POD-like stuff) are returned in the regs, while classes with > non-trivial ctors, etc. are passed / returned on stack.Sort of.> It's frontend responsibility to emit proper IR in this case.Isn't it what's SRet is supposed to be?> See http://llvm.org/bugs/show_bug.cgi?id=5064 and around. This seems > to be the correct description of what's going on.FTR, http://llvm.org/bugs/show_bug.cgi?id=5058 seems to be more up-to-date. Thanks for your reply!