[llvm-commits] [llvm] r57925 - in /llvm/trunk: lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp test/CodeGen/X86/pr2924.ll

[Tomas Lindquist Olsen]

On Thu, Oct 23, 2008 at 7:06 AM, Tanya Lattner <lattner at apple.com> wrote:

> Chris is 100% correct. We don't merge in patches at this point that are not
> regressions in llvm-test or dejanu checks from 2.3. I'll be detailing this
> in a release policy document for 2.5. Because there is some confusion on the
> issue, I've merged this into 2.4, but this would not normally meet our
> criteria.
> We have to draw the line at some point or we would never get out a release.
> With every patch we merge in there is risk associated with it and requires a
> full run of the test suite on all the platforms we support. We try very hard
> to have high quality releases, so you can understand why we are
> conservative.
>
> -Tanya
>
> On Oct 22, 2008, at 10:40 AM, Tomas Lindquist Olsen wrote:
>
>
>
> On Wed, Oct 22, 2008 at 7:23 PM, Chris Lattner <clattner at apple.com> wrote:
>
>>
>> On Oct 22, 2008, at 1:15 AM, Tomas Lindquist Olsen wrote:
>>
>> On Tue, Oct 21, 2008 at 10:00 PM, Dan Gohman <gohman at apple.com> wrote:
>>
>>> Author: djg
>>> Date: Tue Oct 21 15:00:42 2008
>>> New Revision: 57925
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=57925&view=rev
>>> Log:
>>> Fix SelectionDAGBuild lowering of Select instructions to
>>> handle first-class aggregate values. Also, fix a bug in
>>> the Ret handling for empty aggregates.
>>>
>>> Added:
>>>    llvm/trunk/test/CodeGen/X86/pr2924.ll
>>> Modified:
>>>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuild.cpp
>>>
>>>
>> Any chance this will make it into the 2.4 branch ?
>>
>>
>> If you have an example of C code that worked with llvm-gcc in 2.3 but that
>> fails with llvm-gcc in 2.4, then it would be a regression and yes we would
>> want to pull it in.  Otherwise unfortunately we shouldn't pull it in.
>>
>> -Chris
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
> Ok :(
>
> I don't have llvm-gcc, this code is generated by my D compiler.
>
> Using first class aggregates only started working after this fix (at least
> when doing optimizations, which is what produces the select inst)
>
> Sounds like we won't be able to support the 2.4 release. Oh well...
>
> If I do find some time for it, what kind of C/C++ code would I write to
> generate something that produces first class aggregates ?
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
First, thanx a lot for making the exception :)
About the reason that I was asking for this in the first place, I dont't
think I was clear enough on why it was important....

The thing is we don't generate the select, it's produced by one of the LLVM
optimization passes (it was fixed so fast I never found out which one,
rather just updated llvm). Our unoptimized LLVM code codegen'd as it should,
but after optimization the select would be there and break codegen, so we
could have lived without it, if we could live without optimizations...

The other option would have been to revert my changes that introduced these
FCA's, but they're needed as part of my work on following the D calling
convention / ABI.

I hope this is a bit more clear...

I can fully understand the reasons for not merging in every fix, from
working on our own compiler, I've seen how a seemingly small change can
break lots of code that you would never have thought was even affected.

Thanx again for merging it in.
-Tomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20081023/1ac98821/attachment.html>