Fix SRet for thiscall in i686-pc-win32

Timur Iskhodzhanov timurrrr at google.com
Wed Apr 3 04:31:54 PDT 2013


Committed as r178634, thanks!

2013/4/3 Aaron Ballman <aaron at aaronballman.com>:
> This looks correct to me -- thanks for taking care of this!
>
> ~Aaron
>
> On Thu, Mar 28, 2013 at 10:42 PM, Timur Iskhodzhanov
> <timurrrr at google.com> wrote:
>> Hi Aaron, Anton,
>>
>> Can you please review the attached patch?
>>
>> With this patch and some Clang-only changes, all my llvm.org/PR13676
>> compatibility tests pass fine. That said, this is likely all we need
>> to do in LLVM to fix PR13676, at least to the best of my knowledge.
>>
>> This patch substitutes r151123 by Aaron which I find wrong - the
>> incorrect expectations in test/CodeGen/X86/thiscall-struct-return.ll
>> led to a wrong solution, also the test used a wrong triple.
>>
>> As I wrote my own test before realizing there is a test from r151123
>> [1], and I find my test to work with thiscall better (e.g. it uses
>> conventional method mangling rather that Itanium mangling and has
>> better/more expectations), I've decided to delete Aaron's test.
>>
>> Please note that my recent commit r178291 makes sure that EAX has the
>> right value when we leave a thiscall method.
>>
>> [1] it was passing regardless of initial
>> CCIfSubtarget<"isTargetWindows(), ..." change due to a wrong triple. I
>> later removed the CCIfSubtarget condition...
>>
>> Anton,
>> Am I right that there's no thiscall in gcc/MinGW?
>> If there is, I can put CCIfSubtarget<"isTargetWindows()", ..." back.
>>
>> Thanks,
>> Timur



More information about the llvm-commits mailing list