[PATCH] D85929: [InstSimplify] Fold min/max intrinsic based on icmp of operands

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 23 12:42:39 PDT 2020


nikic added a comment.

In D85929#2218748 <https://reviews.llvm.org/D85929#2218748>, @spatel wrote:

> I'm iffy about this. I was hoping that we could either remove the explicit folds or show more wins by using the general icmp simplification.
> Based on the test diffs, we're only getting assume improvements, and that doesn't seem worth the effort of full icmp simplify.
> It would be nice to show a more meaningful motivating case, but I doubt we will get that until we start canonicalizing to the intrinsics. 
> Thoughts?

I only used assume tests because that seemed like the most direct way to test the change. We also get all other folds that are implemented for icmp, e.g. we recognize that

  %add = add nuw i8 %x, 1
  %max = call i8 @llvm.umax.i8(i8 %x, i8 %add)
  ret i8 %max

is the same as `ret i8 %add`. I'm not sure how valuable it would be to explicitly test these, as they are already covered by the icmp tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85929



More information about the llvm-commits mailing list