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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3562
     Constant *GEP = ConstantExpr::getGetElementPtr(AllocTy, NullPtr, One);
-    return getSCEV(ConstantExpr::getPtrToInt(GEP, IntTy));
+    return getUnknown(ConstantExpr::getPtrToInt(GEP, IntTy));
   }
----------------
Is this change necessary?


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5513
+    ConstantRange X = getRangeRef(PtrToInt->getOperand(), SignHint);
+    return setRange(PtrToInt, SignHint, X.zextOrTrunc(BitWidth));
+  }
----------------
zextOrTrunc shouldn't be necessary.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:6043
   else if (isa<ConstantPointerNull>(V))
+    // FIXME: should there be a SCEVPointerConstant?
     return getZero(V->getType());
----------------
I think it would be a useful invariant if SCEVConstant was never a pointer.  We currently have a weird hack in SCEVExpander to deal with expanding pointer expressions that don't actually have any pointer operands.

I think we could just use `getUnknown(V)` here, though.  I can't think of any reason to have a special SCEV expression for null pointers.


================
Comment at: polly/test/Isl/CodeGen/ptrtoint_as_parameter.ll:3
+
+; FIXME: what is this trying to check?
+; XFAIL: *
----------------
I think it's just checking it doesn't crash.

More generally, I would guess your patch is interfering with polly's code to look through ptrtoint instructions.  If you want, I can fix up the polly stuff after this patch is finished.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89456