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

Igor Breger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 14 08:07:23 PST 2019


igorb added a comment.

In D68328#1743867 <https://reviews.llvm.org/D68328#1743867>, @Joe wrote:

> Updated diff:
>
> I am quite new to SCEV, but it really looks like it just shouldn't work on pointers sometimes. I've fixed it such that it works with custom dl on all tests, so hopefully this is okay.
>
> Also fixed a few more places in which idx and ptr size are assumed to be the same.
>
> Added tests which touch as many places I could manage where I changed ptr type to idx type. Many of the failures were caused due to the ptrtoint.
>
> > 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());
>
> Surely this should stay as IndexType?


Without the change it fail in computeKnownBits on assert(ExpectedWidth == BitWidth && "V and Known should have same BitWidth");
And we should check pointer alignment.

I think emitPointerArithmetic (clang) should be updated also

  diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
  index 105e937ca5b..01eb1a1f76e 100644
  --- a/lib/CodeGen/CGExprScalar.cpp
  +++ b/lib/CodeGen/CGExprScalar.cpp
  @@ -3194,10 +3194,10 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
                                                          expr->getRHS()))
       return CGF.Builder.CreateIntToPtr(index, pointer->getType());
   
  -  if (width != DL.getTypeSizeInBits(PtrTy)) {
  +  if (width != DL.getIndexTypeSizeInBits(PtrTy)) {
       // Zero-extend or sign-extend the pointer value according to
       // whether the index is signed or not.
  -    index = CGF.Builder.CreateIntCast(index, DL.getIntPtrType(PtrTy), isSigned,
  +    index = CGF.Builder.CreateIntCast(index, DL.getIndexType(PtrTy), isSigned,
                                         "idx.ext");
     }

Thanks


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

https://reviews.llvm.org/D68328





More information about the llvm-commits mailing list