[PATCH] D68328: Fix occurrences that size and range of pointers are assumed to be the same.

[Igor Breger via Phabricator via llvm-commits]

igorb added a comment.

Hi Joseph, 
Thank you very much for looking into this

I think effective SCEV type must be index size and it should be enforced. Without it, indeed many tests fail.  Any pointer arithmetic should fit into index size, in other case target may not be able to handle it.
Something like

  --- a/lib/Analysis/ScalarEvolution.cpp
  +++ b/lib/Analysis/ScalarEvolution.cpp
  @@ -5692,6 +5692,8 @@ ScalarEvolution::getRangeRef(const SCEV *S,
       if (SignHint == ScalarEvolution::HINT_RANGE_UNSIGNED) {
         // For a SCEVUnknown, ask ValueTracking.
         KnownBits Known = computeKnownBits(U->getValue(), DL, 0, &AC, nullptr, &DT);
  +      if (Known.getBitWidth() > BitWidth)
  +        Known = Known.trunc(BitWidth);
         if (Known.One != ~Known.Zero + 1)

I think you also missed

  @@ -9261,7 +9261,7 @@ unsigned SelectionDAG::InferPtrAlignment(SDValue Ptr) const {
     const GlobalValue *GV;
     int64_t GVOffset = 0;
     if (TLI->isGAPlusOffset(Ptr.getNode(), GV, GVOffset)) {
  -    unsigned IdxWidth = getDataLayout().getIndexTypeSizeInBits(GV->getType());
  +    unsigned IdxWidth = getDataLayout().getPointerTypeSizeInBits(GV->getType());
       KnownBits Known(IdxWidth);
       llvm::computeKnownBits(GV, Known, getDataLayout());

With changes above most of the tests pass, but a few still failing . For example  Analysis/ScalarEvolution/implied-via-addition.ll  with target datalayout="p:40:64:64:32"


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68328/new/

https://reviews.llvm.org/D68328