[PATCH] D69514: [InstCombine] Expand usub_sat patterns to handle constants

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.

(marking as reviewed)

Will you also look into the edge-case patterns, like:

  unsigned t0(unsigned num) {
      return num > 0 ? num-1 : num;
  }

https://godbolt.org/z/DsBxd8

  Name: unsigned minus 1
    %2 = icmp ugt i32 %0, 1
    %3 = add i32 %0, -1
    %4 = select i1 %2, i32 %3, i32 0
    ret i32 %4
  =>
    %r = usub_sat %0, 1
    ret %r
    %3 = add i32 %0, -1
    %2 = icmp ugt i32 %0, 1
    %4 = select i1 %2, i32 %3, i32 0
  
  Done: 1
  Optimization is correct!

(not sure what other ones are there, this is the one that i noticed in real code..)



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:722
   // afterwards.
-  if (!TrueVal->hasOneUse())
+  if (!TrueVal->hasOneUse() || !ICI->hasOneUse())
     return nullptr;
----------------
Err, this isn't quite right. I should have been more specific.

We produce a single instruction `@llvm.usub.sat.`,
so there shouldn't be a one-use check in general.

But we also may need to negate the result.
So *if* we need to negate the result,
*then* we need ensure that a *single* instruction
goes away - **either** `icmp` **or** `sub`.



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

https://reviews.llvm.org/D69514