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

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Tue Nov  8 18:13:11 2016
New Revision: 286315

URL: http://llvm.org/viewvc/llvm-project?rev=286315&view=rev
Log:
[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.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
    llvm/trunk/test/Transforms/InstCombine/max-of-nots.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=286315&r1=286314&r2=286315&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Tue Nov  8 18:13:11 2016
@@ -1308,15 +1308,14 @@ Instruction *InstCombiner::visitSelectIn
     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

Modified: llvm/trunk/test/Transforms/InstCombine/max-of-nots.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/max-of-nots.ll?rev=286315&r1=286314&r2=286315&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/max-of-nots.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/max-of-nots.ll Tue Nov  8 18:13:11 2016
@@ -34,13 +34,15 @@ define i32 @compute_min_3(i32 %x, i32 %y
   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