[PATCH] D134056: [ModuleInliner] Move InlinePriority and its derived classes to InlineOrder.cpp (NFC)

[Liqiang Tao via Phabricator via llvm-commits]

taolq added inline comments.


================
Comment at: llvm/lib/Analysis/InlineOrder.cpp:124
+    while (PriorityPtr->updateAndCheckDecreased(Heap.front())) {
+      std::pop_heap(Heap.begin(), Heap.end(), isLess);
+      std::push_heap(Heap.begin(), Heap.end(), isLess);
----------------
taolq wrote:
> kosarev wrote:
> > It looks something is not quite right about how this heap is formed. An `assert(std::is_heap(Heap.begin(), Heap.end(), isLess));` added before that line would fail on `Transforms/Inline/nested-inline.ll`. Can anyone take a look, please?
> > 
> > Originally caught on a build with libc++'s consistency checks enabled, <https://github.com/llvm/llvm-project/issues/68594>.
> Thanks for the information. I will take a look.
I just figure it out. The `assert` you added failed because the Heap is updated lazily. After line 123 `updateAndCheckDecreased`, the priority has been changed, but the Heap has not been resorted yet.
So `assert(std::is_heap(Heap.begin(), Heap.end(), isLess));` should be appended after std::push_heap.
And `Transforms/Inline/nested-inline.ll` would pass with that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134056