[PATCH] D24226: [PM] Provide an initial, minimal port of the inliner to the new pass manager.

[Piotr Padlewski via llvm-commits]

Prazek added a subscriber: Prazek.
Prazek added a comment.

Some small nits.


================
Comment at: lib/Analysis/CGSCCPassManager.cpp:140-141
@@ +139,4 @@
+    CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR, bool DebugLogging) {
+  typedef LazyCallGraph::SCC SCC;
+  typedef LazyCallGraph::RefSCC RefSCC;
+
----------------
why not using?

================
Comment at: lib/Analysis/CGSCCPassManager.cpp:199-200
@@ +198,4 @@
+                                bool DebugLogging) {
+  typedef LazyCallGraph::SCC SCC;
+  typedef LazyCallGraph::RefSCC RefSCC;
+
----------------
same.

================
Comment at: lib/Analysis/CGSCCPassManager.cpp:269
@@ -179,3 +268,3 @@
       for (Value *Op : I.operand_values())
         if (Constant *C = dyn_cast<Constant>(Op))
           if (Visited.insert(C).second)
----------------
I know that it is not your change, but auto

================
Comment at: lib/Analysis/CGSCCPassManager.cpp:388-391
@@ +387,6 @@
+    CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR, bool DebugLogging) {
+  typedef LazyCallGraph::Node Node;
+  typedef LazyCallGraph::Edge Edge;
+  typedef LazyCallGraph::SCC SCC;
+  typedef LazyCallGraph::RefSCC RefSCC;
+
----------------
usings?

================
Comment at: lib/Analysis/CGSCCPassManager.cpp:432
@@ +431,3 @@
+  // potentially dead edges.
+  for (Instruction &I : instructions(F)) {
+    for (Value *Op : I.operand_values())
----------------
auto?

================
Comment at: lib/Analysis/CGSCCPassManager.cpp:433
@@ +432,3 @@
+  for (Instruction &I : instructions(F)) {
+    for (Value *Op : I.operand_values())
+      if (Constant *C = dyn_cast<Constant>(Op))
----------------
maybe auto

================
Comment at: lib/Analysis/CGSCCPassManager.cpp:434
@@ +433,3 @@
+    for (Value *Op : I.operand_values())
+      if (Constant *C = dyn_cast<Constant>(Op))
+        if (Visited.insert(C).second)
----------------
auto

================
Comment at: lib/Analysis/LazyCallGraph.cpp:28
@@ -26,16 +27,3 @@
                     LazyCallGraph::Edge::Kind EK) {
-  // Note that we consider *any* function with a definition to be a viable
-  // edge. Even if the function's definition is subject to replacement by
-  // some other module (say, a weak definition) there may still be
-  // optimizations which essentially speculate based on the definition and
-  // a way to check that the specific definition is in fact the one being
-  // used. For example, this could be done by moving the weak definition to
-  // a strong (internal) definition and making the weak definition be an
-  // alias. Then a test of the address of the weak function against the new
-  // strong definition's address would be an effective way to determine the
-  // safety of optimizing a direct call edge.
-  if (!F.isDeclaration() &&
-      EdgeIndexMap.insert({&F, Edges.size()}).second) {
-    DEBUG(dbgs() << "    Added callable function: " << F.getName() << "\n");
-    Edges.emplace_back(LazyCallGraph::Edge(F, EK));
-  }
+  if (!EdgeIndexMap.insert({&F, Edges.size()}).second)
+    return;
----------------
emplace/try_emplace?


https://reviews.llvm.org/D24226