[PATCH] D110038: [InstCombine] move add after min/max intrinsic

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: lebedev.ri, nikic, xbolva00, reames.
Herald added subscribers: hiraditya, mcrosier.
spatel requested review of this revision.
Herald added a project: LLVM.

This is another regression noted with the proposal to canonicalize to the min/max intrinsics in D98152 <https://reviews.llvm.org/D98152>.

Here are Alive2 attempts to show correctness without specifying exact constants:
https://alive2.llvm.org/ce/z/bvfCwh (smax)
https://alive2.llvm.org/ce/z/of7eqy (smin)
https://alive2.llvm.org/ce/z/2Xtxoh (umax)
https://alive2.llvm.org/ce/z/Rm4Ad8 (umin)
(if you comment out the assume and/or no-wrap, you should see failures)

The different output for the umin test is due to a fold added with c4fc2cb5b2d98125 <https://reviews.llvm.org/rGc4fc2cb5b2d98125e9035d9498640c7d6f17c8da> :

  // umin(x, 1) == zext(x != 0)

We probably want to adjust that, so it applies more generally (umax --> sext? or patterns where we can fold to select-of-constants). Some folds that were ok when starting with cmp+select may increase instruction count for the equivalent intrinsic, so we have to decide if it's worth altering a min/max.


https://reviews.llvm.org/D110038

Files:
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/minmax-intrinsics.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110038.373457.patch
Type: text/x-patch
Size: 8630 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210919/e98ebba8/attachment.bin>