[PATCH] D33129: [SCEVExpander] Try harder to avoid introducing inttoptr

Keno Fischer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 16 13:42:31 PDT 2017


loladiro added inline comments.


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:494
+    Value *Idx = nullptr;
+    if (AddExpr->getType()->isPointerTy() && V->getType()->isIntegerTy()) {
+      // The AddExpr is of pointer type, while V is of integer type, so
----------------
sanjoy wrote:
> Just to be clear, this is intentionally kicking in for all pointers?  If so, a test case showing that will be great.
Yes, I think it's just generally better to attempt to do this rather than create the inttoptr/ptrtoint pairs. I'll add a test case.


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:1396
     assert(S->isAffine() && "Can't linearly scale non-affine recurrences.");
     Result = InsertNoopCastOfTo(Result, IntTy);
     Result = Builder.CreateMul(Result,
----------------
sanjoy wrote:
> This is annoying, but won't a non-null `PostLoopScale` with an ni pointer type fail here?
> 
I think in the cases I saw, `Result` was of integer type at this point, so that would have been fine. It may be possible to get a pointer typed one, but it seems a bit weird to be multiplying something that would be pointer valued at this point (i.e. I can see multiplying an offset, but once you apply a base pointer, seem like you wouldn't be multiplying any more)? 


https://reviews.llvm.org/D33129





More information about the llvm-commits mailing list