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

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

I am tuning the performance by reordering inlining in downstream. My first try was to use std::priority_queue. But I tried to use the inline cost heuristic to order them. In this patch it looks like sort callsites by HistoryID? What's the intention?
BTW, for the performance side, SPEC2017 shows some regression with some improvement if we use std::priority_queue by InlineCost. It may be irrelevant to this patch.

PS2: Did you consider to move InlineOrder(s) class out of Inlined.cpp into the header and make it as a member of InlinedPass just like the Advisor? I guess it may be more easier to use them.



================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:769-780
+  void erase_if(function_ref<bool(T)> Pred) override {
+    PriorityQueue<T, std::vector<T>, Compare> TempPQ;
+    while (!PQ.empty()) {
+      auto P = PQ.top();
+      PQ.pop();
+      if (Pred({P.first, 0}))
+        InlineHistoryMap.erase(P.first);
----------------
I prefer to use std::vector<T> as data member instead of PriorityQueue. After that, the implementation may be simpler. For example:
```
swap all the required element to the end of the vector
std::make_heap(...)
```

And the implementation for pop and push wouldn't be much harder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104028