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

[Davide Italiano via llvm-commits]

On Fri, Jan 27, 2017 at 3:26 PM, Sanjay Patel via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: spatel
> Date: Fri Jan 27 17:26:27 2017
> New Revision: 293345
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293345&view=rev
> Log:
> [InstCombine] move icmp transforms that might be recognized as min/max and inf-loop (PR31751)
>
> This is a minimal patch to avoid the infinite loop in:
> https://llvm.org/bugs/show_bug.cgi?id=31751
>
> But the general problem is bigger: we're not canonicalizing all of the min/max forms reported
> by value tracking's matchSelectPattern(), and we don't define min/max consistently. Some code
> uses matchSelectPattern(), other code uses matchers like m_Umax, and others have their own
> inline definitions which may be subtly different from any of the above.
>
> The reason that the test cases in this patch need a cast op to trigger is because we don't
> (yet) canonicalize all min/max forms based on matchSelectPattern() in
> canonicalizeMinMaxWithConstant(), but we do make min/max+cast transforms based on
> matchSelectPattern() in visitSelectInst().
>
> The location of the icmp transforms that trigger the inf-loop seems arbitrary at best, so
> I'm moving those behind the min/max fence in visitICmpInst() as the quick fix.
>

Ehm.. this workaround seems a little fragile. Maybe reverting the
original commit which introduced this behavior would be a better thing
for now? https://reviews.llvm.org/rL286318 (i.e. until we have an idea
on how to fix this properly)