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

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 28 13:32:46 PST 2019


lebedev.ri accepted this revision.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

LGTM, thank you.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:707
 
   // Account for swapped form of subtraction: ((a > b) ? b - a : 0).
+  // Checking for both a-b and a+(-b) as a constant.
----------------
dmgreen wrote:
> lebedev.ri wrote:
> > I'm having doubt about the comment.
> > If `b < a`, then `b - a` will wrap, and we'll return wrapped result.
> > And if `b >= a`, then `b - a` will **not** wrap, but we will saturate to `0`.
> > What am i missing?
> From the comment below, this looks like it is referring to
> `(a > b) ? b - a : 0 -> -usub.sat(a, b)`
> I'll try and update it to be clearer.
Aha, okay, thank you.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:722-724
+  // If we are adding a negate and the sub and icmp are used anywhere else, we
+  // would end up with more instructions.
+  if (IsNegative && !TrueVal->hasOneUse() && !ICI->hasOneUse())
----------------
I would recommend commit this as a separate commit first.


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

https://reviews.llvm.org/D69514





More information about the llvm-commits mailing list