[llvm-commits] [llvm] r91811 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/icmp.ll

[Chris Lattner]

On Jan 8, 2010, at 12:47 AM, Duncan Sands wrote:

> Hi Chris,
>
>> +  // From this point on, we know that (X+C <= X) --> (X+C < X)  
>> because C != 0,
>> +  // so the values can never be equal.  Similiarly for all other  
>> "or equals"
>> +  // operators.
>> +
>> +  // (X+1) <u X        --> X >u (MAXUINT-1)        --> X != 255
>
> X != 255 should be X == 255

Nice catch!

>> +  if (Pred == ICmpInst::ICMP_ULT || Pred == ICmpInst::ICMP_ULE) {
>> +    Value *R = ConstantExpr::getSub(ConstantInt::get(CI- 
>> >getType(), -1ULL), CI);
>
> Shouldn't this use ConstantInt::getAllOnesValue not -1ULL, in case  
> these are
> very wide integers?

They are both equivalent because the -1 gets sign extended.   
getAllOnesValue is more "intentional" though, so I switched.

> As a meta point, if you have the NSW/NUW flags set, you can avoid  
> worrying about
> the overflow cases.

Yep, very true!

-Chris