[PATCH] D59378: [InstCombine] Prevent icmp transform that can cause inf loop if part of min/max

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D59378#1433157 <https://reviews.llvm.org/D59378#1433157>, @tejohnson wrote:

> In D59378#1432514 <https://reviews.llvm.org/D59378#1432514>, @nikic wrote:
>
> > > I'd prefer that we IR simplify our way out of this infinite loop instead of looking the other way though. Ie, can we get this in instsimplify using a ConstantRange?
> >
> > That should be relatively simple to do, we just need to support constant range calculation for min/max flavor selects in computeConstantRange().
>
>
> One thing I didn't mention is that this opportunity was exposed only by InstCombine's instruction sinking. I captured the code after sinking to create the test case.
>
> Also, is it reasonable to assume prior passes will transform the code to avoid triggering possible infinite loops in later passes like this? Unless you mean move part of this transformation into InstSimplify and out of InstCombine?


instsimplify and instcombine have a special relationship: instcombine always calls instsimplify as an analysis on an instruction before trying more general combines (because we can avoid a lot of work if we can kill the instruction first). So that lets us assert that some patterns just don't exist by the time general instcombine is running.

> If that is a better longer term fix for these issues, it would be good if this one can go in for now to block the infinite loop.

@nikic - do you have time to work on the instsimplify change? I'm ok if we want to add this patch as an immediate work-around for the hang, but if we need to take that option, let's mark this with a FIXME comment because we know there's a better solution.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59378/new/

https://reviews.llvm.org/D59378