[PATCH] D37355: Add CalledValuePropagation pass

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added a comment.

In https://reviews.llvm.org/D37355#886173, @davide wrote:

> Generally I feel like it's getting there. Some small comments, but I expect this to be ready to be committed soon'ish


Thanks very much, Davide!



================
Comment at: lib/Transforms/IPO/CalledValuePropagation.cpp:181-190
+  void PrintValue(LatticeVal LV, raw_ostream &OS) {
+    if (LV == getUndefVal())
+      OS << "Undefined  ";
+    else if (LV == getOverdefinedVal())
+      OS << "Overdefined";
+    else if (LV == getUntrackedVal())
+      OS << "Untracked  ";
----------------
davide wrote:
> you can simplify just doing 
> `return OS <<` 
Thanks!


================
Comment at: lib/Transforms/IPO/CalledValuePropagation.cpp:286
+  // set.
+  std::set<CVPLatticeVal> LatticeVals;
+
----------------
davide wrote:
> DenseSet maybe?
Yeah, we can use DenseSet if we template the lattice function and get rid of the void pointers.


================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:717
+    // Attach metadata to indirect call sites indicating the set of functions
+    // they may target at run-time. This should follow IPSCCP.
+    PM.add(createCalledValuePropagationPass());
----------------
davide wrote:
> I'd make an RFC on llvm-dev as people might have custom pipelines.
Good idea.


https://reviews.llvm.org/D37355