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`.
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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200624/bff0691f/attachment.html>
> On Jun 24, 2020, at 14:13, 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`. > > 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?I think the pass naming scheme needs a lot of work. The naming conventions seem random at times. For instance, I can never remember how to refer to PrologEpilogInserter. The DEBUG_TYPE name is “prologepilog”, the pass class name is “PEI”. I would expect this to be prolog-epilog-inserter to match the file and formal pass name, and consistently use dashes as word separators. Unfortunately, I recently discovered some optimization pass remarks expose pass names to the user (since you use the name of the pass to filter relevant remarks). I’m not sure how stable this is supposed to be though. -Matt -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200624/0f2f9cdb/attachment.html>
On 6/24/20 11:21 AM, Matt Arsenault via llvm-dev wrote:> > >> On Jun 24, 2020, at 14:13, Arthur Eubanks via llvm-dev >> <llvm-dev at lists.llvm.org <mailto: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`. >> >> 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? > > I think the pass naming scheme needs a lot of work. The naming > conventions seem random at times. For instance, I can never remember > how to refer to PrologEpilogInserter. The DEBUG_TYPE name is > “prologepilog”, the pass class name is “PEI”. I would expect this to > be prolog-epilog-inserter to match the file and formal pass name, and > consistently use dashes as word separators.Can I suggest we allow aliases? We can except all of these names, pick a canonical name, migrate tests, and only remove the aliases once the new canonical names are widely known.> > Unfortunately, I recently discovered some optimization pass remarks > expose pass names to the user (since you use the name of the pass to > filter relevant remarks). I’m not sure how stable this is supposed to > be though. > > -Matt > > > > _______________________________________________ > 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/20200624/c643969a/attachment.html>
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/43336fce/attachment.html>
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>