After talking with some NPM people, I believe the ultimate goal after NPM is enabled by default is to only support `-passes=`, and remove support for `-foo-pass`. However, until NPM is enabled by default, we still want tests using opt to use the legacy PM by default. We could attempt to make `-passes=` work with the legacy PM and have a legacy vs new PM flag, but given the design/syntax of `-passes=` I don't think that's feasible (see llvm/include/llvm/Passes/PassBuilder.h). So for making sure everything works with NPM, I think we need to support `-foo-pass` in NPM to be able to run all opt tests against NPM. Then at some point after NPM is enabled by default we can attempt to migrate everything to `-passes=`. On Thu, Jun 25, 2020 at 9:22 AM Hiroshi Yamauchi <yamauchi at google.com> wrote:> > > On Wed, Jun 24, 2020 at 11:13 AM Arthur Eubanks via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> Hi, >> >> As part of new pass manager work, I've been trying to get something like >> `opt -foo` working under the NPM, where `foo` is the name of a pass. >> >> In the past there's been no reason to keep the names of passes consistent >> between NPM and legacy PM. But now there is a reason to make them match, so >> that we don't have to touch every single test that uses `opt`. >> > > What's the goal here? Does it include removing the -passes option used by > the NPM? > > >> >> There are a couple of names that don't match though, for example the >> "basic alias analysis" pass is named "basicaa" under the legacy PM >> INITIALIZE_PASS_BEGIN(BasicAAWrapperPass, "basicaa", >> "Basic Alias Analysis (stateless AA impl)", true, >> true) >> but named "basic-aa" under the NPM >> FUNCTION_ALIAS_ANALYSIS("basic-aa", BasicAA()) >> . Almost all the other AA passes have a dash in them so I think it makes >> sense to rename "basicaa" -> "basic-aa". >> >> Is there accepted wisdom on renaming pass names? Is a pass name a stable >> interface? When is it ok to rename a pass? If there are 800 usages of a >> flag, should I rename them atomically? >> _______________________________________________ >> 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/20200625/369fa18e/attachment.html>
On Thu, Jun 25, 2020 at 7:48 PM Arthur Eubanks via llvm-dev <llvm-dev at lists.llvm.org> wrote:> > After talking with some NPM people, I believe the ultimate goal after NPM is enabled by default is to only support `-passes=`, and remove support for `-foo-pass`.Hm, is there any written rationale behind such a decision? I would have thought that -passes= is the temporary solution, not the other way around.> However, until NPM is enabled by default, we still want tests using opt to use the legacy PM by default. > We could attempt to make `-passes=` work with the legacy PM and have a legacy vs new PM flag, but given the design/syntax of `-passes=` I don't think that's feasible (see llvm/include/llvm/Passes/PassBuilder.h). > So for making sure everything works with NPM, I think we need to support `-foo-pass` in NPM to be able to run all opt tests against NPM. Then at some point after NPM is enabled by default we can attempt to migrate everything to `-passes=`.Roman.> On Thu, Jun 25, 2020 at 9:22 AM Hiroshi Yamauchi <yamauchi at google.com> wrote: >> >> >> >> On Wed, Jun 24, 2020 at 11:13 AM Arthur Eubanks via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>> >>> Hi, >>> >>> As part of new pass manager work, I've been trying to get something like `opt -foo` working under the NPM, where `foo` is the name of a pass. >>> >>> In the past there's been no reason to keep the names of passes consistent between NPM and legacy PM. But now there is a reason to make them match, so that we don't have to touch every single test that uses `opt`. >> >> >> What's the goal here? Does it include removing the -passes option used by the NPM? >> >>> >>> >>> There are a couple of names that don't match though, for example the "basic alias analysis" pass is named "basicaa" under the legacy PM >>> INITIALIZE_PASS_BEGIN(BasicAAWrapperPass, "basicaa", >>> "Basic Alias Analysis (stateless AA impl)", true, true) >>> but named "basic-aa" under the NPM >>> FUNCTION_ALIAS_ANALYSIS("basic-aa", BasicAA()) >>> . Almost all the other AA passes have a dash in them so I think it makes sense to rename "basicaa" -> "basic-aa". >>> >>> Is there accepted wisdom on renaming pass names? Is a pass name a stable interface? When is it ok to rename a pass? If there are 800 usages of a flag, should I rename them atomically? >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
On Thu, Jun 25, 2020 at 9:59 AM Roman Lebedev <lebedev.ri at gmail.com> wrote:> On Thu, Jun 25, 2020 at 7:48 PM Arthur Eubanks via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > > > After talking with some NPM people, I believe the ultimate goal after > NPM is enabled by default is to only support `-passes=`, and remove support > for `-foo-pass`. > Hm, is there any written rationale behind such a decision? > I would have thought that -passes= is the temporary solution, not the > other way around. >This is really a separate issue that's somewhat orthogonal to the original issue, but someone like asbirlea may be able to chime in more. Maybe a new RFC thread?> > > However, until NPM is enabled by default, we still want tests using opt > to use the legacy PM by default. > > We could attempt to make `-passes=` work with the legacy PM and have a > legacy vs new PM flag, but given the design/syntax of `-passes=` I don't > think that's feasible (see llvm/include/llvm/Passes/PassBuilder.h). > > So for making sure everything works with NPM, I think we need to support > `-foo-pass` in NPM to be able to run all opt tests against NPM. Then at > some point after NPM is enabled by default we can attempt to migrate > everything to `-passes=`. > > Roman. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200625/08bbda59/attachment.html>
On Thu, Jun 25, 2020 at 9:48 AM Arthur Eubanks <aeubanks at google.com> wrote:> After talking with some NPM people, I believe the ultimate goal after NPM > is enabled by default is to only support `-passes=`, and remove support for > `-foo-pass`. > However, until NPM is enabled by default, we still want tests using opt to > use the legacy PM by default. > We could attempt to make `-passes=` work with the legacy PM and have a > legacy vs new PM flag, but given the design/syntax of `-passes=` I don't > think that's feasible (see llvm/include/llvm/Passes/PassBuilder.h). > So for making sure everything works with NPM, I think we need to support > `-foo-pass` in NPM to be able to run all opt tests against NPM. Then at > some point after NPM is enabled by default we can attempt to migrate > everything to `-passes=`. >I think many (but not all) test .ll files already have multiple RUN lines to run with both pass managers. If we do it in every test .ll file, would that work during the transition?> > On Thu, Jun 25, 2020 at 9:22 AM Hiroshi Yamauchi <yamauchi at google.com> > wrote: > >> >> >> On Wed, Jun 24, 2020 at 11:13 AM Arthur Eubanks via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> Hi, >>> >>> As part of new pass manager work, I've been trying to get something like >>> `opt -foo` working under the NPM, where `foo` is the name of a pass. >>> >>> In the past there's been no reason to keep the names of passes >>> consistent between NPM and legacy PM. But now there is a reason to make >>> them match, so that we don't have to touch every single test that uses >>> `opt`. >>> >> >> What's the goal here? Does it include removing the -passes option used by >> the NPM? >> >> >>> >>> There are a couple of names that don't match though, for example the >>> "basic alias analysis" pass is named "basicaa" under the legacy PM >>> INITIALIZE_PASS_BEGIN(BasicAAWrapperPass, "basicaa", >>> "Basic Alias Analysis (stateless AA impl)", true, >>> true) >>> but named "basic-aa" under the NPM >>> FUNCTION_ALIAS_ANALYSIS("basic-aa", BasicAA()) >>> . Almost all the other AA passes have a dash in them so I think it makes >>> sense to rename "basicaa" -> "basic-aa". >>> >>> Is there accepted wisdom on renaming pass names? Is a pass name a stable >>> interface? When is it ok to rename a pass? If there are 800 usages of a >>> flag, should I rename them atomically? >>> _______________________________________________ >>> 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/20200626/8d5044b0/attachment.html>
On Fri, Jun 26, 2020 at 9:19 AM Hiroshi Yamauchi <yamauchi at google.com> wrote:> > > On Thu, Jun 25, 2020 at 9:48 AM Arthur Eubanks <aeubanks at google.com> > wrote: > >> After talking with some NPM people, I believe the ultimate goal after NPM >> is enabled by default is to only support `-passes=`, and remove support for >> `-foo-pass`. >> However, until NPM is enabled by default, we still want tests using opt >> to use the legacy PM by default. >> We could attempt to make `-passes=` work with the legacy PM and have a >> legacy vs new PM flag, but given the design/syntax of `-passes=` I don't >> think that's feasible (see llvm/include/llvm/Passes/PassBuilder.h). >> So for making sure everything works with NPM, I think we need to support >> `-foo-pass` in NPM to be able to run all opt tests against NPM. Then at >> some point after NPM is enabled by default we can attempt to migrate >> everything to `-passes=`. >> > > I think many (but not all) test .ll files already have multiple RUN lines > to run with both pass managers. If we do it in every test .ll file, would > that work during the transition? >I don't think that's much less work than supporting `-foo-pass`, we still have to port all the passes to NPM. The infra for supporting `-foo-pass` is already done (I hope at least) and it reuses the same PassBuilder.parsePassPipeline() interface as `-passes=`. The only extra effort required is making sure pass names match up between the two pass managers. And if we do go with adding `-passes=` to all tests and letting `-foo-pass` run under the legacy PM, then we're sort of saying that everybody should migrate to `-passes=` since otherwise the test would be testing the legacy PM after the NPM switch, and that test's value is diminished. As mentioned before this is a possibility, but a separate concern. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200626/ae49b893/attachment.html>