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

[Reid Kleckner]

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.  :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140327/6842289f/attachment.html>