[PATCH] D104028: [WIP] Use standard priority queue to order inlining

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

With the introduction of the flag, there shouldn't be any more failing tests, right?



================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:104
+static cl::opt<bool> InlineEnablePriorityInlineOrder(
+    "inline-enable-priority-inline-order", cl::Hidden, cl::init(false),
+    cl::desc("Enable the priority inline order for the inliner"));
----------------
nit: drop the second `inline`, simpler:

InlineEnablePriorityOrder
inline-enable-priority-order


================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:832
   // incrementally maknig a single function grow in a super linear fashion.
-  DefaultInlineOrder<std::pair<CallBase *, int>> Calls;
+  std::unique_ptr<InlineOrder<std::pair<CallBase *, int>>> CallsPtr(
+      new DefaultInlineOrder<std::pair<CallBase *, int>>());
----------------
nit: when the flag is enabled, you end up allocating an object just to drop it right after. You can just allocate the appropriate one on each branch of the if statement. Then, if you want, just assert after the if that CallsPtr is not null - this will help maintaining this later, when there are more than 2 ordering implementations, and it makes the design crystal-clear.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104028/new/

https://reviews.llvm.org/D104028