[PATCH] D37353: [SparsePropagation] Enable interprocedural analysis

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added a comment.

I agree, `getStateMapForInstruction` is a bit strange. I've been thinking about what to do here. And one of the simplest things I came up with was to just define a custom `LatticeKey` type like we do a `LatticeVal`. This would allow the solver to maintain a single map (meaning we no longer need something like `getStateMapForInstruction`), and give the client the flexibility to separate the kinds of values it cares about.

So for example, to replicate what I currently have, the `LatticeKey` could be a `PointerIntPair` of a `Value*` and an enum indicating the grouping (value, function, global variable). `getValueState(Value *V)` and the family of functions I added for functions and global variables that may not exist yet in the map would be replaced by a single `getValueState(LatticeKey Key)`. If `Key` doesn't exist in the map it would delegate to a single `LatticeFunc->ComputeLatticeValue(LatticeKey Key)` that would produce a new `LatticeVal` for `Key`. Currently, we have separate compute methods for arguments, functions, global variables, etc., which would also go away. It seems reasonable to move that complexity out of the generic classes. Additionally, the `ComputeInstructionState` functions would then be responsible for coming up with the right key. For example, `visitStore` would look something like:

  auto *GV = dyn_cast<GlobalVariable>(Store.getPointerOperand());
  if (!GV)
    return;
  LatticeKey Key(GV, LatticeKey::GlobalVariable);
  ChangedValues[Key] = MergeValues(...);

What do you think?


https://reviews.llvm.org/D37353