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

Tue Jul 28 09:28:01 PDT 2020

aqjune added a comment.

In D84655#2178544 <https://reviews.llvm.org/D84655#2178544>, @spatel wrote:

> Yes, this reminds me of D77868 <https://reviews.llvm.org/D77868>, so I'm worried that we can't trust SimplifyICmpInst without a stronger undef check. Given that, I'd rather abandon this approach. I think we can put in a handful of direct min/max simplifications and be confident that we have good optimization and safety. Ie, back to the approach started with rG0481e1a <https://reviews.llvm.org/rG0481e1ae3c17a8faa6c8e319495a51a76b77b76b>.

I think this case is a bit different from D77868 <https://reviews.llvm.org/D77868>: min/max can be expressed as a composition of icmp with select.

  v = smax(a, b)
  <=>
  c = icmp sgt a, b
  v = select c, a, b

It seems legal to use `SimplifyICmpInst` to get the simplified value of `c`.
If these two expressions are different (which can happen when e.g. smax/smin are defined to block poison), using `SimplifyICmpInst` may be illegal.
(BTW, this equivalence seems to be the most important thing to prove first once the min/max intrinsic functions are added into Alive2.)

In case of D77868 <https://reviews.llvm.org/D77868>, using `SimplifyAndInst/SimplifyOrInst` to get the value of `select` was illegal, because `select` is not equivalent to `and/or`.

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

https://reviews.llvm.org/D84655