[llvm-commits] RFC: patch for PR1782 (BasicAliasAnalyis)

[Wojciech Matyjewicz]

Chris Lattner wrote:
>> Suppose, we have a target with 32-bit pointers and the following
>> instructions:
>>
>> %p = getelementptr i32* %x, i32 -1
>> %q = getelementptr i32* %x, i32 1073741823  ;(1073741823 == 2^30 - 1)
>>
>> TargetData::getIndexedOffset() uses 64-bit arithmetic to perform  
>> offset
>> computation and return 64-bit values. Hence, it will return -4 as an
>> offset for %p, and 2^32 - 4 for %q. Based on these offsets, it may  
>> seem
>> that %p and %q point to different memory objects. However, they don't,
>> taking into account that pointers are 32-bit long.
> 
> Ok.
> 
>> I guess, such a large positive index in GEP as seen above can be
>> introduced by -instcombine pass.
> 
> Ok.  This is somewhat dubious though, as it is wrapping around the end  
> of the address space which is undefined in C.

I've found one place in instcombine, where something like this can be
introduced. Fixed with this patch:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20071210/056253.html

>> I think, the simplest way to fix it is to truncate the computed offset
>> to the target pointer size before returning it in
>> TargetData::getIndexedOffset(). If you think this is the correct
>> approach, I may prepare a patch.
> 
> Ah, that does make a lot of sense.  Since the fix is simple, please go  
> for it!

I've rethought the issue. If you say that, wrapping around the memory by
getelementptr is undefined, I think it's better to leave it as it is.
This way we'll be able to catch other optimizer bugs, if there are any
of the above kind.

-Wojtek