[llvm] r273105 - [LoadCombine] Combine Loads formed from GEPS with negative indexes

[David Majnemer via llvm-commits]

On Sun, Jun 19, 2016 at 12:08 AM, Eli Friedman <eli.friedman at gmail.com>
wrote:

> On Sat, Jun 18, 2016 at 11:14 PM, David Majnemer via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: majnemer
>> Date: Sun Jun 19 01:14:56 2016
>> New Revision: 273105
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=273105&view=rev
>> Log:
>> [LoadCombine] Combine Loads formed from GEPS with negative indexes
>>
>> Change the underlying offset and comparisons to use int64_t instead of
>> uint64_t.
>>
>> Patch by River Riddle!
>>
>> Differential Revision: http://reviews.llvm.org/D21499
>>
>> Added:
>>     llvm/trunk/test/Transforms/LoadCombine/load-combine-negativegep.ll
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp?rev=273105&r1=273104&r2=273105&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/LoadCombine.cpp Sun Jun 19 01:14:56
>> 2016
>> @@ -40,7 +40,7 @@ STATISTIC(NumLoadsCombined, "Number of l
>>  namespace {
>>  struct PointerOffsetPair {
>>    Value *Pointer;
>> -  uint64_t Offset;
>> +  int64_t Offset;
>>  };
>>
>>  struct LoadPOPPair {
>> @@ -102,7 +102,7 @@ PointerOffsetPair LoadCombine::getPointe
>>        unsigned BitWidth = DL.getPointerTypeSizeInBits(GEP->getType());
>>        APInt Offset(BitWidth, 0);
>>        if (GEP->accumulateConstantOffset(DL, Offset))
>> -        POP.Offset += Offset.getZExtValue();
>> +        POP.Offset += Offset.getSExtValue();
>>
>
> This line seems a bit suspcious... one, it assumes pointers are never more
> than 64 bits wide, and two, the addition could have signed overflow.
>

Should be done with r273126, however accumulateConstantOffset's internal
logic also needs to check for signed overflow.


>
>
> -Eli
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160619/94883a21/attachment.html>