[llvm] 2a0837a - [InstCombine] fix sub(add(X,Y),umin(Y,Z)) --> add(X,usub.sat(Y,Z))

[Chenbing Zheng via llvm-commits]

Author: Chenbing Zheng
Date: 2022-05-13T09:54:10+08:00
New Revision: 2a0837aab1489c88efb03784e34c4dc9f2e28302

URL: https://github.com/llvm/llvm-project/commit/2a0837aab1489c88efb03784e34c4dc9f2e28302
DIFF: https://github.com/llvm/llvm-project/commit/2a0837aab1489c88efb03784e34c4dc9f2e28302.diff

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

This patch fix bug left in D124503. We should do
sub(add(X,Z),umin(Y,Z)) --> add(X,usub.sat(Z,Y)) instead of
sub(add(X,Z),umin(Y,Z)) --> add(X,usub.sat(Y,Z)).

Reviewed By: spatel

Differential Revision: https://reviews.llvm.org/D125352

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/test/Transforms/InstCombine/sub-minmax.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 15176fc2055f4..d7c4f72399a88 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -2031,14 +2031,17 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
 
     {
       // sub(add(X,Y),umin(Y,Z)) --> add(X,usub.sat(Y,Z))
-      // sub(add(X,Z),umin(Y,Z)) --> add(X,usub.sat(Y,Z))
+      // sub(add(X,Z),umin(Y,Z)) --> add(X,usub.sat(Z,Y))
       Value *X, *Y, *Z;
-      if (match(Op1, m_OneUse(m_UMin(m_Value(Y), m_Value(Z)))) &&
-          (match(Op0, m_OneUse(m_c_Add(m_Specific(Y), m_Value(X)))) ||
-           match(Op0, m_OneUse(m_c_Add(m_Specific(Z), m_Value(X)))))) {
-        Value *USub =
-            Builder.CreateIntrinsic(Intrinsic::usub_sat, I.getType(), {Y, Z});
-        return BinaryOperator::CreateAdd(X, USub);
+      if (match(Op1, m_OneUse(m_UMin(m_Value(Y), m_Value(Z))))) {
+        if (match(Op0, m_OneUse(m_c_Add(m_Specific(Y), m_Value(X)))))
+          return BinaryOperator::CreateAdd(
+              X, Builder.CreateIntrinsic(Intrinsic::usub_sat, I.getType(),
+                                         {Y, Z}));
+        if (match(Op0, m_OneUse(m_c_Add(m_Specific(Z), m_Value(X)))))
+          return BinaryOperator::CreateAdd(
+              X, Builder.CreateIntrinsic(Intrinsic::usub_sat, I.getType(),
+                                         {Z, Y}));
       }
     }
   }

diff  --git a/llvm/test/Transforms/InstCombine/sub-minmax.ll b/llvm/test/Transforms/InstCombine/sub-minmax.ll
index 832c723be7159..286e9d69444d6 100644
--- a/llvm/test/Transforms/InstCombine/sub-minmax.ll
+++ b/llvm/test/Transforms/InstCombine/sub-minmax.ll
@@ -762,9 +762,7 @@ define i8 @
diff _add_use_umin_use(i8 %x, i8 %y) {
   ret i8 %s
 }
 
-;
-; TODO: sub(add(X,Y),umin(Y,Z)) --> add(X,usubsat(Y,Z))
-;
+; sub(add(X,Y),umin(Y,Z)) --> add(X,usubsat(Y,Z))
 
 define i8 @sub_add_umin(i8 %x, i8 %y, i8 %z) {
 ; CHECK-LABEL: define {{[^@]+}}@sub_add_umin
@@ -782,7 +780,7 @@ define i8 @sub_add_umin(i8 %x, i8 %y, i8 %z) {
 define i8 @sub_add_umin_commute_umin(i8 %x, i8 %y, i8 %z) {
 ; CHECK-LABEL: define {{[^@]+}}@sub_add_umin_commute_umin
 ; CHECK-SAME: (i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[Z:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.usub.sat.i8(i8 [[Z]], i8 [[Y]])
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.usub.sat.i8(i8 [[Y]], i8 [[Z]])
 ; CHECK-NEXT:    [[S:%.*]] = add i8 [[TMP1]], [[X]]
 ; CHECK-NEXT:    ret i8 [[S]]
 ;
@@ -808,7 +806,7 @@ define i8 @sub_add_umin_commute_add(i8 %x, i8 %y, i8 %z) {
 define i8 @sub_add_umin_commute_add_umin(i8 %x, i8 %y, i8 %z) {
 ; CHECK-LABEL: define {{[^@]+}}@sub_add_umin_commute_add_umin
 ; CHECK-SAME: (i8 [[X:%.*]], i8 [[Y:%.*]], i8 [[Z:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.usub.sat.i8(i8 [[Z]], i8 [[Y]])
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.usub.sat.i8(i8 [[Y]], i8 [[Z]])
 ; CHECK-NEXT:    [[S:%.*]] = add i8 [[TMP1]], [[X]]
 ; CHECK-NEXT:    ret i8 [[S]]
 ;