[PATCH] D23738: [PM] Extend the explicit 'invalidate' method API on analysis results to accept an Invalidator that allows them to invalidate themselves if their dependencies are in turn invalidated.

[Sean Silva via llvm-commits]

silvas added a subscriber: silvas.
silvas added a comment.

This is a big improvement on the current state of things and can probably get rid of most if not all of the insufficient invalidation issues I was running into without a big refactor to pass stuff into the query path of a ton of analyses.

This is somewhat limited because it only works within a single IRUnit. So it won't cover cases like:

- AAManager (function analysis) can be holding a pointer to GlobalsAA (module analysis)
- LoopAccessAnalysis (loop analysis) holds pointers to various function analysis
- Loop analyses implicitly hold pointers to Loop objects owned by LoopAnalysis. (analogously for CGSCC analyses and LazyCallGraph)

Those will likely not be an issue when running a standard pipeline as long as we don't have any transformations that preserve the inner proxies. E.g. during standard pipelines, when we invalidate module analyses and thus GlobalsAA, as long as transformations don't preserve the module to function proxy, the proxy will clear out the inner analysis manager.

In other words, the approach in this patch depends on having the for-loop in `PreservedAnalyses invalidate(IRUnitT &IR, PreservedAnalyses PA)` iterate over all analysis results that may need to be invalidated; however, since it is only iterating over the analysis results cached on a single IRUnit, any cross-IRUnit dependencies will not be correctly handled. (the invalidation of the inner proxies will invalidate analysis results cached on any inner IRUnitT's, so the main concern here is analysis results that hold pointers to analyses cached on their outer IRUnitT's or the their same IRUnitT)

IIRC, LoopAccessAnalysis was the only analysis that had data-structure-dependencies on other analyses (it holds `SCEV*` and `Loop*`) and so the cross-IRUnit case is probably worth thinking about long-term, even though currently I think we'll get away with it LoopAccessAnalysis getting invalidated by the function to loop proxy during standard pipelines.

FYI I'll be on vacation for the next two weeks, so feel free to move forward with this if others are onboard with the idea. I think this is a nice improvement.


https://reviews.llvm.org/D23738