[PATCH] D16802: [LCG] Construct an actual call graph with call-edge SCCs nested inside reference-edge SCCs.

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

Comments addressed.


================
Comment at: include/llvm/Analysis/LazyCallGraph.h:52
@@ -51,2 +51,3 @@
 #include <iterator>
+#include <utility>
 
----------------
sanjoy wrote:
> Looks like this header is unused.
std::pair comes from here.

================
Comment at: include/llvm/Analysis/LazyCallGraph.h:497
@@ +496,3 @@
+    ///
+    /// If SourceN ant may be split up due to breaking a cycle in the call
+    /// edges that formed it. If that happens, then this will potentially
----------------
sanjoy wrote:
> SourceN ant?  This first sentence sounds malformed.
Cleaned it up, sorry about that.

================
Comment at: lib/Analysis/LazyCallGraph.cpp:105
@@ +104,3 @@
+void LazyCallGraph::Node::insertEdgeInternal(Node &TargetN, Edge::Kind EK) {
+  EdgeIndexMap.insert(std::make_pair(&TargetN.getFunction(), Edges.size()));
+  Edges.emplace_back(TargetN, EK);
----------------
sanjoy wrote:
> Here and elsewhere: why not `{&TargetN.getFunction(), Edges.size()}` instead of an explicit `std::make_pair`?
When I first wrote the code, not all of our compilers supported {} syntax, and this make_pair didn't end up getting completely rewritten. I can update more of them though.

================
Comment at: lib/Analysis/LazyCallGraph.cpp:110
@@ +109,3 @@
+void LazyCallGraph::Node::setEdgeKind(Function &TargetF, Edge::Kind EK) {
+  Edges[EdgeIndexMap.find(&TargetF)->second].setKind(EK);
+}
----------------
sanjoy wrote:
> Why not `(*this)[&TargetF].setKind(EK)`?
The public interface doesn't expose a mutable edge.


http://reviews.llvm.org/D16802