[PATCH] D16802: [LCG] Construct an actual call graph with call-edge SCCs nested inside reference-edge SCCs.
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 16 13:54:47 PST 2016
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
More information about the llvm-commits
mailing list