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

[Aaron Ballman]

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.

~Aaron
-------------- next part --------------
A non-text attachment was scrubbed...
Name: msvc thiscall llvm.patch
Type: application/octet-stream
Size: 2481 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120218/37c09041/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: msvc thiscall clang.patch
Type: application/octet-stream
Size: 5079 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120218/37c09041/attachment-0001.obj>