> On Jun 24, 2020, at 19:17, Arthur Eubanks <aeubanks at google.com> wrote: > > > > On Wed, Jun 24, 2020 at 12:23 PM Philip Reames <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote: > > > 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. > An alias sounds good.In what contexts? I think aliases for the handful of potentially end user facing cases may be acceptable, but would worry about adding aliases everywhere. I think renaming things directly referring to a pass can be done pretty easily with a simple script? -Matt -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200624/38ba5980/attachment.html>
On Wed, Jun 24, 2020 at 4:48 PM Matt Arsenault <arsenm2 at gmail.com> wrote:> > > On Jun 24, 2020, at 19:17, Arthur Eubanks <aeubanks at google.com> wrote: > > > > On Wed, Jun 24, 2020 at 12:23 PM Philip Reames <listmail at philipreames.com> > wrote: > >> >> 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> 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. >> > An alias sounds good. > > > In what contexts? I think aliases for the handful of potentially end user > facing cases may be acceptable, but would worry about adding aliases > everywhere. I think renaming things directly referring to a pass can be > done pretty easily with a simple script? >My plan was to add an alias in PassNameParser, switch over all references to use the new one, then remove the alias. In order to avoid one change that touches 800 files. Also any rollbacks are easier.> > -Matt > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200624/5ef9a829/attachment-0001.html>
On 2020-06-24, Arthur Eubanks via llvm-dev wrote:>On Wed, Jun 24, 2020 at 4:48 PM Matt Arsenault <arsenm2 at gmail.com> wrote: > >> >> >> On Jun 24, 2020, at 19:17, Arthur Eubanks <aeubanks at google.com> wrote: >> >> >> >> On Wed, Jun 24, 2020 at 12:23 PM Philip Reames <listmail at philipreames.com> >> wrote: >> >>> >>> 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> 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. >>> >> An alias sounds good. >> >> >> In what contexts? I think aliases for the handful of potentially end user >> facing cases may be acceptable, but would worry about adding aliases >> everywhere. I think renaming things directly referring to a pass can be >> done pretty easily with a simple script? >> >My plan was to add an alias in PassNameParser, switch over all references >to use the new one, then remove the alias. In order to avoid one change >that touches 800 files. Also any rollbacks are easier.An alias facility in PassNameParser sounds good. Dropping the alias can be a separate change so that rollback will be easier.