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

Mircea Trofin via llvm-dev llvm-dev at lists.llvm.org
Thu May 20 11:32:25 PDT 2021 

On Thu, May 20, 2021 at 11:02 AM David Blaikie <dblaikie at gmail.com> wrote:

> 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") -
>
Yup, that's what I meant :) thanks!


> 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/0ef17609/attachment.html>

More information about the llvm-dev mailing list