[PATCH] D84655: [InstSimplify] fold min/max intrinsics using icmp simplification

Mon Jul 27 12:03:18 PDT 2020

nikic added a subscriber: fhahn.
nikic added a comment.

> But (if my undef reasoning is correct), this is not as easy as always returning the existing operand. In the case of undefs, we need to clamp to the limit constant. This is similar to what we do with the saturating math intrinsics.

Gah, undef strikes again. I don't think what you did here is sufficient, because the SimplifyICmpInst() may depend on undef in a more roundabout manner than a direct undef element in one of the min/max operands. I think in the short term this needs to be guarded by guaranteed-not-undef, at least for the operand that we simplify to.

In the longer term, I think we really need to introduce an InstSimplify query flag that disables undef-based folds. I think at this point we have multiple places where we need this. This is one, it is also the imho more proper solution to D84250 <https://reviews.llvm.org/D84250>, and @fhahn mentioned that this may be needed for NewGVN correctness as well.

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

https://reviews.llvm.org/D84655