[ms-cxxabi] Set proper SRet flags for most functions; also handle empty struct arguments correctly

[Timur Iskhodzhanov]

2013/4/12 John McCall <rjmccall at apple.com>:
> On Apr 12, 2013, at 8:44 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> 2013/4/9 John McCall <rjmccall at apple.com>
>>>
>>>> If that looks OK - I assume I should do the same for other ABIInfos ?
>>>> (I hope they are covered with tests well...)
>> See the attached patch where I've applied the same approach to other ABIs.
>>
>>>>> /// How should we pass a particular record type as an argument?
>>>>> enum RecordArgABI {
>>>>>   /// Pass it using the normal C aggregate rules for the ABI, potentially
>>>>>   /// introducing extra copies and passing some or all of it in registers.
>>>>>   RAA_DirectInRegisters,
>>>>>
>>>>>   /// Pass it on the stack using its defined layout.  The argument must be
>>>>>   /// evaluated directly into the correct stack position in the arguments area,
>>>>>   /// and the call machinery must not move it or introduce extra copies.
>>>>>   RAA_DirectInMemory,
>>>>>
>>>>>   /// Pass it as a pointer to temporary memory.
>>>>>   RAA_Indirect
>>>>> };
>>>> I'm not sure I can effectively use the first two cases right now and
>>>> there should also be RAA_IndirectByval (see what I do for Microsoft
>>>> ABI), so I've created a slightly simpler enum.
>>>
>>> Despite how it looks in IR, "byval" is direct in memory.
>>>
>>> Just to be clear, the Itanium ABI would use RAA_Indirect if the copy constructor
>>> or destructor is non-trivial and RAA_DirectInRegisters otherwise,
>>
>> OK, after re-reading the RAA_DirectInRegisters comment a few times I
>> now understand it's exactly the same as I proposed for RAA_Default.
>> Not sure why I had not seen that before...
>
> RAA_Default is fine for the name, actually.  Go with that.
OK, done

>> I now use the same enum you've suggested, but I also explicitly set
>> the RAA_DirectInRegisters's value to 0
>> so now I can write like this:
>>
>>  if (RecordArgABI RAA = getRecordArgABI(...))
>>    ABIArgInfo::getIndirect(0, RAA == CGCXXABI::RAA_DirectInMemory);
>>  // continue if RAA == RAA_DirectInRegisters.
>>
>> Is it OK to do so?
>>
>>> and the MS ABI would use RAA_DirectInMemory if there's any non-trivial
>>> constructor or destructor and RAA_DirectInRegisters otherwise.
>> I think you've meant just "MS ABI would use RAA_DirectInMemory.",
>> otherwise my tests fail.
>
> Are you saying that the MS ABI should just *always* use RAA_DirectInMemory?
Yes, at least I haven't seen counter-examples so far, though I have
like 20 different tests for passing structs as arguments that actually
run the code and verify it works.
(plus 30 more for returning structs)

> Because I don't think that's true.  Microsoft's calling conventions are
> definitely big on passing things on the stack, but some of them do use
> registers, and it's at least possible that they'll promote sufficiently small
> structs to registers as well.  At any rate, it should be up to the normal CC
> code.
>
> Test cases to consider about that would be:
>   struct OnePointer { void *P; };
>   __fastcall void test0(OnePointer op);
> or:
>   struct TwoPointers { void *P1, *P2; };
>   __fastcall void test1(TwoPointers tp);
It does put the arguments onto stack and these tests work fine with my
proposed patch.
(I had to apply the stdcall-double-mangle.patch patch by rnk@ to fix
the fastcall mangling though)

> John.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug_13676_p1_6.patch
Type: application/octet-stream
Size: 22347 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130412/12983089/attachment.obj>