[PATCH] D37353: [SparsePropagation] Enable interprocedural analysis

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added a comment.

In https://reviews.llvm.org/D37353#886179, @dberlin wrote:

> It's worth noting: You can make propagation much faster by keeping a separate worklist for overdefined  , and  prioritizing it over the normal one.
>  This will move things to overdefined as quickly as possible.
>  (Because anything meet overdefined is just about always overdefined).
>
> This is what SCCP does.
>  Again, just recording it here, i can do it in a followup.


Thanks again for taking a look at this! Right, we should eventually make a separate worklist for overdefined, like SCCP does.



================
Comment at: include/llvm/Analysis/SparsePropagation.h:48
+  using LatticeVal = const void *;
+  using StateTy = DenseMap<Value *, LatticeVal>;
 
----------------
dberlin wrote:
> Is this, and the associated changes you make to do the mapping, still needed if lattice values can be something other than void pointers?
I added `StateTy` here only as shorthand - it's not really needed, especially with the `auto` suggestions you have. The main changes I'm making to the mapping are adding distinct maps for values loaded-from/stored-to global variables and function return values. I also changed the existing mapping from Instruction -> LatticeVal to Value -> LatticeVal to handle arguments. This mimics the behavior of IPSCCP.


================
Comment at: include/llvm/Analysis/SparsePropagation.h:167
+  /// BBWorkList - Holds basic blocks that should be processed.
+  std::vector<BasicBlock *> BBWorkList;
 
----------------
dberlin wrote:
> SmallVector please
Sounds good.


================
Comment at: include/llvm/Analysis/SparsePropagation.h:171
+  /// that should be processed.
+  std::vector<Value *> ValueWorkList;
 
----------------
dberlin wrote:
> SmallVector please
Sounds good.


================
Comment at: lib/Analysis/SparsePropagation.cpp:58
+SparseSolver::LatticeVal SparseSolver::getValueState(Value *V) const {
+  StateTy::const_iterator I = ValueState.find(V);
+  return I != ValueState.end() ? I->second : LatticeFunc->getUntrackedVal();
----------------
dberlin wrote:
> dberlin wrote:
> > auto
> Given this occurs so often, can you please just pull it out into a templated helper.
> 
> template <class T, class V>
> LatticeVal findOrReturnUntracked(T Mapping, V Value)
> {
> auto I = Mapping.find(V);
> return  I != Mapping.end() ? I->second : LatticeFunc->getUntrackedVal;
> }
> 
> or whatever.
> 
> 
Sounds good.


https://reviews.llvm.org/D37353