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

[Dinesh Dwivedi]

Thanks for review.

================
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())
----------------
Rafael Ávila de Espíndola wrote:
> Why not implement it in here? In any case, this patch is no changing this, right? Maybe just leave the FIXME as is?
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.

================
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;
----------------
Rafael Ávila de Espíndola wrote:
> please clang-format the patch.
> 
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.

================
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);
----------------
Rafael Ávila de Espíndola wrote:
> use a dyn_cast instead of an isa above. That way you don't need to cast in here.
updated

================
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);
----------------
Rafael Ávila de Espíndola wrote:
> 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.
> 
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.

http://reviews.llvm.org/D3629