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

[Aaron Ballman]

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?

> +  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?

> The indentation in the clang patch still seems a bit weird in a few
> places, like the following:
>
> -    FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
> +    FI.getReturnInfo() = classifyReturnType(FI.getReturnType(),
> +                            FI.getCallingConvention());

Hmm, that is strange.  Visual Studio likes to mess with me sometimes.

> Otherwise, this is looking good.

Great, thanks!

~Aaron