Added instcombine for 'MIN(MIN(A, 27), 93)' and 'MAX(MAX(A, 93), 27)'

[dinesh.d]

Summary

Added instcombine for 'MIN(MIN(A, 27), 93)' and 'MAX(MAX(A, 93), 27)'

MIN(MIN(A, 23), 97) -> MIN(A, 23)
MAX(MAX(A, 97), 23) -> MAX(A, 97)

[dinesh.d]

Changed if blocks to switch block and updated test cases

[• rafael]

Thanks.

lib/Transforms/InstCombine/InstCombineSelect.cpp

69

Why not implement it in here? In any case, this patch is no changing this, right? Maybe just leave the FIXME as is?

662

please clang-format the patch.

668

use a dyn_cast instead of an isa above. That way you don't need to cast in here.

671

I don't thin get can fail.

Creating a ConstExpr just to evaluate it seems wasteful. Can you compare the constants directly?

Do we a/ready handle cases like

MIN(MIN(A, 97), 23)?

If not, can you leave a FIXME in place for it.

[dinesh.d]

Thanks for review.

lib/Transforms/InstCombine/InstCombineSelect.cpp

69

I tried doing it but it has already been handled in visitSelectInstWithICmp()
If we want it here, I can refactor it and put that code here.

662

clang-format was formatting code lines in case statement in new line.
I have seen similar pattern in other places so tried to keep it similar.
I did run clang-format in update patch.

668

updated

671

I missed 'SI.getType()->isIntegerTy()' call before calling this function.
So wasn't sure that I can use ConstantInt and compare them. Update
in next patch.

Added TODO for MIN(MIN(A, 97), 23). I will try to fix it and send patch
for review soon.

[dinesh.d]

Updated as per comments.

[dinesh.d]

Gentle ping

[• rafael]

LGTM

[dinesh.d]

Submitted as r208849.

Thanks for review and support.

lib/Transforms/InstCombine/InstCombineSelect.cpp

switch (ICI->getPredicate()) {
       case ICmpInst::ICMP_SGE: return SPF_SMIN;
       case ICmpInst::ICMP_ULT:
       case ICmpInst::ICMP_ULE: return SPF_UMAX;
       case ICmpInst::ICMP_SLT:
       case ICmpInst::ICMP_SLE: return SPF_SMAX;
     }
   }
 
-  // TODO: (X > 4) ? X : 5   -->  (X >= 5) ? X : 5  -->  MAX(X, 5)
+  // (X > 4) ? X : 5   -->  (X >= 5) ? X : 5  -->  MAX(X, 5)
+  // Handled in visitSelectInstWithICmp())
 
   return SPF_UNKNOWN;
 }
 
 
 /// GetSelectFoldableOperands - We want to turn code that looks like this:
 ///   %C = or %A, %B
 ///   %D = select %cond, %C, %A
if (C == A || C == B) {
     // MIN(MAX(a, b), a) -> a
     if ((SPF1 == SPF_SMIN && SPF2 == SPF_SMAX) ||
         (SPF1 == SPF_SMAX && SPF2 == SPF_SMIN) ||
         (SPF1 == SPF_UMIN && SPF2 == SPF_UMAX) ||
         (SPF1 == SPF_UMAX && SPF2 == SPF_UMIN))
       return ReplaceInstUsesWith(Outer, C);
   }
 
-  // TODO: MIN(MIN(A, 23), 97)
+  // MIN(MIN(A, 23), 97) -> MIN(A, 23)
+  // MAX(MAX(A, 97), 23) -> MAX(A, 97)
+  if (SPF1 == SPF2 && isa<Constant>(B) && isa<Constant>(C)) {
+    unsigned short Pred;
+    switch (SPF1) {
+    default:       Pred = ICmpInst::BAD_ICMP_PREDICATE;
+    case SPF_SMIN: Pred = ICmpInst::ICMP_SLE; break;
+    case SPF_UMIN: Pred = ICmpInst::ICMP_ULE; break;
+    case SPF_SMAX: Pred = ICmpInst::ICMP_SGE; break;
+    case SPF_UMAX: Pred = ICmpInst::ICMP_UGE; break;
+    }
+
+    if (Pred != ICmpInst::BAD_ICMP_PREDICATE) {
+      Constant *CB = cast<Constant>(B);
+      Constant *CC = cast<Constant>(C);
+      Constant *IsCPredB = ConstantExpr::getICmp(Pred, CC, CB);
+      if (IsCPredB && IsCPredB->isZeroValue()) {
+        return ReplaceInstUsesWith(Outer, Inner);
+      }
+    }
+  }
   return nullptr;
 }
 
-
 /// foldSelectICmpAnd - If one of the constants is zero (we know they can't
 /// both be) and we have an icmp instruction with zero, and we have an 'and'
 /// with the non-constant value and a power of two we can turn the select
 /// into a shift on the result of the 'and'.
 static Value *foldSelectICmpAnd(const SelectInst &SI, ConstantInt *TrueVal,
                                 ConstantInt *FalseVal,
                                 InstCombiner::BuilderTy *Builder) {
   const ICmpInst *IC = dyn_cast<ICmpInst>(SI.getCondition());

test/Transforms/InstCombine/select.ll

define i32 @test67(i16 %x) {
   ret i32 %3
 
 ; CHECK-LABEL: @test67(
 ; CHECK: and i16 %x, 4
 ; CHECK: zext i16 %1 to i32
 ; CHECK: lshr exact i32 %2, 1
 ; CHECK: xor i32 %3, 42
 }
+
+; SMIN(SMIN(X, 11), 92) -> SMIN(X, 11)
+define i32 @test68(i32 %x) {
+entry:
+  %cmp = icmp slt i32 11, %x
+  %cond = select i1 %cmp, i32 11, i32 %x
+  %cmp3 = icmp slt i32 92, %cond
+  %retval = select i1 %cmp3, i32 92, i32 %cond
+  ret i32 %retval
+; CHECK-LABEL: @test68(
+; CHECK: ret i32 %cond
+}
+
+; MIN(MIN(X, 24), 83) -> MIN(X, 24)
+define i32 @test69(i32 %x) {
+entry:
+  %cmp = icmp ult i32 24, %x
+  %cond = select i1 %cmp, i32 24, i32 %x
+  %cmp3 = icmp ult i32 83, %cond
+  %retval = select i1 %cmp3, i32 83, i32 %cond
+  ret i32 %retval
+; CHECK-LABEL: @test69(
+; CHECK: ret i32 %cond
+}
+
+; SMAX(SMAX(X, 75), 36) -> SMAX(X, 75)
+define i32 @test70(i32 %x) {
+entry:
+  %cmp = icmp slt i32 %x, 75
+  %cond = select i1 %cmp, i32 75, i32 %x
+  %cmp3 = icmp slt i32 %cond, 36
+  %retval = select i1 %cmp3, i32 36, i32 %cond
+  ret i32 %retval
+; CHECK-LABEL: @test70(
+; CHECK: ret i32 %cond
+}
+
+; MAX(MAX(X, 68), 47) -> MAX(X, 68)
+define i32 @test71(i32 %x) {
+entry:
+  %cmp = icmp ult i32 %x, 68
+  %cond = select i1 %cmp, i32 68, i32 %x
+  %cmp3 = icmp ult i32 %cond, 47
+  %retval = select i1 %cmp3, i32 47, i32 %cond
+  ret i32 %retval
+; CHECK-LABEL: @test71(
+; CHECK: ret i32 %cond
+}
 No newline at end of file