[llvm-commits] [llvm] r123034 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineSelect.cpp test/Transforms/InstCombine/select.ll

[Benjamin Kramer]

On 07.01.2011, at 22:33, Tobias Grosser wrote:

> Author: grosser
> Date: Fri Jan  7 15:33:14 2011
> New Revision: 123034
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=123034&view=rev
> Log:
> InstCombine: Match min/max hidden by sext/zext
> 
> X = sext x; x >s c ? X : C+1 --> X = sext x; X <s C+1 ? C+1 : X
> X = sext x; x <s c ? X : C-1 --> X = sext x; X >s C-1 ? C-1 : X
> X = zext x; x >u c ? X : C+1 --> X = zext x; X <u C+1 ? C+1 : X
> X = zext x; x <u c ? X : C-1 --> X = zext x; X >u C-1 ? C-1 : X
> X = sext x; x >u c ? X : C+1 --> X = sext x; X <u C+1 ? C+1 : X
> X = sext x; x <u c ? X : C-1 --> X = sext x; X >u C-1 ? C-1 : X
> 
> Instead of calculating this with mixed types promote all to the
> larger type. This enables scalar evolution to analyze this
> expression. PR8866
> 
> Modified:
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
>    llvm/trunk/test/Transforms/InstCombine/select.ll
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=123034&r1=123033&r2=123034&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Fri Jan  7 15:33:14 2011
> @@ -285,48 +285,83 @@
>   // place here, so make sure the select is the only user.
>   if (ICI->hasOneUse())
>     if (ConstantInt *CI = dyn_cast<ConstantInt>(CmpRHS)) {
> +      // X < MIN ? T : F  -->  F
> +      if ((Pred == ICmpInst::ICMP_SLT || Pred == ICmpInst::ICMP_ULT)
> +          && CI->isMinValue(Pred == ICmpInst::ICMP_SLT))
> +        return ReplaceInstUsesWith(SI, FalseVal);
> +      // X > MAX ? T : F  -->  F
> +      else if ((Pred == ICmpInst::ICMP_SGT || Pred == ICmpInst::ICMP_UGT)
> +               && CI->isMaxValue(Pred == ICmpInst::ICMP_SGT))
> +        return ReplaceInstUsesWith(SI, FalseVal);
>       switch (Pred) {
>       default: break;
>       case ICmpInst::ICMP_ULT:
> -      case ICmpInst::ICMP_SLT: {
> -        // X < MIN ? T : F  -->  F
> -        if (CI->isMinValue(Pred == ICmpInst::ICMP_SLT))
> -          return ReplaceInstUsesWith(SI, FalseVal);
> -        // X < C ? X : C-1  -->  X > C-1 ? C-1 : X
> -        Constant *AdjustedRHS =
> -          ConstantInt::get(CI->getContext(), CI->getValue()-1);
> -        if ((CmpLHS == TrueVal && AdjustedRHS == FalseVal) ||
> -            (CmpLHS == FalseVal && AdjustedRHS == TrueVal)) {
> -          Pred = ICmpInst::getSwappedPredicate(Pred);
> -          CmpRHS = AdjustedRHS;
> -          std::swap(FalseVal, TrueVal);
> -          ICI->setPredicate(Pred);
> -          ICI->setOperand(1, CmpRHS);
> -          SI.setOperand(1, TrueVal);
> -          SI.setOperand(2, FalseVal);
> -          Changed = true;
> -        }
> -        break;
> -      }
> +      case ICmpInst::ICMP_SLT:
>       case ICmpInst::ICMP_UGT:
>       case ICmpInst::ICMP_SGT: {
> -        // X > MAX ? T : F  -->  F
> -        if (CI->isMaxValue(Pred == ICmpInst::ICMP_SGT))
> -          return ReplaceInstUsesWith(SI, FalseVal);
> +        Constant *AdjustedRHS;
> +        if (Pred == ICmpInst::ICMP_UGT || Pred == ICmpInst::ICMP_SGT)
> +          AdjustedRHS = ConstantInt::get(CI->getContext(), CI->getValue() + 1);
> +        else // (Pred == ICmpInst::ICMP_ULT || Pred == ICmpInst::ICMP_SLT)
> +          AdjustedRHS = ConstantInt::get(CI->getContext(), CI->getValue() - 1);
> +
>         // X > C ? X : C+1  -->  X < C+1 ? C+1 : X
> -        Constant *AdjustedRHS =
> -          ConstantInt::get(CI->getContext(), CI->getValue()+1);
> +        // X < C ? X : C-1  -->  X > C-1 ? C-1 : X
>         if ((CmpLHS == TrueVal && AdjustedRHS == FalseVal) ||
> -            (CmpLHS == FalseVal && AdjustedRHS == TrueVal)) {
> -          Pred = ICmpInst::getSwappedPredicate(Pred);
> -          CmpRHS = AdjustedRHS;
> -          std::swap(FalseVal, TrueVal);
> -          ICI->setPredicate(Pred);
> -          ICI->setOperand(1, CmpRHS);
> -          SI.setOperand(1, TrueVal);
> -          SI.setOperand(2, FalseVal);
> -          Changed = true;
> -        }
> +            (CmpLHS == FalseVal && AdjustedRHS == TrueVal))
> +          ; // Nothing to do here. Values match without any sign/zero extension.
> +
> +        // Types do not match. Instead of calculating this with mixed types
> +        // promote all to the larger type. This enables scalar evolution to
> +        // analyze this expression.
> +        else if (CmpRHS->getType()->getScalarSizeInBits()
> +                 < SI.getType()->getScalarSizeInBits()) {
> +          Constant *sextRHS = ConstantExpr::getSExt(AdjustedRHS,
> +                                                    SI.getType());

Hi Tobias,

This line (ConstantExpr::getSExt) hits an assertion failure in some files in the test suite

MultiSource/Applications/JM/lencod/lencod
MultiSource/Applications/oggenc/oggenc
MultiSource/Benchmarks/MiBench/consumer-lame/consumer-lame
SingleSource/Benchmarks/Misc/perlin

The assert is:
  Assertion failed: (Ty->isIntOrIntVectorTy() && "SExt produces only integer"), function getSExt, file Constants.cpp, line 1268.

And the offending IR is:
  %175 = sitofp i32 %j.3 to double
  %174 = icmp slt i32 %j.3, 2
  %176 = select i1 %174, double 2.000000e+00, double %175