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

[David Li via llvm-commits]

davidxl added a subscriber: davidxl.

================
Comment at: lib/Transforms/IPO/Inliner.cpp:842
@@ +841,3 @@
+      // instruction iterator and backing up one.
+      auto LastBBI = std::prev(F.end());
+
----------------
Where is this behavior documented? It seems very fragile to depend on implication details here -- there is not even a way to do assert.

================
Comment at: lib/Transforms/IPO/Inliner.cpp:851
@@ +850,3 @@
+      RefVisited.clear();
+      for (BasicBlock &BB : make_range(std::next(LastBBI), F.end()))
+        for (Instruction &I : BB) {
----------------
It is quite unfortunate IR will need to be traversed again here -- there is compile time overhead here. Why can't IFI be used to get the newly exposed callsite?

================
Comment at: lib/Transforms/IPO/Inliner.cpp:863
@@ +862,3 @@
+              }
+          for (Value *Op : I.operand_values())
+            if (Constant *C = dyn_cast<Constant>(Op))
----------------
If a CGSCC pass has to go through hoops to get SCC graph update properly, I feel that something is wrong. Having multiple levels of SCC is one thing whose complexity can probably be hidden,  but requiring every pass to pay the penalty is a different story.


https://reviews.llvm.org/D24226