Index: lib/Transforms/InstCombine/InstCombineCompares.cpp =================================================================== --- lib/Transforms/InstCombine/InstCombineCompares.cpp +++ lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -3970,6 +3970,19 @@ } } + // Test if the FCmpInst instruction is used exclusively by a select as + // part of a minimum or maximum operation. If so, refrain from doing + // any other folding. This helps out other analyses which understand + // non-obfuscated minimum and maximum idioms, such as ScalarEvolution + // and CodeGen. And in this case, at least one of the comparison + // operands has at least one user besides the compare (the select), + // which would often largely negate the benefit of folding anyway. + if (I.hasOneUse()) + if (SelectInst *SI = dyn_cast(*I.user_begin())) + if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) || + (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1)) + return nullptr; + // Handle fcmp with constant RHS if (Constant *RHSC = dyn_cast(Op1)) { if (Instruction *LHSI = dyn_cast(Op0)) Index: lib/Transforms/InstCombine/InstCombineSelect.cpp =================================================================== --- lib/Transforms/InstCombine/InstCombineSelect.cpp +++ lib/Transforms/InstCombine/InstCombineSelect.cpp @@ -1154,18 +1154,38 @@ } // See if we can fold the select into one of our operands. - if (SI.getType()->isIntegerTy()) { + if (SI.getType()->isIntOrIntVectorTy()) { if (Instruction *FoldI = FoldSelectIntoOp(SI, TrueVal, FalseVal)) return FoldI; Value *LHS, *RHS, *LHS2, *RHS2; - SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS); + Instruction::CastOps CastOp; + SelectPatternFlavor SPF = matchSelectPattern(&SI, LHS, RHS, &CastOp); - // 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 if (SPF) { + // Canonicalize so that type casts are outside select patterns, not inside. + if (LHS->getType()->getPrimitiveSizeInBits() != + SI.getType()->getPrimitiveSizeInBits()) { + CmpInst::Predicate Pred; + switch (SPF) { + case SPF_SMIN: Pred = CmpInst::ICMP_SLT; break; + case SPF_SMAX: Pred = CmpInst::ICMP_SGT; break; + case SPF_UMIN: Pred = CmpInst::ICMP_ULT; break; + case SPF_UMAX: Pred = CmpInst::ICMP_UGT; break; + default: llvm_unreachable("Unknown SPF type!"); + } + return CastInst::Create(CastOp, + SelectInst::Create( + CmpInst::Create(Instruction::ICmp, Pred, + LHS, RHS, "", &SI), + LHS, RHS, "", &SI), + SI.getType()); + } + + // 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 if (SelectPatternFlavor SPF2 = matchSelectPattern(LHS, LHS2, RHS2)) if (Instruction *R = FoldSPFofSPF(cast(LHS),SPF2,LHS2,RHS2, SI, SPF, RHS)) Index: lib/Transforms/InstCombine/InstructionCombining.cpp =================================================================== --- lib/Transforms/InstCombine/InstructionCombining.cpp +++ lib/Transforms/InstCombine/InstructionCombining.cpp @@ -714,6 +714,22 @@ return nullptr; } + // Test if a CmpInst instruction is used exclusively by a select as + // part of a minimum or maximum operation. If so, refrain from doing + // any other folding. This helps out other analyses which understand + // non-obfuscated minimum and maximum idioms, such as ScalarEvolution + // and CodeGen. And in this case, at least one of the comparison + // operands has at least one user besides the compare (the select), + // which would often largely negate the benefit of folding anyway. + if (auto *CI = dyn_cast(SI->getCondition())) { + if (CI->hasOneUse()) { + Value *Op0 = CI->getOperand(0), *Op1 = CI->getOperand(1); + if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) || + (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1)) + return nullptr; + } + } + Value *SelectTrueVal = FoldOperationIntoSelectOperand(Op, TV, this); Value *SelectFalseVal = FoldOperationIntoSelectOperand(Op, FV, this); @@ -723,7 +739,6 @@ return nullptr; } - /// FoldOpIntoPhi - Given a binary operator, cast instruction, or select which /// has a PHI node as operand #0, see if we can fold the instruction into the /// PHI (which is only possible if all operands to the PHI are constants). Index: test/Transforms/InstCombine/minmax-fold.ll =================================================================== --- /dev/null +++ test/Transforms/InstCombine/minmax-fold.ll @@ -0,0 +1,73 @@ +; RUN: opt -S -instcombine < %s | FileCheck %s + +; CHECK-LABEL: @t1 +; CHECK-NEXT: icmp +; CHECK-NEXT: select +; CHECK-NEXT: sext +define i64 @t1(i32 %a) { + ; This is the canonical form for a type-changing min/max. + %1 = icmp slt i32 %a, 5 + %2 = select i1 %1, i32 %a, i32 5 + %3 = sext i32 %2 to i64 + ret i64 %3 +} + +; CHECK-LABEL: @t2 +; CHECK-NEXT: icmp +; CHECK-NEXT: select +; CHECK-NEXT: sext +define i64 @t2(i32 %a) { + ; Check this is converted into canonical form, as above. + %1 = icmp slt i32 %a, 5 + %2 = sext i32 %a to i64 + %3 = select i1 %1, i64 %2, i64 5 + ret i64 %3 +} + +; CHECK-LABEL: @t3 +; CHECK-NEXT: icmp +; CHECK-NEXT: select +; CHECK-NEXT: zext +define i64 @t3(i32 %a) { + ; Same as @t2, with flipped operands and zext instead of sext. + %1 = icmp ult i32 %a, 5 + %2 = zext i32 %a to i64 + %3 = select i1 %1, i64 5, i64 %2 + ret i64 %3 +} + +; CHECK-LABEL: @t4 +; CHECK-NEXT: icmp +; CHECK-NEXT: select +; CHECK-NEXT: trunc +define i32 @t4(i64 %a) { + ; Same again, with trunc. + %1 = icmp slt i64 %a, 5 + %2 = trunc i64 %a to i32 + %3 = select i1 %1, i32 %2, i32 5 + ret i32 %3 +} + +; CHECK-LABEL: @t5 +; CHECK-NEXT: icmp +; CHECK-NEXT: zext +; CHECK-NEXT: select +define i64 @t5(i32 %a) { + ; Same as @t3, but with mismatched signedness between icmp and zext. + ; InstCombine should leave this alone. + %1 = icmp slt i32 %a, 5 + %2 = zext i32 %a to i64 + %3 = select i1 %1, i64 5, i64 %2 + ret i64 %3 +} + +; CHECK-LABEL: @t6 +; CHECK-NEXT: icmp +; CHECK-NEXT: select +; CHECK-NEXT: sitofp +define float @t6(i32 %a) { + %1 = icmp slt i32 %a, 0 + %2 = select i1 %1, i32 %a, i32 0 + %3 = sitofp i32 %2 to float + ret float %3 +}