[PATCH] D36311: [ThinLTO] Add GraphTraits for FunctionSummaries

[Charles Saternos via Phabricator via llvm-commits]

ncharlie added a comment.

> Maybe add an scc dumper facility for the module summary graph, which creates an scc_iterator from the module summary and just dumps the nodes in each scc, that can be checked using a Lit based test

I'll look into adding this.



================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:838
+    // (since scc_iterator doesn't accept nullptrs)
+    auto F = llvm::make_unique<FunctionSummary>(
+        FunctionSummary::GVFlags(
----------------
tejohnson wrote:
> ncharlie wrote:
> > This is causing problems. The scc_iterator gets stuck on these functionsummaries since they're not in the original graph, which causes it to terminate before going through all the SCCs in the summary callgraph.
> In what way does it get stuck? The CallGraph.h has a dedicated CallsExternalNode for calls to external functions, but it seems like you are doing something similar here with a static ExternalFunction.
Sorry, nvm that comment - I was misattributing it to external nodes before, but it was really just because my entry point for the Index was wrong.


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:28
 #include "llvm/IR/Module.h"
+#include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include <algorithm>
----------------
tejohnson wrote:
> Not needed in this patch?
Oops - leftover from a different branch. I'll remove that.


https://reviews.llvm.org/D36311