[PATCH] D111533: [SCEV] Invalidate user SCEVs along with operand SCEVs to avoid cache corruption

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

I think tracking users is generally the right thing to do, but I have some reservations about the approach. This makes a big implicit change, and I don't think we have a good understanding of the effects it has for all users of this API.

For example, if I look at https://github.com/llvm/llvm-project/blob/13c31539f7da403fee11fe2163249837460c3bf2/llvm/lib/Analysis/ScalarEvolution.cpp#L4424, which forgets all values using a SCEVUnknown placeholder, it seems like we should be replacing that code with //just// an invalidation of the SCEV node and it's users, rather than doing walks both on SCEV users and IR users. Of course, this would require also clearing out the corresponding SCEV <-> IR mappings at the same time.

I'm also thinking that the concept of invalid SCEVs (see checkValidity()) becomes rather silly if we're tracking users -- we should be dropping the invalid SCEV users directly and guarantee that none of the SCEVs in the map are invalid.

I would prefer it if we initially only landed the infrastructure to track users (without touching invalidation) and then migrate over individual invalidation users.


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

https://reviews.llvm.org/D111533