[PATCH] D94808: [NewPM][Inliner] Move mandatory inliner inside function simplification pipeline

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:661
                         FunctionAnalysisManager &FAM, Module &M) {
+  if (Mandatory) {
+    OwnedAdvisor = std::make_unique<MandatoryInlineAdvisor>(FAM);
----------------
mtrofin wrote:
> This should move to line 675: if we have installed an advisor, we use it, and the inliner pass passes to getAdvice() whether it only needs mandatory advice. All advisors need to support the mandatory case anyway.
The whole point of https://bugs.llvm.org/show_bug.cgi?id=46945 was that we need to run the alwaysinliner first. Even if all advisors support the mandatory case, they may inline in the wrong order. That can either be fixed by running alwaysinliner first, or by having the inliner look at alwaysinline calls first.

If we have an advisor like the ML one, we will always use that one, then potentially end up inlining in the wrong order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94808