[PATCH] D124503: [InstCombine] sub(add(X,Y),umin(Y,Z)) --> add(X,usub.sat(Y,Z))

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2043
+        if (match(Op1, m_UMin(m_Specific(Y), m_Value(Z))) ||
+            match(Op1, m_UMin(m_Value(Z), m_Specific(Y)))) {
+          Value *USub =
----------------
Is it worth putting the fold within the dyn_cast<MinMaxIntrinsic>(Op1) check above?


================
Comment at: llvm/test/Transforms/InstCombine/sub-minmax.ll:734
 
+define i8 @sub_add_umin_1(i8 %x, i8 %y, i8 %z) {
+; CHECK-LABEL: @sub_add_umin_1(
----------------
(style) Don't enumerate tests - use the name to describe the actual test (commutations etc).


================
Comment at: llvm/test/Transforms/InstCombine/sub-minmax.ll:767
+  %s = sub i8 %a, %m
+  ret i8 %s
+}
----------------
vector tests?
you probably want negative tests for where the m_Specific() match fails as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124503