[cfe-commits] [PATCH] Supporting thiscall compatibility with MSVC

[Aaron Ballman]

On Thu, Feb 16, 2012 at 9:50 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Thu, Feb 16, 2012 at 7:38 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Thu, Feb 16, 2012 at 8:52 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>> On Thu, Feb 16, 2012 at 6:25 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>> Here's the next attempt at the MSVC thiscall support patch, this time
>>>> with test cases and an improved tablegen declaration.
>>>>
>>>> This is the first time I've done a test case for clang or llvm
>>>> codegen, so please pay special attention to the test cases and let me
>>>> know if I'm off-base (and what I should do differently).
>>>
>>> You might want to make the LLVM testcase a bit stronger by using CHECK-NEXT.
>>
>> Does CHECK-NEXT basically chain the checks together to check several
>> lines as a group?
>
> Basically, yes.

The problem is that the assembly is different between O levels.  So
-O0 does leal, leal, but -O1 does leal, movl.

Is there a way for me to handle this in the test case, or should I
just be explicit about the O level and its behavior?

>>> +  if (callingConvention == llvm::CallingConv::X86_ThisCall &&
>>> +      Context.getLangOptions().MicrosoftMode && RT->isStructureType()) {
>>> +    return false;
>>> +  }
>>>
>>> You shouldn't make the behavior of thiscall depend on MicrosoftMode;
>>> checking that the calling convention is thiscall should be sufficient.
>>>  (If someone explicitly requests thiscall outside of MicrosoftMode,
>>> they expect a Microsoft-compatible calling convention.)
>>
>> I didn't want to break compatibility if people were already using
>> thiscall (for instance, for MinGW compiles).
>>
>> Takumi, I believe you know far more about MinGW than I do -- do you
>> think this change would break anything if I were to make it the
>> default for thiscall instead of MicrosoftMode only?
>
> Note that MicrosoftMode isn't the right way to check for mingw anyway;
> take a look at how IsWin32FloatStructABI works.

Thanks for the pointer -- but the question still stands about whether
we'd want this change for non-MSVC compatibility as well.

~Aaron