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 outputJust an aside: generally Clang tests should just verify the emitted LLVM bitcode. 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.> 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
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. 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
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> 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