[llvm] [polly] [SCEV] Use SCEVPtrToAddr instead of SCEVPtrToInt in SCEV. (PR #180244)

[Nikita Popov via llvm-commits]

================
@@ -1155,10 +1116,22 @@ const SCEV *ScalarEvolution::getPtrToAddrExpr(const SCEV *Op) {
 const SCEV *ScalarEvolution::getPtrToIntExpr(const SCEV *Op, Type *Ty) {
   assert(Ty->isIntegerTy() && "Target type must be an integer type!");
 
-  const SCEV *IntOp = getLosslessPtrToIntExpr(Op);
-  if (isa<SCEVCouldNotCompute>(IntOp))
-    return IntOp;
+  // It isn't legal for optimizations to construct new ptrtoint expressions
+  // for non-integral pointers.
+  if (getDataLayout().isNonIntegralPointerType(Op->getType()))
+    return getCouldNotCompute();
+
+  Type *IntPtrTy = getDataLayout().getIntPtrType(Op->getType());
+
+  // We can only trivially model ptrtoint via ptrtoaddr if SCEV's effective
+  // (integer) type is sufficiently wide to represent all possible pointer
+  // values. We could theoretically teach SCEV to truncate wider pointers, but
+  // that isn't implemented for now.
+  if (getDataLayout().getTypeSizeInBits(getEffectiveSCEVType(Op->getType())) !=
+      getDataLayout().getTypeSizeInBits(IntPtrTy))
+    return getCouldNotCompute();
 
+  const SCEV *IntOp = getPtrToAddrExpr(Op);
----------------
nikic wrote:

In theory, yes, in practice, no. If we expand the SCEV and replace the original with it, that's fine in isolation, because the old ptrtoint is still there so provenance is properly exposed. However, because LLVM currently doesn't model the side effect at the IR level, that ptrtoint can then be DCE, which means that overall we end up replacing a ptrtoint with ptrtoaddr altogether, which is incorrect.

My hope here was that as long as SCEV itself only generates ptrtoaddr for everything, we can treat IR level ptrtoint as SCEVUnknown. Is the problem here that we run into issues with pointer differences currently being represented as sub of ptrtoint in IR, instead of sub of ptrtoaddr?

https://github.com/llvm/llvm-project/pull/180244