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

Fri Oct 13 06:07:42 PDT 2023

kosarev 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:
> 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.
I think `std:pop_heap()` requires the range to be a valid heap by the moment of the call; we should not rely on `std::push_heap()` fixing the heap.

Here's the backtrace:
```
******************** TEST 'LLVM :: Transforms/Inline/nested-inline.ll' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
RUN: at line 1: /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt < /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll -passes=inline -S | /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/FileCheck /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
+ /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt -passes=inline -S
+ /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/FileCheck /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
RUN: at line 2: /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt < /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll -passes='cgscc(inline)' -S | /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/FileCheck /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
+ /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt '-passes=cgscc(inline)' -S
+ /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/FileCheck /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
RUN: at line 3: /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt < /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll -passes='module-inline' -S | /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/FileCheck /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
+ /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt -passes=module-inline -S
+ /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/FileCheck /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
/usr/include/c++/11/bits/stl_heap.h:325:
In function:
    void std::pop_heap(_RAIter, _RAIter, _Compare) [with _RAIter = 
    llvm::CallBase**; _Compare = std::function<bool(const llvm::CallBase*, 
    const llvm::CallBase*)>]

Error: elements in iterator range [first, last) do not form a heap with 
respect to the predicate __comp.

Objects involved in the operation:
    iterator "first" @ 0x7ffe489f0268 {
    }
    iterator "last" @ 0x7ffe489f0260 {
    }
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt -passes=module-inline -S
 #0 0x00007fd0a21586c8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/kosarev/labs/llvm-project/llvm/lib/Support/Unix/Signals.inc:723:22
 #1 0x00007fd0a2158ae4 PrintStackTraceSignalHandler(void*) /home/kosarev/labs/llvm-project/llvm/lib/Support/Unix/Signals.inc:798:1
 #2 0x00007fd0a2155f29 llvm::sys::RunSignalHandlers() /home/kosarev/labs/llvm-project/llvm/lib/Support/Signals.cpp:105:20
 #3 0x00007fd0a2157f60 SignalHandler(int) /home/kosarev/labs/llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
 #4 0x00007fd0a19dd520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #5 0x00007fd0a1a319fc __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #6 0x00007fd0a1a319fc __pthread_kill_internal ./nptl/pthread_kill.c:78:10
 #7 0x00007fd0a1a319fc pthread_kill ./nptl/pthread_kill.c:89:10
 #8 0x00007fd0a19dd476 gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #9 0x00007fd0a19c37f3 abort ./stdlib/abort.c:81:7
#10 0x00007fd0a1c681fb std::__throw_bad_exception() (/lib/x86_64-linux-gnu/libstdc++.so.6+0xa51fb)
#11 0x00007fd0a35e2228 void std::pop_heap<llvm::CallBase**, std::function<bool (llvm::CallBase const*, llvm::CallBase const*)>>(llvm::CallBase**, llvm::CallBase**, std::function<bool (llvm::CallBase const*, llvm::CallBase const*)>) /usr/include/c++/11/bits/stl_heap.h:327:18
#12 0x00007fd0a35da8b5 (anonymous namespace)::PriorityInlineOrder<(anonymous namespace)::SizePriority>::adjust() /home/kosarev/labs/llvm-project/llvm/lib/Analysis/InlineOrder.cpp:227:20
#13 0x00007fd0a35d9f98 (anonymous namespace)::PriorityInlineOrder<(anonymous namespace)::SizePriority>::pop() /home/kosarev/labs/llvm-project/llvm/lib/Analysis/InlineOrder.cpp:256:30
#14 0x00007fd0a85337d0 llvm::ModuleInlinerPass::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) /home/kosarev/labs/llvm-project/llvm/lib/Transforms/IPO/ModuleInliner.cpp:180:24
#15 0x00007fd0a90fb66d llvm::detail::PassModel<llvm::Module, llvm::ModuleInlinerPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) /home/kosarev/labs/llvm-project/llvm/include/llvm/IR/PassManagerInternal.h:90:3
#16 0x00007fd0a294e171 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) /home/kosarev/labs/llvm-project/llvm/include/llvm/IR/PassManager.h:521:20
#17 0x000055c2085d4371 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) /home/kosarev/labs/llvm-project/llvm/tools/opt/NewPMDriver.cpp:527:10
#18 0x000055c208605385 main /home/kosarev/labs/llvm-project/llvm/tools/opt/opt.cpp:698:27
#19 0x00007fd0a19c4d90 __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#20 0x00007fd0a19c4e40 call_init ./csu/../csu/libc-start.c:128:20
#21 0x00007fd0a19c4e40 __libc_start_main ./csu/../csu/libc-start.c:379:5
#22 0x000055c2085d1605 _start (/home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/opt+0x1c605)
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /home/kosarev/labs/llvm-project/build/debug+expensive_checks/bin/FileCheck /home/kosarev/labs/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134056