[InstCombine] Canonicalize clamp of float types to minmax in fast mode.

[a.elovikov]

Summary

This commit allows matchSelectPattern to recognize clamp of float
arguments in the presence of FMF the same way as already done for
integers.

This case is a little different though. With integers, given the
min/max pattern is recognized, DAGBuilder starts selecting MIN/MAX
"automatically". That is not the case for float, because for them only
full FMINNAN/FMINNUM/FMAXNAN/FMAXNUM ISD nodes exist and they do care
about NaNs. On the other hand, some backends (e.g. X86) have only
FMIN/FMAX nodes that do not care about NaNS and the former NAN/NUM
nodes are illegal thus selection is not happening. So I decided to do
such kind of transformation in IR (InstCombiner) instead of
complicating the logic in the backend.

[jmolloy]

Hi,

On the other hand, some backends (e.g. X86) have only

FMIN/FMAX nodes that do not care about NaNS and the former NAN/NUM
nodes are illegal thus selection is not happening.

For my own reference so I can more effectively review this, could you please explain this a bit more? What is the defined behaviour for such instructions when given a NaN?

[a.elovikov]

In D33186#754812, @jmolloy wrote:

Hi,

On the other hand, some backends (e.g. X86) have only

FMIN/FMAX nodes that do not care about NaNS and the former NAN/NUM
nodes are illegal thus selection is not happening.

For my own reference so I can more effectively review this, could you please explain this a bit more? What is the defined behaviour for such instructions when given a NaN?

I was referring to the following comment from the X86ISelLowering.cpp (combineFMinNumFMaxNum routine):

// There are 4 possibilities involving NaN inputs, and these are the required
// outputs:
//                   Op1
//               Num     NaN
//            ----------------
//       Num  |  Max  |  Op0 |
// Op0        ----------------
//       NaN  |  Op1  |  NaN |
//            ----------------
//
// The SSE FP max/min instructions were not designed for this case, but rather
// to implement:
//   Min = Op1 < Op0 ? Op1 : Op0
//   Max = Op1 > Op0 ? Op1 : Op0
//
// So they always return Op0 if either input is a NaN. However, we can still
// use those instructions for fmaxnum by selecting away a NaN input.

[jmolloy]

Right; that's X86's strange not-a-maxnum-not-a-maxnan behaviour where it just selects the zeroth operand.

You should be able to select that for either of FMAXNAN or FMAXNUM though when the "nnan" flag is set on the instruction, which should be the case in fast-math mode?

[a.elovikov]

In D33186#754827, @jmolloy wrote:

Right; that's X86's strange not-a-maxnum-not-a-maxnan behaviour where it just selects the zeroth operand.

You should be able to select that for either of FMAXNAN or FMAXNUM though when the "nnan" flag is set on the instruction, which should be the case in fast-math mode?

Yes, but they're not selected at all in SelectionDAGBuilder::visitSelect because ISD::FMINNAN/FMINNUM are not isOperationLegalOrCustom for X86 target.
And stating that they are would be incorrect. So the only way to get X86::FMIN/FMAX would be to fix combineSelect in X86ISelLowering.cpp in the same way that I've done for IR. I believe that such manipulation is better be done on IR than on SDNodes

[a.elovikov]

ping.

[jmolloy]

Hi,

And stating that they are would be incorrect.

Why wouldn't you state that they are custom lowered, and copy the generic bailout code from SelectionDAGBuilder.cpp if "nnan" isn't set?

James

[a.elovikov]

In D33186#760622, @jmolloy wrote:

Why wouldn't you state that they are custom lowered, and copy the generic bailout code from SelectionDAGBuilder.cpp if "nnan" isn't set?

If I understand correctly, making that "custom" would have negative impact in the presence of nans because cmp/select sequence would be turned to min/max+cmp(for nans)+select. And I have not seen the cases where lowering is set to custom due to global fast math flags. Is that really an option?

One option can be to check for nnan flags in *both* SelectionDagBuilder and combineSelect but that would add an implicit dependency between them which does not look right.

[a.elovikov]

Ping.

[a.elovikov]

Ping.

[a.elovikov]

ping?..

[spatel]

If there are no objections, I would add the regression tests with the current output as a preliminary step, so we document the current behavior.

@jmolloy - do you plan to continue the review of this patch?

[spatel]

Adding more potential reviewers.

[spatel]

@a.elovikov - I didn't find your name in bugzilla; you might be interested in:
https://bugs.llvm.org/show_bug.cgi?id=33467

[a.elovikov]

Re-base onto D34350 where the current status of the tests was captured.

That did not work as intended, sorry for the noise. :(

[a.elovikov]

Now, properly re-base onto D34350 where the current status of the tests was captured.

[efriedma]

llvm/lib/Analysis/ValueTracking.cpp

3948 ↗

(On Diff #103051)

Missing close-paren? Also, MaxMin and MinMax are a little confusing; just write out one, I think

3951 ↗

(On Diff #103051)

"inversed" isn't a word.

3957 ↗

(On Diff #103051)

"Assume success" is a little confusing... maybe something more like "Return the LHS and RHS early".

4206 ↗

(On Diff #103051)

Need an explanation here describing why signed zeros matter. (Something about the sign of the result?)

llvm/test/Transforms/InstCombine/clamp-to-minmax.ll

146 ↗

(On Diff #103051)

"dit"?

[a.elovikov]

• Update comments as requested in the review.

[a.elovikov]

llvm/lib/Analysis/ValueTracking.cpp

3957 ↗

(On Diff #103051)

This is copy-paste from the integer case in function matchMinMax below. Do you want me to change both places? Also, in the integer case the intent was pretty clear for me with no confusion.

[a.elovikov]

Properly update keeping the dependence on D34350.

[efriedma]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

1329 ↗

(On Diff #103921)

This change isn't guarded for floating-point operations in particular... does this have an impact on integer clamp? If not, why?

[a.elovikov]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

1329 ↗

(On Diff #103921)

Yes, I missed that. This does have impact on integer clamp but I think it won't matter for end-to-end compilation because before this change integer cases were already handled during ISel. Anyway, there are two options here:

• Limit the transformation for floating-point operations only, like this
• Leave the code as is and modify title/summary + add tests to document the change in the behavior for integers.

I would prefer to go with the first approach and do the change for integers in a separate review, if necessary. Unless someone has a preference for the second approach, of course.

[efriedma]

I'm okay with limiting it to FP vectors for now.

[a.elovikov]

In D33186#792461, @efriedma wrote:

I'm okay with limiting it to FP vectors for now.

I believe for scalars it can be beneficial too, similar to cmov vs. branch.

[efriedma]

Err, sorry, typo. Yes, it's fine to limit it to FP scalars and vectors.

[a.elovikov]

Limit clamp->min/max transformation to float types only.

[efriedma]

LGTM.

[a.elovikov]

• Fix UBSan error after D33186/r306525.

[a.elovikov]

Hi @efriedma, original commit caused the failure under UBSan. The latest uploaded revision fixes it.
Can you please take a look and say if it's ok?

Thanks,
Andrei

[efriedma]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

1326 ↗

(On Diff #105035)

This is very suspicious.

The way the implementation of matchSelectPattern is written, CastOp is uninitialized if the type of the compare matches the type of the select; otherwise, it's set to whatever cast we looked through. That cast might not be a cast which changes the size of the type; it could bit a BitCast/FPToUI/etc.

I'd like to see a few testcases which cover the situations where we insert casts.

[a.elovikov]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

1326 ↗

(On Diff #105035)

Yes, this exactly the case causing UB - passing uninitialized object by value to createCast Corresponding argument is not even used in that function because of the early return when the types are equal.

I believe the case with cast should be already covered by earlier tests as I did not touch that part - will try to find them tomorrow, or add new ones if they're missing.

[a.elovikov]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

1326 ↗

(On Diff #105035)

One more note - neither Aslan nor MSan catches this, only UBSan because the value of the uninitialized argument to createCast is not used.

[efriedma]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

1326 ↗

(On Diff #105035)

I believe the case with cast should be already covered by earlier tests as I did not touch that part - will try to find them tomorrow, or add new ones if they're missing.

I'm specifically concerned about cases where transforming a floating-point clamp requires inserting a cast.

[a.elovikov]

Rebase onto D35002.

[a.elovikov]

Compare types instead of their sizes to decide if CastInst is needed.

Note, that casts of "inner" min/max prohibit clamp pattern recognition
because the casts are being looked by one level of use only, and to
get the original value being clamped two levels of look through are
required.

[efriedma]

I'd like to see a testcase where CastOp is fptosi.

[a.elovikov]

Rebased to updated D35002 where fptosi tests were added. That also included re-base to the current master.
No changes in the instcombine results for these added tests though.

[efriedma]

LGTM

llvm/trunk/include/llvm/IR/PatternMatch.h

if (V->getType()->isVectorTy())
       if (const auto *C = dyn_cast<Constant>(V))
         if (auto *CI = dyn_cast_or_null<ConstantInt>(C->getSplatValue())) {
           Res = &CI->getValue();
           return true;
         }
     return false;
   }
 };
+// Either constexpr if or renaming ConstantFP::getValueAPF to
+// ConstantFP::getValue is needed to do it via single template
+// function for both apint/apfloat.
+struct apfloat_match {
+  const APFloat *&Res;
+  apfloat_match(const APFloat *&R) : Res(R) {}
+  template <typename ITy> bool match(ITy *V) {
+    if (auto *CI = dyn_cast<ConstantFP>(V)) {
+      Res = &CI->getValueAPF();
+      return true;
+    }
+    if (V->getType()->isVectorTy())
+      if (const auto *C = dyn_cast<Constant>(V))
+        if (auto *CI = dyn_cast_or_null<ConstantFP>(C->getSplatValue())) {
+          Res = &CI->getValueAPF();
+          return true;
+        }
+    return false;
+  }
+};
 
 /// \brief Match a ConstantInt or splatted ConstantVector, binding the
 /// specified pointer to the contained APInt.
 inline apint_match m_APInt(const APInt *&Res) { return Res; }
 
+/// \brief Match a ConstantFP or splatted ConstantVector, binding the
+/// specified pointer to the contained APFloat.
+inline apfloat_match m_APFloat(const APFloat *&Res) { return Res; }
+
 template <int64_t Val> struct constantint_match {
   template <typename ITy> bool match(ITy *V) {
     if (const auto *CI = dyn_cast<ConstantInt>(V)) {
       const APInt &CIV = CI->getValue();
       if (Val >= 0)
         return CIV == static_cast<uint64_t>(Val);
       // If Val is negative, and CI is shorter than it, truncate to the right
       // number of bits.  If it is larger, then we have to sign extend.  Just

llvm/trunk/lib/Analysis/ValueTracking.cpp

 }
 
 static bool isKnownNonZero(const Value *V) {
   if (auto *C = dyn_cast<ConstantFP>(V))
     return !C->isZero();
   return false;
 }
 
+/// Match clamp pattern for float types without care about NaNs or signed zeros.
+/// Given non-min/max outer cmp/select from the clamp pattern this
+/// function recognizes if it can be substitued by a "canonical" min/max
+/// pattern.
+static SelectPatternResult matchFastFloatClamp(CmpInst::Predicate Pred,
+                                               Value *CmpLHS, Value *CmpRHS,
+                                               Value *TrueVal, Value *FalseVal,
+                                               Value *&LHS, Value *&RHS) {
+  // Try to match
+  //   X < C1 ? C1 : Min(X, C2) --> Max(C1, Min(X, C2))
+  //   X > C1 ? C1 : Max(X, C2) --> Min(C1, Max(X, C2))
+  // and return description of the outer Max/Min.
+
+  // First, check if select has inverse order:
+  if (CmpRHS == FalseVal) {
+    std::swap(TrueVal, FalseVal);
+    Pred = CmpInst::getInversePredicate(Pred);
+  }
+
+  // Assume success now. If there's no match, callers should not use these anyway.
+  LHS = TrueVal;
+  RHS = FalseVal;
+
+  const APFloat *FC1;
+  if (CmpRHS != TrueVal || !match(CmpRHS, m_APFloat(FC1)) || !FC1->isFinite())
+    return {SPF_UNKNOWN, SPNB_NA, false};
+
+  const APFloat *FC2;
+  switch (Pred) {
+  case CmpInst::FCMP_OLT:
+  case CmpInst::FCMP_OLE:
+  case CmpInst::FCMP_ULT:
+  case CmpInst::FCMP_ULE:
+    if (match(FalseVal,
+              m_CombineOr(m_OrdFMin(m_Specific(CmpLHS), m_APFloat(FC2)),
+                          m_UnordFMin(m_Specific(CmpLHS), m_APFloat(FC2)))) &&
+        FC1->compare(*FC2) == APFloat::cmpResult::cmpLessThan)
+      return {SPF_FMAXNUM, SPNB_RETURNS_ANY, false};
+    break;
+  case CmpInst::FCMP_OGT:
+  case CmpInst::FCMP_OGE:
+  case CmpInst::FCMP_UGT:
+  case CmpInst::FCMP_UGE:
+    if (match(FalseVal,
+              m_CombineOr(m_OrdFMax(m_Specific(CmpLHS), m_APFloat(FC2)),
+                          m_UnordFMax(m_Specific(CmpLHS), m_APFloat(FC2)))) &&
+        FC1->compare(*FC2) == APFloat::cmpResult::cmpGreaterThan)
+      return {SPF_FMINNUM, SPNB_RETURNS_ANY, false};
+    break;
+  default:
+    break;
+  }
+
+  return {SPF_UNKNOWN, SPNB_NA, false};
+}
+
 /// Match non-obvious integer minimum and maximum sequences.
 static SelectPatternResult matchMinMax(CmpInst::Predicate Pred,
                                        Value *CmpLHS, Value *CmpRHS,
                                        Value *TrueVal, Value *FalseVal,
                                        Value *&LHS, Value *&RHS) {
   // Assume success. If there's no match, callers should not use these anyway.
   LHS = TrueVal;
   RHS = FalseVal;
if ((CmpLHS == TrueVal && match(FalseVal, m_Neg(m_Specific(CmpLHS)))) ||
       // ABS(X) ==> (X <s 0) ? -X : X and (X <s 1) ? -X : X
       // NABS(X) ==> (X <s 0) ? X : -X and (X <s 1) ? X : -X
       if (Pred == ICmpInst::ICMP_SLT && (*C1 == 0 || *C1 == 1)) {
         return {(CmpLHS == FalseVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};
       }
     }
   }
 
+  if (CmpInst::isIntPredicate(Pred))
     return matchMinMax(Pred, CmpLHS, CmpRHS, TrueVal, FalseVal, LHS, RHS);
+
+  // According to (IEEE 754-2008 5.3.1), minNum(0.0, -0.0) and similar
+  // may return either -0.0 or 0.0, so fcmp/select pair has stricter
+  // semantics than minNum. Be conservative in such case.
+  if (NaNBehavior != SPNB_RETURNS_ANY ||
+      (!FMF.noSignedZeros() && !isKnownNonZero(CmpLHS) &&
+       !isKnownNonZero(CmpRHS)))
+    return {SPF_UNKNOWN, SPNB_NA, false};
+
+  return matchFastFloatClamp(Pred, CmpLHS, CmpRHS, TrueVal, FalseVal, LHS, RHS);
 }
 
 static Value *lookThroughCast(CmpInst *CmpI, Value *V1, Value *V2,
                               Instruction::CastOps *CastOp) {
   auto *Cast1 = dyn_cast<CastInst>(V1);
   if (!Cast1)
     return nullptr;
 

llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp

if (Instruction *FoldI = foldSelectIntoOp(SI, TrueVal, FalseVal))
       return FoldI;
 
     Value *LHS, *RHS, *LHS2, *RHS2;
     Instruction::CastOps CastOp;
     SelectPatternResult SPR = matchSelectPattern(&SI, LHS, RHS, &CastOp);
     auto SPF = SPR.Flavor;
 
     if (SelectPatternResult::isMinOrMax(SPF)) {
-      // Canonicalize so that type casts are outside select patterns.
-      if (LHS->getType()->getPrimitiveSizeInBits() !=
-          SelType->getPrimitiveSizeInBits()) {
+      // Canonicalize so that
+      // - type casts are outside select patterns.
+      // - float clamp is transformed to min/max pattern
+
+      bool IsCastNeeded = LHS->getType() != SelType;
+      Value *CmpLHS = cast<CmpInst>(CondVal)->getOperand(0);
+      Value *CmpRHS = cast<CmpInst>(CondVal)->getOperand(1);
+      if (IsCastNeeded ||
+          (LHS->getType()->isFPOrFPVectorTy() &&
+           ((CmpLHS != LHS && CmpLHS != RHS) ||
+            (CmpRHS != LHS && CmpRHS != RHS)))) {
         CmpInst::Predicate Pred = getCmpPredicateForMinMax(SPF, SPR.Ordered);
 
         Value *Cmp;
         if (CmpInst::isIntPredicate(Pred)) {
           Cmp = Builder.CreateICmp(Pred, LHS, RHS);
         } else {
           IRBuilder<>::FastMathFlagGuard FMFG(Builder);
           auto FMF = cast<FPMathOperator>(SI.getCondition())->getFastMathFlags();
           Builder.setFastMathFlags(FMF);
           Cmp = Builder.CreateFCmp(Pred, LHS, RHS);
         }
 
-        Value *NewSI = Builder.CreateCast(
-            CastOp, Builder.CreateSelect(Cmp, LHS, RHS, SI.getName(), &SI),
-            SelType);
+        Value *NewSI = Builder.CreateSelect(Cmp, LHS, RHS, SI.getName(), &SI);
+        if (!IsCastNeeded)
           return replaceInstUsesWith(SI, NewSI);
+
+        Value *NewCast = Builder.CreateCast(CastOp, NewSI, SelType);
+        return replaceInstUsesWith(SI, NewCast);
       }
     }
 
     if (SPF) {
       // MAX(MAX(a, b), a) -> MAX(a, b)
       // MIN(MIN(a, b), a) -> MIN(a, b)
       // MAX(MIN(a, b), a) -> a
       // MIN(MAX(a, b), a) -> a

llvm/trunk/test/Transforms/InstCombine/clamp-to-minmax.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 ; (X < C1) ? C1 : MIN(X, C2)
 define float @clamp_float_fast_ordered_strict_maxmin(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_ordered_strict_maxmin(
 ; CHECK-NEXT:    [[CMP2:%.*]] = fcmp fast olt float [[X:%.*]], 2.550000e+02
 ; CHECK-NEXT:    [[MIN:%.*]] = select i1 [[CMP2]], float [[X]], float 2.550000e+02
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast olt float [[X]], 1.000000e+00
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 1.000000e+00, float [[MIN]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast oge float [[MIN]], 1.000000e+00
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MIN]], float 1.000000e+00
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast olt float %x, 255.0
   %min = select i1 %cmp2, float %x, float 255.0
   %cmp1 = fcmp fast olt float %x, 1.0
   %r = select i1 %cmp1, float 1.0, float %min
   ret float %r
 }
 
 ; (X <= C1) ? C1 : MIN(X, C2)
 define float @clamp_float_fast_ordered_nonstrict_maxmin(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_ordered_nonstrict_maxmin(
 ; CHECK-NEXT:    [[CMP2:%.*]] = fcmp fast olt float [[X:%.*]], 2.550000e+02
 ; CHECK-NEXT:    [[MIN:%.*]] = select i1 [[CMP2]], float [[X]], float 2.550000e+02
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast ole float [[X]], 1.000000e+00
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 1.000000e+00, float [[MIN]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast oge float [[MIN]], 1.000000e+00
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MIN]], float 1.000000e+00
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast olt float %x, 255.0
   %min = select i1 %cmp2, float %x, float 255.0
   %cmp1 = fcmp fast ole float %x, 1.0
   %r = select i1 %cmp1, float 1.0, float %min
   ret float %r
 }
 
 ; (X > C1) ? C1 : MAX(X, C2)
 define float @clamp_float_fast_ordered_strict_minmax(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_ordered_strict_minmax(
 ; CHECK-NEXT:    [[CMP2:%.*]] = fcmp fast ogt float [[X:%.*]], 1.000000e+00
 ; CHECK-NEXT:    [[MAX:%.*]] = select i1 [[CMP2]], float [[X]], float 1.000000e+00
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast ogt float [[X]], 2.550000e+02
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 2.550000e+02, float [[MAX]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast ole float [[MAX]], 2.550000e+02
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MAX]], float 2.550000e+02
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast ogt float %x, 1.0
   %max = select i1 %cmp2, float %x, float 1.0
   %cmp1 = fcmp fast ogt float %x, 255.0
   %r = select i1 %cmp1, float 255.0, float %max
   ret float %r
 }
 
 ; (X >= C1) ? C1 : MAX(X, C2)
 define float @clamp_float_fast_ordered_nonstrict_minmax(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_ordered_nonstrict_minmax(
 ; CHECK-NEXT:    [[CMP2:%.*]] = fcmp fast ogt float [[X:%.*]], 1.000000e+00
 ; CHECK-NEXT:    [[MAX:%.*]] = select i1 [[CMP2]], float [[X]], float 1.000000e+00
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast oge float [[X]], 2.550000e+02
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 2.550000e+02, float [[MAX]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast ole float [[MAX]], 2.550000e+02
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MAX]], float 2.550000e+02
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast ogt float %x, 1.0
   %max = select i1 %cmp2, float %x, float 1.0
   %cmp1 = fcmp fast oge float %x, 255.0
   %r = select i1 %cmp1, float 255.0, float %max
   ret float %r
 }
 
 
 ; The same for unordered
 
 ; (X < C1) ? C1 : MIN(X, C2)
 define float @clamp_float_fast_unordered_strict_maxmin(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_unordered_strict_maxmin(
 ; CHECK-NEXT:    [[CMP2_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
 ; CHECK-NEXT:    [[MIN:%.*]] = select i1 [[CMP2_INV]], float 2.550000e+02, float [[X]]
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast ult float [[X]], 1.000000e+00
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 1.000000e+00, float [[MIN]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast oge float [[MIN]], 1.000000e+00
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MIN]], float 1.000000e+00
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast ult float %x, 255.0
   %min = select i1 %cmp2, float %x, float 255.0
   %cmp1 = fcmp fast ult float %x, 1.0
   %r = select i1 %cmp1, float 1.0, float %min
   ret float %r
 }
 
 ; (X <= C1) ? C1 : MIN(X, C2)
 define float @clamp_float_fast_unordered_nonstrict_maxmin(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_unordered_nonstrict_maxmin(
 ; CHECK-NEXT:    [[CMP2_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
 ; CHECK-NEXT:    [[MIN:%.*]] = select i1 [[CMP2_INV]], float 2.550000e+02, float [[X]]
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast ule float [[X]], 1.000000e+00
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 1.000000e+00, float [[MIN]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast oge float [[MIN]], 1.000000e+00
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MIN]], float 1.000000e+00
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast ult float %x, 255.0
   %min = select i1 %cmp2, float %x, float 255.0
   %cmp1 = fcmp fast ule float %x, 1.0
   %r = select i1 %cmp1, float 1.0, float %min
   ret float %r
 }
 
 ; (X > C1) ? C1 : MAX(X, C2)
 define float @clamp_float_fast_unordered_strict_minmax(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_unordered_strict_minmax(
 ; CHECK-NEXT:    [[CMP2_INV:%.*]] = fcmp fast ole float [[X:%.*]], 1.000000e+00
 ; CHECK-NEXT:    [[MAX:%.*]] = select i1 [[CMP2_INV]], float 1.000000e+00, float [[X]]
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast ugt float [[X]], 2.550000e+02
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 2.550000e+02, float [[MAX]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast ole float [[MAX]], 2.550000e+02
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MAX]], float 2.550000e+02
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast ugt float %x, 1.0
   %max = select i1 %cmp2, float %x, float 1.0
   %cmp1 = fcmp fast ugt float %x, 255.0
   %r = select i1 %cmp1, float 255.0, float %max
   ret float %r
 }
 
 ; (X >= C1) ? C1 : MAX(X, C2)
 define float @clamp_float_fast_unordered_nonstrict_minmax(float %x) {
 ;
 ; CHECK-LABEL: @clamp_float_fast_unordered_nonstrict_minmax(
 ; CHECK-NEXT:    [[CMP2_INV:%.*]] = fcmp fast ole float [[X:%.*]], 1.000000e+00
 ; CHECK-NEXT:    [[MAX:%.*]] = select i1 [[CMP2_INV]], float 1.000000e+00, float [[X]]
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp fast uge float [[X]], 2.550000e+02
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP1]], float 2.550000e+02, float [[MAX]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast ole float [[MAX]], 2.550000e+02
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[MAX]], float 2.550000e+02
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %cmp2 = fcmp fast ugt float %x, 1.0
   %max = select i1 %cmp2, float %x, float 1.0
   %cmp1 = fcmp fast uge float %x, 255.0
   %r = select i1 %cmp1, float 255.0, float %max
   ret float %r
 }
 
 ; Some more checks with fast
 
 ; (X > 1.0) ? min(x, 255.0) : 1.0
 ; That did not match because select was in inverse order.
 define float @clamp_test_1(float %x) {
 ; CHECK-LABEL: @clamp_test_1(
 ; CHECK-NEXT:    [[INNER_CMP_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
 ; CHECK-NEXT:    [[INNER_SEL:%.*]] = select i1 [[INNER_CMP_INV]], float 2.550000e+02, float [[X]]
-; CHECK-NEXT:    [[OUTER_CMP:%.*]] = fcmp fast ugt float [[X]], 1.000000e+00
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[OUTER_CMP]], float [[INNER_SEL]], float 1.000000e+00
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[DOTINV:%.*]] = fcmp fast oge float [[INNER_SEL]], 1.000000e+00
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[DOTINV]], float [[INNER_SEL]], float 1.000000e+00
+; CHECK-NEXT:    ret float [[R1]]
 ;
   %inner_cmp = fcmp fast ult float %x, 255.0
   %inner_sel = select i1 %inner_cmp, float %x, float 255.0
   %outer_cmp = fcmp fast ugt float %x, 1.0
   %r = select i1 %outer_cmp, float %inner_sel, float 1.0
   ret float %r
 }
 
;
   %cmp1 = fcmp uge float %x, 255.0                  ;             true
   %r = select i1 %cmp1, float 255.0, float %max     ;             255.0
   ret float %r
 }
 
 ;; Check casts behavior
 define float @ui32_clamp_and_cast_to_float(i32 %x) {
 ; CHECK-LABEL: @ui32_clamp_and_cast_to_float(
-; CHECK-NEXT:    [[F_X:%.*]] = uitofp i32 [[X:%.*]] to float
-; CHECK-NEXT:    [[UP_CMP:%.*]] = icmp ugt i32 [[X]], 255
-; CHECK-NEXT:    [[LO_CMP:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:    [[MIN:%.*]] = select i1 [[UP_CMP]], float 2.550000e+02, float [[F_X]]
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[LO_CMP]], float 1.000000e+00, float [[MIN]]
+; CHECK-NEXT:    [[LO_CMP:%.*]] = icmp eq i32 [[X:%.*]], 0
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ult i32 [[X]], 255
+; CHECK-NEXT:    [[MIN1:%.*]] = select i1 [[TMP1]], i32 [[X]], i32 255
+; CHECK-NEXT:    [[TMP2:%.*]] = uitofp i32 [[MIN1]] to float
+; CHECK-NEXT:    [[R:%.*]] = select i1 [[LO_CMP]], float 1.000000e+00, float [[TMP2]]
 ; CHECK-NEXT:    ret float [[R]]
 ;
   %f_x = uitofp i32 %x to float
   %up_cmp = icmp ugt i32 %x, 255
   %lo_cmp = icmp ult i32 %x, 1
   %min = select i1 %up_cmp, float 255.0, float %f_x
   %r = select i1 %lo_cmp, float 1.0, float %min
   ret float %r
;
   %r = select i1 %lo_cmp, float 1.0, float %min
   ret float %r
 }
 
 define float @mixed_clamp_to_float_1(i32 %x) {
 ; CHECK-LABEL: @mixed_clamp_to_float_1(
 ; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i32 [[X:%.*]], 255
 ; CHECK-NEXT:    [[SI_MIN:%.*]] = select i1 [[TMP1]], i32 [[X]], i32 255
-; CHECK-NEXT:    [[F_MIN:%.*]] = sitofp i32 [[SI_MIN]] to float
-; CHECK-NEXT:    [[LO_CMP:%.*]] = icmp slt i32 [[X]], 1
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[LO_CMP]], float 1.000000e+00, float [[F_MIN]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp sgt i32 [[SI_MIN]], 1
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[TMP2]], i32 [[SI_MIN]], i32 1
+; CHECK-NEXT:    [[TMP3:%.*]] = sitofp i32 [[R1]] to float
+; CHECK-NEXT:    ret float [[TMP3]]
 ;
   %si_min_cmp = icmp sgt i32 %x, 255
   %si_min = select i1 %si_min_cmp, i32 255, i32 %x
   %f_min = sitofp i32 %si_min to float
   %f_x = sitofp i32 %x to float
   %lo_cmp = fcmp ult float %f_x, 1.0
   %r = select i1 %lo_cmp, float 1.0, float %f_min
   ret float %r
;
   %r = select i1 %lo_cmp, i32 1, i32 %i32_min
   ret i32 %r
 }
 
 define float @mixed_clamp_to_float_2(i32 %x) {
 ; CHECK-LABEL: @mixed_clamp_to_float_2(
 ; CHECK-NEXT:    [[TMP1:%.*]] = icmp slt i32 [[X:%.*]], 255
 ; CHECK-NEXT:    [[SI_MIN:%.*]] = select i1 [[TMP1]], i32 [[X]], i32 255
-; CHECK-NEXT:    [[F_MIN:%.*]] = sitofp i32 [[SI_MIN]] to float
-; CHECK-NEXT:    [[LO_CMP:%.*]] = icmp slt i32 [[X]], 1
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[LO_CMP]], float 1.000000e+00, float [[F_MIN]]
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp sgt i32 [[SI_MIN]], 1
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[TMP2]], i32 [[SI_MIN]], i32 1
+; CHECK-NEXT:    [[TMP3:%.*]] = sitofp i32 [[R1]] to float
+; CHECK-NEXT:    ret float [[TMP3]]
 ;
   %si_min_cmp = icmp sgt i32 %x, 255
   %si_min = select i1 %si_min_cmp, i32 255, i32 %x
   %f_min = sitofp i32 %si_min to float
   %lo_cmp = icmp slt i32 %x, 1
   %r = select i1 %lo_cmp, float 1.0, float %f_min
   ret float %r
 }

llvm/trunk/test/Transforms/InstCombine/minmax-fold.ll

;
   %1 = icmp sgt i32 %a, -1
   %2 = sext i32 %a to i64
   %3 = select i1 %1, i64 %2, i64 4294967295
   ret i64 %3
 }
 
 define float @t10(i32 %x) {
 ; CHECK-LABEL: @t10(
-; CHECK-NEXT:    [[F_X:%.*]] = sitofp i32 [[X:%.*]] to float
-; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X]], 255
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP]], float [[F_X]], float 2.550000e+02
-; CHECK-NEXT:    ret float [[R]]
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i32 [[X:%.*]], 255
+; CHECK-NEXT:    [[R1:%.*]] = select i1 [[TMP1]], i32 [[X]], i32 255
+; CHECK-NEXT:    [[TMP2:%.*]] = sitofp i32 [[R1]] to float
+; CHECK-NEXT:    ret float [[TMP2]]
 ;
   %f_x = sitofp i32 %x to float
   %cmp = icmp sgt i32 %x, 255
   %r = select i1 %cmp, float %f_x, float 255.0
   ret float %r
 }
 
 define float @t11(i64 %x) {

llvm/trunk/test/Transforms/InstCombine/pr27236.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -instcombine < %s | FileCheck %s
 
 define float @test1(i32 %scale) {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt i32 [[SCALE:%.*]], 1
 ; CHECK-NEXT:    [[TMP2:%.*]] = select i1 [[TMP1]], i32 [[SCALE]], i32 1
 ; CHECK-NEXT:    [[TMP3:%.*]] = sitofp i32 [[TMP2]] to float
-; CHECK-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[TMP2]], 0
-; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[TMP4]], float [[TMP3]], float 0.000000e+00
-; CHECK-NEXT:    ret float [[SEL]]
+; CHECK-NEXT:    ret float [[TMP3]]
 ;
   %1 = icmp sgt i32 1, %scale
   %2 = select i1 %1, i32 1, i32 %scale
   %3 = sitofp i32 %2 to float
   %4 = icmp sgt i32 %2, 0
   %sel = select i1 %4, float %3, float 0.000000e+00
   ret float %sel
 }