[PATCH] D89456: [SCEV] Introduce SCEVPtrToIntExpr (PR46786)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5513
+    ConstantRange X = getRangeRef(PtrToInt->getOperand(), SignHint);
+    return setRange(PtrToInt, SignHint, X.zextOrTrunc(BitWidth));
+  }
----------------
lebedev.ri wrote:
> efriedma wrote:
> > lebedev.ri wrote:
> > > efriedma wrote:
> > > > zextOrTrunc shouldn't be necessary.
> > > Agree, but unfortunately it is, because `ScalarEvolution::getEffectiveSCEVType()` is broken.
> > > ```
> > >   // The only other support type is pointer.
> > >   assert(Ty->isPointerTy() && "Unexpected non-pointer non-integer type!");
> > >   return getDataLayout().getIndexType(Ty);
> > > ```
> > > Index type can be, but is not always, the same type as the pointer-sized integer type.
> > > There's existing test that crashes with assertion without that `zextOrTrunc`.
> > If the index width is narrower than pointer width, the range computed using zextOrTrunc is nonsense: the actual pointer value may not fit into the index type at all.
> > 
> > Off the top of my head, I'm not sure what the canonical IR looks like in cases like that; do we canonicalize the  ptrtoint to the pointer width, or the index width?
> Let me take one more look at what is going on there..
Addressed by D89540.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89456