Fix SRet for thiscall in i686-pc-win32

Aaron Ballman aaron at aaronballman.com
Tue Apr 2 17:39:56 PDT 2013


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