[PATCH] D84655: [InstSimplify] fold min/max intrinsics using icmp simplification
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 28 13:15:11 PDT 2020
nikic added a comment.
In D84655#2179046 <https://reviews.llvm.org/D84655#2179046>, @aqjune wrote:
> 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
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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84655/new/
https://reviews.llvm.org/D84655
More information about the llvm-commits
mailing list