[llvm-dev] [RFC] Introducing classes for the codegen driven by new pass manager

Mon Jul 13 21:28:00 PDT 2020

Sorry, I should have included the bug:
https://bugs.llvm.org/show_bug.cgi?id=46651. Just today I added the
directories containing the failures, sorted by count. Repro instructions
are in the first comment.

On Mon, Jul 13, 2020 at 9:23 PM Eric Christopher <echristo at gmail.com> wrote:

> Let's get a burn down list and some directions, I'll try to help :)
>
> -eric
>
> On Mon, Jul 13, 2020 at 9:21 PM Arthur Eubanks via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> I looked more closely at some of the remaining check-llvm failures under
>> NPM, and quite a few are due to passes that haven't been ported to NPM yet.
>> The ones I looked at all share the trait of needing some analysis to
>> provide a TargetMachine, which doesn't exist in NPM yet. So actually some
>> of your work is required for the optimizer pipeline NPM switch.
>>
>> On Mon, Jul 13, 2020 at 6:23 PM Chen, Yuanfang <Yuanfang.Chen at sony.com>
>> wrote:
>>
>>>
>>>
>>>
>>> -Yuanfang
>>>
>>> > -----Original Message-----
>>> > From: Arthur Eubanks <aeubanks at google.com>
>>> > Sent: Monday, July 13, 2020 12:49 PM
>>> > To: Chen, Yuanfang <Yuanfang.Chen at sony.com>
>>> > Cc: LLVM Developers' List <llvm-dev at lists.llvm.org>
>>> > Subject: Re: [llvm-dev] [RFC] Introducing classes for the codegen
>>> driven by
>>> > new pass manager
>>> >
>>> >       While we're still working towards using NPM for optimizer
>>> pipeline by
>>> > default, we still don't have a machine pass interface and the
>>> corresponding
>>> > machine pass manager using NPM. The potential benefits using NPM aside,
>>> > this inhibits us from making any progress on deprecating LPM for the
>>> > codegen pipeline which blocks removing LPM altogether. The purpose of
>>> this
>>> > series of patches is to (1) let pass developers write or port machine
>>> passes to
>>> > a new machine pass interface and be able to test it with `llc`. (2)
>>> let a target
>>> > have the choice of implementing the codegen pipeline using NPM
>>> (Work-in-
>>> > Progress). Maybe it is obvious already, but I also want to mention
>>> that these
>>> > patches do not intend to force a target to migrate to NPM right way.
>>> >
>>> >
>>> > Awesome!
>>> >
>>> > It would be awesome to delete all the LPM infra at some point in the
>>> future.
>>> > But even just deleting all the optimizer pipeline LPM infra would be a
>>> big win,
>>> > and that shouldn't be tied to codegen.
>>> >
>>>
>>> True. Opt and codegen pipeline could make independent progress of NPM
>>> migration.
>>>
>>> >
>>> >       * Goal-1 *
>>> >       https://reviews.llvm.org/D67687
>>> >
>>> >       Four member methods of a machine pass are recognized by the
>>> > machine pass manager:
>>> >       (1) `PreservedAnalyses run(MachineFunction &,
>>> > MachineFunctionAnalysisManager &)`. Majority of the machine passes use
>>> > this.
>>> >       (2) `Error doInitialization(Module &,
>>> > MachineFunctionAnalysisManager &)`. Passes like AsmPrinter needs a hook
>>> > to lower/transform global constructs. (invoked before all passes `run`
>>> > method)
>>> >       (3) `Error doFinalization(Module &,
>>> > MachineFunctionAnalysisManager &)`. Client: PrintMIRPass.  This is
>>> also for
>>> > completeness. (invoked after all passes `run` method)
>>> >       (4) `Error run(Module &, MachineFunctionAnalysisManager &)`.
>>> > Client: MachineOutliner, DebugifyMachineModule. I would call this
>>> machine
>>> > module pass which needs a global scope. It is like (1) but subject to
>>> pass
>>> > ordering. Currently, a pass either define this or (1), but not both.
>>> >
>>> >       (doInitialization/doFinalization is currently not supported by
>>> the NPM
>>> > optimizer pipeline because there is no real need for it.
>>> >       http://lists.llvm.org/pipermail/llvm-dev/2018-
>>> > September/126504.html)
>>> >
>>> >
>>> > Are doInitialization/doFinalization really necessary? As mentioned in
>>> the
>>> > previous discussion, it seems like usually the things in
>>> > doInitialization/doFinalization are not logically in the right place.
>>> > For example, maybe PrintMIRPass should just be a module pass, like
>>> > PrintModulePass.
>>>
>>> Good point! It is very likely that PrintMIRPass could be implemented
>>> with (4) above.
>>> For AsmPrinter's uses of ` doInitialization`, I'm not 100% sure.  It
>>> looks like it could also be replaced by (4).
>>> But difference is ` doInitialization` is invoked before all passes `run`
>>> method, whereas (4) is invoked by pass order.
>>> I don't know if there are any implicit/subtle dependency in this regard.
>>>
>>> It would be great to have some codegen folks to shed light on it here.
>>>
>>> >
>>> >
>>> >
>>> >       * Goal-2 *
>>> >       https://reviews.llvm.org/D67687
>>> >
>>> >       Unlike IR where `has-a` relationship exists among
>>> > module/function/loop/cgscc etc., the MIR does not have `has-a`
>>> relationship
>>> > with any kind of IR. It does have a one-on-one relationship with IR
>>> function.
>>> > So, transforming MIR does not change any IR unit at all. Invalidating
>>> a MIR
>>> > analysis result also does not invalidate any IR analysis result.
>>> >
>>> >
>>> >       Based on the above observation, the machine pass manager runs
>>> > standalone, i.e. combining it with other types of pass managers using
>>> adaptor
>>> > passes are not supported. There is also no proxy defined for machine
>>> > analysis manager with any other types of analysis managers. The machine
>>> > analysis manager does provide API to register and query IR analysis
>>> because
>>> > machine passes need IR analysis result in general.
>>> >
>>> >
>>> > Maybe this is my lack of familiarity with codegen, but why doesn't a
>>> Module
>>> > contain all its MachineFunctions? It seems like that adaptor would be
>>> > necessary.
>>>
>>> Because their data structures are separated. Logically they are
>>> representations of the same/similar thing
>>> at different abstraction level.
>>>
>>> >
>>> >
>>> >       ** Testing **
>>> >       - Since the `llc` options are compatible, as passes are ported
>>> to NPM
>>> > and various issues got resolved, we should see more tests passing when
>>> `llc -
>>> > enable-new-pm` is turned on implicitly via an (maybe) knob similar to
>>> `cmake
>>> > -DENABLE_EXPERIMENTAL_NEW_PASS_MANAGER`.
>>> >       - A buildbot to make sure no regression when `llc
>>> -enable-new-pm` is
>>> > implicitly on?
>>> >       - Any idea on this regard is much appreciated.
>>> >
>>> >
>>> > Manually running tests once in a while might be good enough, not sure
>>> if the
>>> > cost of setting up a bot that maintains some sort of list of tests
>>> that have
>>> > passed in the past is worth it. From my limited experience, tests
>>> won't really
>>> > tend to regress under NPM as long as you have some tests explicitly
>>> testing
>>> > NPM sprinkled around.
>>>
>>> Sounds good. I was afraid if it takes too long to migration codegen
>>> passes, there may be regressions introduced
>>> into IR-to-obj tests under NPM (they exercise mush more than a single or
>>> a few consecutive passes). I think it's fine to consider this in the future.
>>>
>>>
>>>
>>> _______________________________________________
>> 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/20200713/79ae463d/attachment-0001.html>