[llvm] r204912 - InstCombine: merge constants in both operands of icmp.

[Chandler Carruth]

I would suggest reverting and discussing more.

I'm not really happy with the one-use hack to "fix" this. I think there
should be a better way that more directly addresses the problem of
extending the liveness of other SSA values, or essentially the
canonicalization phase ordering problem that is here.


On Thu, Mar 27, 2014 at 1:47 PM, Tobias Grosser <tobias at grosser.es> wrote:

> On 03/27/2014 08:42 PM, Reid Kleckner wrote:
>
>> On Thu, Mar 27, 2014 at 12:30 PM, Benjamin Kramer <benny.kra at gmail.com
>> >wrote:
>>
>>
>>> On 27.03.2014, at 20:23, Erik Verbruggen <erikjv at me.com> wrote:
>>>
>>>  Actually, I'm wondering if it's better to revert my patch (and yours).
>>>>
>>> The "nightly" test-suite pointed out a regression. If you have a tight
>>> for
>>> loop, like:
>>>
>>>>     for (i = 0; i <= 39; i++)
>>>>        doSomethingHere;
>>>>
>>>> then before the patch it would be:
>>>>     %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>>>>     %exitcond = icmp eq i64 %indvars.iv.next, 40
>>>>
>>>> while with the patch it's:
>>>>     %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>>>>     %exitcond = icmp eq i64 %indvars.iv, 39
>>>>
>>>> That keeps the "old" %indvars.iv alive. This causes some performance
>>>>
>>> tests to fail, like MultiSource/Benchmarks/MiBench/telecomm-gsm.
>>>
>>>>
>>>> I Cc-ed d0k because he did the review, so what do you guys think?
>>>>
>>>
>>> A simple fix would be to check if the add has just one use. I was hoping
>>> that this kind of workaround wouldn't be necessary anymore but it looks
>>> like we still need it in this case :|
>>>
>>
>>
>> I'm not really an optimizer person, so whatever works for you guys, revert
>> or fix forward.  I'm mostly concerned with the miscompile, and that has a
>> test case now.  :)
>>
>
> Yes, I am getting spammed by the LNT buildbots the whole day. Would you
> guys mind to either commit a fix now or just revert until the issue has
> been solved.
>
> Thank you,
> Tobias
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140327/9f409219/attachment.html>