[llvm-commits] [llvm] r150962 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineCompares.cpp test/Transforms/InstCombine/icmp.ll

[Benjamin Kramer]

On 20.02.2012, at 19:11, Duncan Sands wrote:

> Hi Benjamin,
> 
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Mon Feb 20 09:07:47 2012
>> @@ -607,6 +607,20 @@
>>          return new ICmpInst(ICmpInst::getSignedPredicate(Cond),
>>                              GEPLHS->getOperand(0), GEPRHS->getOperand(0));
>> 
>> +      // If we're comparing GEPs with two base pointers that only differ in type
>> +      // and both GEPs have only constant indices or just one use, then fold
>> +      // the compare with the adjusted indices.
>> +      if (TD&&
>> +          (GEPLHS->hasAllConstantIndices() || GEPLHS->hasOneUse())&&
>> +          (GEPRHS->hasAllConstantIndices() || GEPRHS->hasOneUse())&&
>> +          PtrBase->stripPointerCasts() ==
>> +            GEPRHS->getOperand(0)->stripPointerCasts()) {
>> +        Value *Cmp = Builder->CreateICmp(ICmpInst::getSignedPredicate(Cond),
>> +                                         EmitGEPOffset(GEPLHS),
>> +                                         EmitGEPOffset(GEPRHS));
> 
> for non-equality comparisons, isn't this only safe if the GEP is inbounds?
> Otherwise, by removing the base, you may change an overflowing (non-overflowing)
> GEP into a non-overflowing (overflowing) one, which can completely change the
> result of the comparison.

Right. For some reason I was thinking that this was checked already before this transform, but looking again shows that's not the case. Fixed in r150978!

- Ben
> 
> Ciao, Duncan.
> 
>> +        return ReplaceInstUsesWith(I, Cmp);
>> +      }
>> +
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits