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

[Tobias Grosser]

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