[PATCH] D51582: [IndVars] Turn isValidRewrite into an assertion

[Sanjoy Das via llvm-commits]

I think this particular problem was fixed a long time ago, likely
before I started working on SCEV.  I think the relevant bit is here:
https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/ScalarEvolution.cpp#L6451

// It's tempting to handle inttoptr and ptrtoint as no-ops, however this can
// lead to pointer expressions which cannot safely be expanded to GEPs,
// because ScalarEvolution doesn't respect the GEP aliasing rules when
// simplifying integer expressions.

because we don't handle ptrtoint (&p[n] - &p[0]) will be a SCEVUnknown
and SCEV should not reassociate through this expression.

-- Sanjoy
On Tue, Sep 4, 2018 at 9:52 AM Andrew Trick via Phabricator
<reviews at reviews.llvm.org> wrote:
>
> atrick added a comment.
>
> @sanjoy may have some insight as to why SCEV is now safe. If we know this why can't happen, you can completely remove `isValidRewrite` and don't need to assert.
>
> If you want to keep the logic under an assert, then you could add a more understandable comment:
>
> "Avoid converting `gep Base, (&p[n] - &p[0])` into `gep &p[n], Base - &p[0]`"
>
>
> https://reviews.llvm.org/D51582
>
>
>