[llvm-dev] How to use a custom InlineAdvisor with the new pass manager

Thu May 20 11:01:54 PDT 2021

Ah, is this more a NPM/LPM inliner design issue more broadly then - looks
like the NPM inliner didn't have a virtual "getInlineCost" even prior to
the inline advisor work, yeah? It has InlineParams, which then has
"getInlineCost", but it's not virtual, etc.

But, yes, adding this functionality (for 3rd party code to reuse the
inliner with custom heuristics) does seem reasonable, and the inline
advisor seems like a reasonable abstraction to use - hopefully there's an
easy/reasonable enough way to add a custom InlineAdvisor (maybe that's more
abstractions than needed? Not sure).

Yeah, sounds like Mircea's latest reply where you could provide a callback
to build the InlineAdvisor (I assume that's what Mircea meant, rather than
"InlineAnalysis") - if the advisor can't be built ahead of time (looks like
it needs the specific module, etc) and handed to the InlineAdvisorAnalysis,
then a functor to defer the creation until the parameters are available
makes sense to me.

- Dave

On Thu, May 20, 2021 at 9:09 AM Neil Henning via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> So for us we don't use the passes from PassBuilder - we've got our own cut
> down set to improve compile time by 2.5x over the default pipeline, while
> still maintaining vectorization and all the other fun things (tangentially
> - I was going to talk about this at EuroLLVM then COVID killed that!).
>
> At the moment what I've hacked in locally is:
>
> struct InlinerPass final : llvm::PassInfoMixin<InlinerPass>
> {
>     explicit InlinerPass(llvm::InlineParams&& params) : params(params),
> pass()
>     {
>     }
>
>     llvm::PreservedAnalyses run(llvm::LazyCallGraph::SCC&,
> llvm::CGSCCAnalysisManager&, llvm::LazyCallGraph&,
> llvm::CGSCCUpdateResult&);
>
> private:
>     const llvm::InlineParams params;
>     llvm::InlinerPass pass;
> };
>
> llvm::PreservedAnalyses InlinerPass::run(llvm::LazyCallGraph::SCC& scc,
> llvm::CGSCCAnalysisManager& analysisManager, llvm::LazyCallGraph&
> callGraph, llvm::CGSCCUpdateResult& updater)
> {
>     llvm::Module& module = *scc.begin()->getFunction().getParent();
>     llvm::FunctionAnalysisManager& functionAnalysisManager =
> analysisManager.getResult<llvm::FunctionAnalysisManagerCGSCCProxy>(scc,
> callGraph).getManager();
>
>     // Do something really awful - `OwnedAdvisor` is private in the
> `llvm::InlinerPass`, but we need to set it to our own advisor. Cast the
> class to a struct
>     // that happens to have the same layout, and set the field that way.
> This is totally undefined behaviour and bad, but LLVM hasn't given us the
> tools with
>     // the new pass manager to do this properly :'(.
>
> reinterpret_cast<LLVMInlinerPassPrivateMemberGetterHackeroo*>(&pass)->OwnedAdvisor.reset(new
> BurstInlineAdvisor(module, functionAnalysisManager, params));
>
>     return pass.run(scc, analysisManager, callGraph, updater);
> }
>
> So we can use the default llvm::InlinerPass but with our own custom
> InlineAdvisor. I *know* this is a hack though, and I think the correct
> solution (but please correct me if I'm wrong!) would be to add a version of
> InlineAdvisorAnalysis that takes our own InlineAdvisor?
>
> With the old pass manager the Inliner just had a virtual getInlineCost
> method that we could extend and override, so we didn't have this problem
> there.
>
> On Thu, May 20, 2021 at 4:45 PM Mircea Trofin <mtrofin at google.com> wrote:
>
>>
>>
>> On Thu, May 20, 2021 at 8:14 AM Neil Henning <neil.henning at unity3d.com>
>> wrote:
>>
>>> I can't edit InlineAdvisorAnalysis::Result::tryCreate to add my own
>>> InlineAdvisor there is the issue.
>>>
>> Not sure I follow: ReplayInlineAdvisor is constructed
>> in InlinerPass::getAdvisor. That aside, on the comment about editing: I
>> think I'm still missing some aspects of your scenario: wouldn't your
>> advisor be part of llvm?
>>
>>>
>>> I guess the easiest thing for our use-case might be to add another
>>> constructor to InlineAdvisorAnalysis that takes an InlineAdvisor, and this
>>> would set the Advisor field. That way I could just add to our
>>> AnalysisManager our own InlineAdvisorAnalysis, and we'd get the expected
>>> behaviour.
>>>
>> Oh - your scenario involves an advisor implemented outside the llvm tree,
>> is that the case? Can you share more details, e.g. how would it be loaded;
>> do you use the optimization pipelines and analysis managers in
>> PassBuilder.cpp, or you'd set up your own? By better understanding the
>> scenario, we can come up with a design that can probably help others, too.
>>
>> Thanks!
>>
>>
>>>
>>> Seems like a small enough thing to do in a PR - is that an acceptable
>>> change you think?
>>>
>>> On Thu, May 20, 2021 at 3:16 PM Mircea Trofin <mtrofin at google.com>
>>> wrote:
>>>
>>>> Ah, I see. There's a precedent for using custom InlineAdvisors, see for
>>>> instance (in Inliner.cpp) how the ReplayInlineAdvisor is handled. I suppose
>>>> you could do the same?
>>>>
>>>> On Thu, May 20, 2021 at 12:04 AM Neil Henning <neil.henning at unity3d.com>
>>>> wrote:
>>>>
>>>>> So what I need is for the default LLVM inliner to be able to use my
>>>>> InlineAdvisor in some fashion - without modifying tip LLVM *locally *to
>>>>> do so.
>>>>>
>>>>> So what I think I will have to do is land one of the proposals I
>>>>> stated originally (or a better idea from any of you fine folk) into LLVM *before
>>>>> *the LLVM 13 cutoff, so that when we pick up the LLVM 13 release in
>>>>> future we'll have the APIs available to set our own InlinerAdvisor.
>>>>>
>>>>> So to be clear - I'm totally ok to do a patch to LLVM to fix this, I
>>>>> just can't patch LLVM myself *locally *post-release because we are
>>>>> provided with a pre-built LLVM for some platforms we support.
>>>>>
>>>>> Hopefully that makes it a bit clearer?
>>>>>
>>>>> On Wed, May 19, 2021 at 4:21 PM Mircea Trofin <mtrofin at google.com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> On Wed, May 19, 2021 at 5:27 AM Neil Henning via llvm-dev <
>>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>>
>>>>>>> Hey list,
>>>>>>>
>>>>>>> I'm currently porting our HPC# Burst compiler over from the legacy
>>>>>>> pass manager to the new pass manager. While nearly everything went fine,
>>>>>>> I've hit one major hiccup that I can't seem to workaround - how can we have
>>>>>>> a custom `InlineAdvisor` for Burst without modifying tip LLVM.
>>>>>>>
>>>>>>
>>>>>> I'm trying to understand this better - you mean you'd want to load
>>>>>> the InlineAdvisor from a dynamic library, or something like that?
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> At present I've managed to completely bodge this locally by getting
>>>>>>> access to the `OwnedAdvisor` member of `InlinerPass` through very UB means
>>>>>>> (make a class of the same layout, casteroo, assign the field). Now this
>>>>>>> works in that I don't have codegen regressions anymore, but obviously this
>>>>>>> isn't the solution I want to ship!
>>>>>>>
>>>>>>> I was wondering if the list would object to us either:
>>>>>>>
>>>>>>>    1. Making the `OwnedAdvisor` field of `InlinerPass` protected,
>>>>>>>    so I could derive from `InlinerPass` and set the advisor.
>>>>>>>    2. I could make the `getAdvisor` virtual, and assign it that way.
>>>>>>>    3. Probably the 'best' fix would be to make
>>>>>>>    `InlineAdvisorAnalysis` somehow able to take a user-provided
>>>>>>>    `InlineAdvisor` - although I'd rather not use the static option
>>>>>>>    `UseInlineAdvisor` to set this. I don't really know how this solution would
>>>>>>>    look if I'm honest.
>>>>>>>
>>>>>>> I'm trying to understand what amount of changes to tip of tree are
>>>>>> OK for your scenario. Option 1 means modifying a .h; maybe option 2 needs a
>>>>>> recompile though (because virtual). So would option 3 (at this point, we
>>>>>> can talk about purpose-building support for your scenario, basically - if
>>>>>> rebuilding the compiler binaries is on the table)
>>>>>>
>>>>>> Thoughts from anyone? This is a blocker for us in the LLVM 13
>>>>>>> timeframe when we hope to enable the new pass manager as the default.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> -Neil.
>>>>>>>
>>>>>>> --
>>>>>>> Neil Henning
>>>>>>> Senior Software Engineer Compiler
>>>>>>> unity.com
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org
>>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Neil Henning
>>>>> Senior Software Engineer Compiler
>>>>> unity.com
>>>>>
>>>>
>>>
>>> --
>>> Neil Henning
>>> Senior Software Engineer Compiler
>>> unity.com
>>>
>>
>
> --
> Neil Henning
> Senior Software Engineer Compiler
> unity.com
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210520/cbac15f7/attachment-0001.html>