[PATCH] D37353: [SparsePropagation] Enable interprocedural analysis

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 9 13:30:45 PDT 2017


dberlin added inline comments.


================
Comment at: lib/Analysis/SparsePropagation.cpp:318
+  case Instruction::Ret:   return FunctionState;
+  case Instruction::Store: return GlobalVariableState;
+  default:                 return ValueState;
----------------
This assumes a very specific set of lattices being used here.
The ret seems pretty generic. The store one, ....

I guess i feel like this kind of mapping function  should provide, not something part of the sparse solver.

IE maybe you need SparseSolverTraits, and this is one of those functions.

(that would avoid the overhead of virtual calling).

Essentially, i feel like this is one step too specific.

I'm not sure it's worth going all the way to having the clients own *all* the state maps, and do their initialization.
But i definitely don't think you should need to special case store here (and not, say, load).
For example, use case wise, the sparse solver should be able to be used easily replace GlobalsModRef if you wanted to.

Maybe halfway? The function and value states are managed by the sparse solver, and other state maps are owned/initialized by the client, and this function is the only interface to get the right map?



https://reviews.llvm.org/D37353





More information about the llvm-commits mailing list