Details
- Reviewers
mtrofin
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Wondering if this makes sense to you (before I go and finish cleaning up all the tests)
I think InlineAdvisor::getAdvice needs to take a bool MandatoryOnly, which Inliner then passes. This allows us to then use the same advisor for both the always case and the policy-driven inliner instances, which allows that advisor to correctly book-keep any module wide state it may want to.
llvm/include/llvm/Transforms/IPO/Inliner.h | ||
---|---|---|
111 | Nit: const bool | |
llvm/lib/Transforms/IPO/Inliner.cpp | ||
661 | 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. |
llvm/lib/Transforms/IPO/Inliner.cpp | ||
---|---|---|
661 | 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. |
Nit: const bool