[PATCH] D37353: [SparsePropagation] Enable interprocedural analysis

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 13 19:59:29 PDT 2017


dberlin accepted this revision.
dberlin added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D37353#896220, @mssimpso wrote:

> Implement the previously discussed approach using generic LatticeKeys. A few things to note about this version of the patch:
>
> First, there's a requirement that LatticeKeys are able to be the key_type of whatever mapping structure the generic solver uses internally (i.e., DenseMap). If a client uses a non-trivial type for LatticeKey, it will have to provide a specialization of DenseMapInfo. But we already have specializations for pointers and PointerIntPairs, which I think probably covers most potential clients. I've added a comment at the top of AbstractLatticeFunction mentioning this.


This seems a reasonable restriction, it's true in other parts of llvm as well anyway, so ..

> Second, there are cases when the generic solver needs to translate between LatticeKeys and their corresponding LLVM Values. One case is when it needs to add LLVM Values to the work list after the state of a LatticeKey changes. Another case is when the solver tries to determine the executable blocks, and needs to get the state of an LLVM Value (e.g., a branch or switch condition or phi node). It needs to know the LatticeKey of a Value before it can get its associated state. I originally thought about adding two virtual functions to AbstractLatticeFunction that would let the client define the conversions between LatticeKeys and LLVM Values. But I thought the overhead of the calls might be too large, especially for simple cases, like when the LatticeKey is just an LLVM Value. I instead went with a LatticeKeyInfo template that clients can specialize for their chosen LatticeKey type.

SGTM

> The test cases show how this works for simple ipa-cp-like problems. If the approach looks good, I'll update the summary and the CalledValuePropagation pass (https://reviews.llvm.org/D37355).

At this point, i think it looks great to start. I don't think it's worth making any more changes until their are more clients. Thanks for working through this, i think the sparse propagation solver is a *lot* more useful thanks to your work.


https://reviews.llvm.org/D37353





More information about the llvm-commits mailing list