[PATCH] D38501: [ValueTracking] Fix a misuse of APInt in GetPointerBaseWithConstantOffset

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:3068
+      ByteOffset += GEPOffset.sextOrTrunc(ByteOffset.getBitWidth());
 
       Ptr = GEP->getPointerOperand();
----------------
hfinkel wrote:
> We still need to make sure that the result doesn't overflow. I think something like this would work:
> 
>   APInt OrigByteOffset(ByteOffset);
> 
>   ByteOffset += GEPOffset.sextOrTrunc(ByteOffset.getBitWidth());
>   if (ByteOffset.getActiveBits() >= 64) {
>     ByteOffset = OrigByteOffset;
>     break;
>   }
> 
Why does it matter if the result overflows?  GEP arithmetic is wrapping.


================
Comment at: lib/Analysis/ValueTracking.cpp:3081
   }
   Offset = ByteOffset.getSExtValue();
   return Ptr;
----------------
Yes, you need a check to make sure the value can be represented in 64 bits.  getSExtValue() will cause an assertion failure if it doesn't.

Granted, that's only relevant if you have pointers wider than 64 bits.


https://reviews.llvm.org/D38501