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

[Chris Lattner]

Thanks for the review!

-Chris

On Nov 21, 2010, at 9:04 AM, Duncan Sands <baldrick at free.fr> wrote:

> Hi Chris, actually I think now that the transform is correct in all cases.
> Sorry for the noise,
> 
> Duncan.
> 
>>> --- 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.
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits