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

[Keno Fischer via Phabricator via llvm-commits]

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
----------------
loladiro wrote:
> 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.
Hmm, looks like all other callers make sure this doesn't happen in the caller, so it looks like the non-integral case is the only way to currently reach here. Doesn't mean of course that the other callers will handle the non-integral case properly. If you prefer, I can lift this check into the ptr and swap the operands there (it's a bit awkward, because one's a Value* and one's a SEC* and it may be the wrong way around), but either way is fine with me. Let me know which you think is cleaner.


https://reviews.llvm.org/D33129