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

Thu May 20 11:48:54 PDT 2021

Ok cool! I think that'd work for us. I'll put together a PR and add you as
reviewers if that's ok!

On Thu, May 20, 2021 at 7:32 PM Mircea Trofin <mtrofin at google.com> wrote:

>
>
> 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
>>>
>>

-- 
Neil Henning
Senior Software Engineer Compiler
unity.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210520/4a61d7c7/attachment.html>