[llvm-commits] [llvm] r119922 - in /llvm/trunk: lib/Target/README.txt lib/Transforms/InstCombine/InstCombineCompares.cpp test/Transforms/InstCombine/icmp.ll

[Duncan Sands]

Hi Chris,

> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Sun Nov 21 00:44:42 2010
> @@ -1744,14 +1744,84 @@
>       // simplify this comparison.  For example, (x&4)<  8  is always true.
>       switch (I.getPredicate()) {
>       default: llvm_unreachable("Unknown icmp opcode!");
> -    case ICmpInst::ICMP_EQ:
> +    case ICmpInst::ICMP_EQ: {
>         if (Op0Max.ult(Op1Min) || Op0Min.ugt(Op1Max))
>           return ReplaceInstUsesWith(I, ConstantInt::getFalse(I.getContext()));
> +
> +      // If all bits are known zero except for one, then we know at most one
> +      // bit is set.   If the comparison is against zero, then this is a check
> +      // to see if *that* bit is set.
> +      APInt Op0KnownZeroInverted = ~Op0KnownZero;

If Op0KnownZeroInverted has only one bit set, then I don't think you know that
that bit is actually set in the original value, you only know that you were
unable to prove that the bit was zero.  If the bit is zero rather than one, then
isn't the second transform below potentially wrong (the first one looks ok):

> +        // If the LHS is 8>>u x, and we know the result is a power of 2 like 1,
> +        // then turn "((8>>u x)&1) == 0" into "x == 3".
> +        ConstantInt *CI = 0;
> +        if (Op0KnownZeroInverted == 1&&
> +            match(LHS, m_LShr(m_ConstantInt(CI), m_Value(X)))&&
> +            CI->getValue().isPowerOf2()) {
> +          unsigned CmpVal = CI->getValue().countTrailingZeros();
> +          return new ICmpInst(ICmpInst::ICMP_EQ, X,
> +                              ConstantInt::get(X->getType(), CmpVal));
> +        }
> +      }

^ This one, if in fact all bits are zero but it was only possible to prove that
all bits except bit 0 are zero.

Ciao,

Duncan.