[InstCombine] fix profitability equation for max-of-nots transform

As the test change shows, we can increase the critical path by adding
a 'not' instruction, so make sure that we're actually removing an
instruction if we do this transform.

This transform could also cause us to miss folds of min/max pairs.

llvm-svn: 286315

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 6bee87c..06b769b 100644 (file)
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1308,15 +1308,14 @@ Instruction *InstCombiner::visitSelectInst(SelectInst &SI) {
     if ((SPF == SPF_SMAX || SPF == SPF_UMAX) &&
         IsFreeToInvert(LHS, LHS->hasNUses(2)) &&
         IsFreeToInvert(RHS, RHS->hasNUses(2))) {
-      // This transform adds a not operation, and that extra cost needs to be
-      // justified. We look for simplifications that will result from applying
-      // this rule:
-      bool Profitable =
-          (LHS->hasNUses(2) && match(LHS, m_Not(m_Value()))) ||
-          (RHS->hasNUses(2) && match(RHS, m_Not(m_Value()))) ||
+      // For this transform to be profitable, we need to eliminate at least two
+      // 'not' instructions if we're going to add one 'not' instruction.
+      int NumberOfNots =
+          (LHS->hasNUses(2) && match(LHS, m_Not(m_Value()))) +
+          (RHS->hasNUses(2) && match(RHS, m_Not(m_Value()))) +
           (SI.hasOneUse() && match(*SI.user_begin(), m_Not(m_Value())));
 
-      if (Profitable) {
+      if (NumberOfNots >= 2) {
         Value *NewLHS = Builder->CreateNot(LHS);
         Value *NewRHS = Builder->CreateNot(RHS);
         Value *NewCmp = SPF == SPF_SMAX

diff --git a/llvm/test/Transforms/InstCombine/max-of-nots.ll b/llvm/test/Transforms/InstCombine/max-of-nots.ll
index ca9520a..96fac52 100644 (file)
--- a/llvm/test/Transforms/InstCombine/max-of-nots.ll
+++ b/llvm/test/Transforms/InstCombine/max-of-nots.ll
@@ -34,13 +34,15 @@ define i32 @compute_min_3(i32 %x, i32 %y, i32 %z) {
   ret i32 %min
 }
 
+; Don't increase the critical path by moving the 'not' op after the 'select'.
+
 define i32 @compute_min_arithmetic(i32 %x, i32 %y) {
 ; CHECK-LABEL: @compute_min_arithmetic(
-; CHECK-NEXT:    [[TMP1:%.*]] = add i32 %x, -4
-; CHECK-NEXT:    [[TMP2:%.*]] = icmp slt i32 [[TMP1]], %y
-; CHECK-NEXT:    [[TMP3:%.*]] = select i1 [[TMP2]], i32 [[TMP1]], i32 %y
-; CHECK-NEXT:    [[TMP4:%.*]] = xor i32 [[TMP3]], -1
-; CHECK-NEXT:    ret i32 [[TMP4]]
+; CHECK-NEXT:    [[NOT_VALUE:%.*]] = sub i32 3, %x
+; CHECK-NEXT:    [[NOT_Y:%.*]] = xor i32 %y, -1
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[NOT_VALUE]], [[NOT_Y]]
+; CHECK-NEXT:    [[NOT_MIN:%.*]] = select i1 [[CMP]], i32 [[NOT_VALUE]], i32 [[NOT_Y]]
+; CHECK-NEXT:    ret i32 [[NOT_MIN]]
 ;
   %not_value = sub i32 3, %x
   %not_y = sub i32 -1, %y