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?

661

please clang-format the patch.

667

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

670

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.

661

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.

667

updated

670

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)
 
   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)
-  return nullptr;
+  // MIN(MIN(A, 23), 97) -> MIN(A, 23)
+  // MAX(MAX(A, 97), 23) -> MAX(A, 97)
+  if (SPF1 == SPF2) {
+    if (ConstantInt *CB = dyn_cast<ConstantInt>(B)) {
+      if (ConstantInt *CC = dyn_cast<ConstantInt>(C)) {
+        APInt ACB = CB->getValue();
+        APInt ACC = CC->getValue();
+        if ((SPF1 == SPF_UMIN && ACB.ule(ACC)) ||
+            (SPF1 == SPF_SMIN && ACB.sle(ACC)) ||
+            (SPF1 == SPF_UMAX && ACB.uge(ACC)) ||
+            (SPF1 == SPF_SMAX && ACB.sge(ACC)))
+          return ReplaceInstUsesWith(Outer, Inner);
+      }
+    }
   }
 
+  // TODO: MIN(MIN(A, 97), 23) -> MIN(A, 23)
+  // TODO: MAX(MAX(A, 23), 97) -> MAX(A, 97)
+  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) {

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