[PATCH] D21464: [PM] WIP: Introduce basic update capabilities to the new PM's CGSCC pass manager, including both plumbing and logic to handle function pass updates.

[Sean Silva via llvm-commits]

silvas added a comment.

Some initial comments.


================
Comment at: include/llvm/Analysis/CGSCCPassManager.h:169
@@ +168,3 @@
+/// ignore this argument to their run method.
+struct CGSCCUpdateResult {
+  SmallPriorityWorklist<LazyCallGraph::RefSCC *, 1> &RCWorklist;
----------------
Please add specific documentation about how this CGSCCUpdateResult is used by CGSCC passes to communicate the modifications. One possibility for "documentation" may be to add unit tests covering basic patterns like how to update them when removing a function, how to update them when replacing a function, how to update them when adding edges, etc.

But I would also expect at least some basic comments on each member explaining under what scenarios it needs to be used and how.

================
Comment at: include/llvm/Analysis/CGSCCPassManager.h:388
@@ -190,2 +387,3 @@
     FunctionAnalysisManager &FAM =
-        AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C).getManager();
+        AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
+
----------------
This is error prone passing in both C and CG. For a given C there is only one valid CG and this signature gives the false impression that there are two degrees of freedom here. You already have the relevant back pointers to fetch the CG as needed so just use that.


http://reviews.llvm.org/D21464