[llvm] r293345 - [InstCombine] move icmp transforms that might be recognized as min/max and inf-loop (PR31751)

[Sanjay Patel via llvm-commits]

Understood - my POV is from trunk.

If people think it's safer to revert patches for 4.0, I'd suggest reverting
everything that was added to value tracking's matchMinMax() since 3.9. That
should remove doubt (but miss quite a few min/max improvements).

On Fri, Jan 27, 2017 at 5:52 PM, Davide Italiano <davide at freebsd.org> wrote:

> On Fri, Jan 27, 2017 at 4:42 PM, Sanjay Patel <spatel at rotateright.com>
> wrote:
> > This patch targets precisely the patterns that were matched in r286318,
> so I
> > don't see how reverting that would be a better solution. And I was being
> > generous by calling the existing code "arbitrary" - those folds were in
> > foldICmpUsingKnownBits(), and they have nothing do with known bits. :)
> >
>
> This problem seems to manifest only after you added the matchers, right?
> So maybe a different solution would be that not matching these
> constructs until there are problems surrounding them (as we don't
> really know what could be the effect down the road). If you don't want
> to do that in trunk and see if things stick together, fair enough, but
> I think it's more cautious removing the matchers for 4.0 as they may
> expose other problems we'
> re not really aware of. Up to you anyway.
>
> > We'd still have the general problem that various parts of the code define
> > min/max differently. The bigger solution is to canonicalize min/max
> harder
> > and standardize the min/max definitions, but that's going to take time to
> > sort out all the places we have ad hoc min/max checks and then make sure
> > that the code still does what was intended (and doesn't induce inf-loop
> from
> > some other transform).
> >
>
> Agree. I think this problem should be addressed before adding new
> matchers for min/max and not in parallel (or after).
>
> Thanks,
>
> --
> Davide
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170127/a29ef9c6/attachment.html>