[LLVMdev] Handling SRet on Windows x86

[Timur Iskhodzhanov]

On Tue, Oct 2, 2012 at 8:44 PM, Anton Korobeynikov <asl at math.spbu.ru> wrote:
>>>> b) Should I be altering CC_X86_32_ThisCall
>>>>     OR should I introduce CC_X86_Win32_ThisCall instead?
>>>>     [Answer not clear to me - is there any platform besides Windows
>>>>     that uses thiscall?]
>>> no
>> Can you please clarify which question you've answered here?
> To both. You're assuming that Windows == MSVC. This is not true.
Ah, got it.
Sounds like we might need to introduce CC_X86_Win32_MSVC_ThisCall then?..

On Tue, Oct 2, 2012 at 10:22 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Tue, Oct 2, 2012 at 11:02 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> On Tue, Oct 2, 2012 at 8:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> On Tue, Oct 2, 2012 at 8:54 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>>>> [+cfe-dev as this does seem like both LLVM+Clang issue]
>>>> [Sorry for an incomplete e-mail context, please see
>>>> http://llvm.org/PR13676#c6 if you're interested]
>>>>
>>>> I've read these bugs and now I'm even more confused than I was before :)
>>>>
>>>> What do you think about the following approach:
>>>> a) I'll create test cases for the major issues I've observed so far
>>>>    These test cases will check both -emit-llvm and llc output
>>>
>>> Just an aside: generally Clang tests should just verify the emitted
>>> LLVM bitcode.
>> I know about that, see below.
>>
>>> If you want to test what machine code/assembly that
>>> compiles down to, that should be an LLVM test that starts at LLVM
>>> bitcode and goes down to machine code/assembly.
>> Are there any serious reasons not to do combined -emit-llvm+llc tests
>> for such issues that need both LLVM and Clang support?
>> I understand usually it's easy to split but in this particular case it
>> seems like the generated bitcode might need to be changed during Clang
>> patching.
>
> I'm not sure I follow your reasoning here. That Clang will produce
> different bitcode wouldn't change the desire to test these components
> separately.
>
> * when the tests fail we have a more precise idea about what went wrong.
> * less duplicated test work - we should be testing the LLVM bitcode
> features once in the LLVM suite, not once for each different client
> use case
> * better chance of catching regressions - if we test LLVM in the Clang
> suite, those working only on LLVM might regress Clang without
> realizing it
I got your point.
I'll start with combined clang+llc tests locally without committing it
and split into separate clang and llc tests before sending out a final
patch for review then.
I do understand the convenience of "more precise idea of what went
wrong" but I'm not sure it'll help much during early development as
it's not clear what is the "right" LL code is for a give assembly :)
There might be many options (e.g. SRet or not SRet? ThisCall or
MSVC_ThisCall? etc)

Does that sound reasonable this way?

>> In this particular case I believe having a combined test is much more
>> convenient.
>> WDYT?
>>
>>>>    They'll have CHECKs for stuff that already works and
>>>> FIXME+CHECK-NOT for stuff that doesn't.
>>>>  I guess I should put these tests in clang/test/CodeGen[CXX] ?
>>>>
>>>> b) As a short-term solution to avoid blocking progress for those who
>>>> are interested in a functioning Windows compiler I'll publish my patch
>>>> [which breaks the non-Windows compatibility but improves Windows
>>>> compat] in PR13676.
>>>>
>>>> c) Having these test cases at hand, we can come up with a decent
>>>> long-term solution
>>>>
>>>> Does that sound good to you?
>>>>
>>>> On Tue, Oct 2, 2012 at 7:31 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>>>>> On Tue, Oct 2, 2012 at 7:03 PM, Anton Korobeynikov <asl at math.spbu.ru> wrote:
>>>>>> Hello Timur,
>>>>>>
>>>>>>> I'd like to ask for advice:
>>>>>>> a) Is it OK to change the SRet/ThisCall behaviour on non-Windows platforms?
>>>>>>>     [I suppose no]
>>>>>> no
>>>>>>
>>>>>>>
>>>>>>> b) Should I be altering CC_X86_32_ThisCall
>>>>>>>     OR should I introduce CC_X86_Win32_ThisCall instead?
>>>>>>>     [Answer not clear to me - is there any platform besides Windows
>>>>>>>     that uses thiscall?]
>>>>>> no
>>>>> Can you please clarify which question you've answered here?
>>>>> Sorry for making the ambiguous questions in the first place :)
>>>>>
>>>>>> It seems for me that you're trying to solve the problem from the wrong
>>>>>> end. As far as I remember, there is a difference - "simple" (probable
>>>>>> POD-like stuff) are returned in the regs, while classes with
>>>>>> non-trivial ctors, etc. are passed / returned on stack.
>>>>> Sort of.
>>>>>
>>>>>> It's frontend responsibility to emit proper IR in this case.
>>>>> Isn't it what's SRet is supposed to be?
>>>>>
>>>>>> See http://llvm.org/bugs/show_bug.cgi?id=5064 and around. This seems
>>>>>> to be the correct description of what's going on.
>>>>> FTR, http://llvm.org/bugs/show_bug.cgi?id=5058 seems to be more up-to-date.
>>>>>
>>>>> Thanks for your reply!
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev