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

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added a comment.

In D84655#2179906 <https://reviews.llvm.org/D84655#2179906>, @nikic wrote:

> The problem is that this does not hold. It's okay to go from icmp+select to max, but not the other way around. The difference is exactly that max requires a unique value for a and b, while the icmp+select sequence allows a and b to be chosen independently for the icmp and the select, if they are undef. This is also the reason why many of our existing min/max transforms are not actually legal when undef is taken into consideration.

I see your point. You're right, they are not equivalent. `max(undef, 3) -> undef` is invalid, which is ok if it was represented with icmp + select.
It is sad that the decomposition of max to icmp + select is invalid. It could have made this kind of reasoning simpler.

I think either allowing handful transformations for max/min only or using InstSimplify after a flag for unusing undef is added is fine.


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

https://reviews.llvm.org/D84655