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

Eli Friedman eli.friedman at gmail.com
Tue Feb 21 17:45:14 PST 2012


On Sat, Feb 18, 2012 at 9:36 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Fri, Feb 17, 2012 at 7:30 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Fri, Feb 17, 2012 at 3:41 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>> On Thu, Feb 16, 2012 at 10:42 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>>> On Thu, Feb 16, 2012 at 8:39 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>>> 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?
>>>>
>>>> We always run tests at the same -O level, so you shouldn't worry about
>>>> that.  That said, if you think splitting your testcase into multiple
>>>> functions would make it more stable, please do that.
>>>
>>> Here's another shot at the patch -- this time, I am using the proper
>>> way to test for the Win32 ABI, and I've updated the X86 codegen test
>>> case so that it's a bit more clear.
>>
>> Your test somehow disappeared form the clang patch.
>>
>> Please rename IsWin32FloatStructABI if you're going to use it this
>> way.  I'm still not convinced it's necessary to behave differently
>> when targeting MinGW, though.
>
> That was truly odd -- no idea how the test case got missed, but it's
> back again.  I've also removed the check for the Win32 ABI as it
> appears this is used by gcc (4.6 and up) for MS compat as well.

Both patches look good.

-Eli




More information about the llvm-commits mailing list