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

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

Hi,
Is there a case where depending on SimplifyICmpInst may be problematic? I'm not very familiar with the history of miscompilation related with InstSimplify (except the distribution of undef), so it is possible that I'm missing something.

  v = smax(x, y)
  use(v)
    => // isICmpTrue(sgt, x, y) is true
  v = smax(x, y)
  use(x) // Is this valid?

For the D84250 <https://reviews.llvm.org/D84250>, the fix had to be conservative because distribution is incorrect under presence of undef. If InstSimplify can be arbitrarily smart so e.g. it iterates over the basic block and find the equivalent expression (as GVN does), this can cause miscompliation:

  tmp = x + x
  v = x * (1 + 1) // InstSimplify reaches to the conclusion that this is equivalent to x + x, finds tmp, so it uses tmp instead.
  ->
  tmp = x + x
  v = tmp // This is wrong, because previously v is always an even, but not it can be anything if x is undef.


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

https://reviews.llvm.org/D84655