[PATCH] D98147: [SCEV] Improve modelling for pointer constants

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1076
+    // Does the operand happen to be a (pointer) constant? If so, see if we can
+    // produce an integer constant (but *NOT* an ptr2int constant expression!).
+    if (auto *PC = dyn_cast<Constant>(U->getValue()))
----------------
Meinersbur wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > Meinersbur wrote:
> > > > typo is still there
> > > > 
> > > > A `GlobalValue`would but such a constant, but why handle it differently than a local value?
> > > Because if we allow `ConstantExpr::getPtrToInt()` to return `ptrtoint` constant expression,
> > > `getSCEV()` will immediately ask createSCEV() to model it as a SCEV expression,
> > > which will result in calling `ScalarEvolution::getPtrToIntExpr()`,
> > > which will then ask `ConstantExpr::getPtrToInt()`, which will then return `ptrtoint` constant expression,
> > > `getSCEV()` will immediately ask createSCEV() to model it as a SCEV expression,
> > > which will result in calling `ScalarEvolution::getPtrToIntExpr()`,
> > > which will then ask `ConstantExpr::getPtrToInt()`, which will then return `ptrtoint` constant expression,
> > > `getSCEV()` will immediately ask createSCEV() to model it as a SCEV expression,
> > > which will result in calling `ScalarEvolution::getPtrToIntExpr()`,
> > > which will then ask `ConstantExpr::getPtrToInt()`, which will then return `ptrtoint` constant expression,
> > > `getSCEV()` will immediately ask createSCEV() to model it as a SCEV expression,
> > > which will result in calling `ScalarEvolution::getPtrToIntExpr()`,
> > > which will then ask `ConstantExpr::getPtrToInt()`, which will then return `ptrtoint` constant expression,
> > > `getSCEV()` will immediately ask createSCEV() to model it as a SCEV expression,
> > > which will result in calling `ScalarEvolution::getPtrToIntExpr()`,
> > Edit: 
> > 
> > > IIUC, this is to reduce ptr2int(int2ptr(x)) -> x?
> > 
> > Hm, that's funny. I thought i replied to that question yesterday, but the inline comment is gone.**bold text**
> > 
> > > A GlobalValuewould but such a constant, but why handle it differently than a local value?
> > 
> > Mainly we want this so that if we call `ScalarEvolution::getPtrToIntExpr()`
> > on a `SCEVUnknown` that is `ConstantPointerNull`, we don't end up with a
> > `SCEVPtrToIntCast` of an `SCEVUnknown` of a `ConstantPointerNull`,
> > but with a nice `SCEVConstant` with a value of 0.
> > Mainly we want this so that if we call ScalarEvolution::getPtrToIntExpr() on a SCEVUnknown that is ConstantPointerNull [...]
> 
> Why not test `if (auto *PC = dyn_cast<ConstantPointerNull>(U->getValue()))` specifically?
That would be conservatively correct for sure, yes.
But i'm just not sure as to why should we make such a restriction?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98147