[llvm-commits] [llvm] r122678 - in /llvm/trunk: lib/Transforms/Scalar/LoopIdiomRecognize.cpp test/Transforms/LoopIdiom/basic.ll

[Chris Lattner]

On Jan 3, 2011, at 3:43 PM, Dan Gohman wrote:

> 
> On Jan 1, 2011, at 7:37 PM, Chris Lattner wrote:
>> 
>> +  // The # stored bytes is (BECount+1)*Size.  Expand the trip count out to
>> +  // pointer size if it isn't already.
>> +  const Type *IntPtr = TD->getIntPtrType(SI->getContext());
>> +  unsigned BESize = SE->getTypeSizeInBits(BECount->getType());
>> +  if (BESize < TD->getPointerSizeInBits())
>> +    BECount = SE->getZeroExtendExpr(BECount, IntPtr);
>> +  else if (BESize > TD->getPointerSizeInBits())
>> +    BECount = SE->getTruncateExpr(BECount, IntPtr);
> 
> This code (and other instances of it) can be simplified using
> ScalarEvolution's getTruncateOrZeroExtend.
> 
> Also, truncating is surprising here, though I guess maybe it's safe
> since any loop where it would change the value would be iterating
> over the entire address space.
> 
>> +  
>> +  const SCEV *NumBytesS = SE->getAddExpr(BECount, SE->getConstant(IntPtr, 1),
>> +                                         true, true /*nooverflow*/);
> 
> Setting nuw here is surprising, but as above, overflow would mean
> the loop is iterating over the entire address space.
> 
> Setting nsw here seems incorrect, as there's no fundamental reason why
> iterating over half of the address space is invalid.

Great points, fixed in r122790, thanks for the review!

-Chris