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

[John McCall]

On Apr 12, 2013, at 12:38 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> 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)

If that's the C ABI rule, it should be handled by the C ABI, i.e. the normal
ABIInfo handling for the type.  But, for example, it seems like X64 does not
follow this rule, and will happily pass structs of size 1, 2, 4, or 8 in a register.

John.