Currently, AAReachability does not compute anything and only use isPotentiallyReachable.
This patch makes it possible for AAReachability to use liveness information from AAIsDead.
In AAReachability::updateImpl, we collect reachable successor basic blocks for each basic block and construct a graph.
When reachability between two instructions is queried, check it using the graph.
Details
Diff Detail
Event Timeline
llvm/test/Transforms/Attributor/noalias.ll | ||
---|---|---|
919 | Make an NFC pre-commit with the tests and "old" check lines. That makes it clear in this patch what changed. Just commit the test without additional review. |
Thanks for working on this! Reachability should be used more and with an actual optimistic implementation it makes sense. The next user (in addition to AANoAlias) could be value simplification, for load/store simplification. Similarly to the noalias case we can preserve other properties for a program point if it is not reachable from all potential violations (I am thinking a use of a dereferenceable argument that cannot be reached from a function call that is not nofree. This makes sense once we split dereferenceable_globally from dereferenceable.) Let's get this in first though ;)
Below some comments.
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
---|---|---|
3462 | This seems odd. If there is no reason for this version let's remove both initialize methods. | |
3467 | Also not needed as far as I can tell. | |
3481–3484 | ||
3533 | This looks like the AAIsDead code, right? Maybe we can instead expose some edge based interface, e.g., AAIsDead::isEdgeDead(BasicBlock, BasicBlock) or AAIsDead::getLiveOutEdges(BasicBlock). WDYT? | |
3575 | We need to profile this but similar maps were causing memory problems before. Make the value a std::unique_ptr and that should already help. I'm not sure about this cache in general. I guess it mainly depends on the usage of this AA later on. For now we can keep it but we should invest in some time tracking for AAs at some point and thereby determine if this is too costly. If that is the case we could "cache" only the queries that were asked and recompute them in every update, not the entire graph. |
llvm/include/llvm/Transforms/IPO/Attributor.h | ||
---|---|---|
895 | Why don't these life in the AAReachabilityFunction? | |
llvm/lib/Transforms/IPO/AttributorAttributes.cpp | ||
2409 | Remove this and the initialize in the Impl class. | |
2415 | typo above. | |
2418 | Define this as EdgeTy or similar in the class once you moved the query vectors there. | |
2457 | Don't use count above but the second element of the return value of insert. |
Why don't these life in the AAReachabilityFunction?