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

[Rafael Ávila de Espíndola]

Thanks.

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:69
@@ -68,2 +68,3 @@
 
-  // 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())
----------------
Why not implement it in here? In any case, this patch is no changing this, right? Maybe just leave the FIXME as is?

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:662
@@ +661,3 @@
+    case SPF_SMIN: Pred = ICmpInst::ICMP_SLE; break;
+    case SPF_UMIN: Pred = ICmpInst::ICMP_ULE; break;
+    case SPF_SMAX: Pred = ICmpInst::ICMP_SGE; break;
----------------
please clang-format the patch.


================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:668
@@ +667,3 @@
+    if (Pred != ICmpInst::BAD_ICMP_PREDICATE) {
+      Constant *CB = cast<Constant>(B);
+      Constant *CC = cast<Constant>(C);
----------------
use a dyn_cast instead of an isa above. That way you don't need to cast in here.

================
Comment at: lib/Transforms/InstCombine/InstCombineSelect.cpp:671
@@ +670,3 @@
+      Constant *IsCPredB = ConstantExpr::getICmp(Pred, CC, CB);
+      if (IsCPredB && IsCPredB->isZeroValue()) {
+        return ReplaceInstUsesWith(Outer, Inner);
----------------
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.

http://reviews.llvm.org/D3629