Chen, Yuanfang via llvm-dev
2020-Jul-14 01:23 UTC
[llvm-dev] [RFC] Introducing classes for the codegen driven by new pass manager
-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.
Arthur Eubanks via llvm-dev
2020-Jul-14 04:20 UTC
[llvm-dev] [RFC] Introducing classes for the codegen driven by new pass manager
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. > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200713/9ff2b763/attachment.html>
Eric Christopher via llvm-dev
2020-Jul-14 04:22 UTC
[llvm-dev] [RFC] Introducing classes for the codegen driven by new pass manager
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/0e6b02d2/attachment.html>
James Y Knight via llvm-dev
2020-Jul-14 21:29 UTC
[llvm-dev] [RFC] Introducing classes for the codegen driven by new pass manager
I'd just note that not every pass you can run with "opt" is actually part of the optimization pipeline. There are a few important IR-level passes that only run in the codegen pipeline, but are still nameable with opt to run individually for testing purposes. Switching over doesn't need to block on these passes being migrated. So I'm not sure this method of determining progress towards switching to NPM actually makes sense. E.g., looking at the first few entries on your list, amdgpu-lower-intrinsics and codegenprepare are codegen only. However, objc-arc is not -- that does really need to be ported. Then there's things like loop-unswitch -- the functionality is ported to NPM, but as a different 'simple-loop-unswitch' pass. Probably there's no reason to migrate loop-unswitch itself. On Tue, Jul 14, 2020 at 12:20 AM 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/20200714/452c79cd/attachment.html>