I'm not sure if there's a lot of value in having a very strictly set defined set of tags. That's sort of (but not entirely) like having a fixed set of email subject lines. Sure, it makes it easier to group things, but not everything will fit in the predefined categories. I think needless differences (arch-sparc vs. sparc vs. whatever) would be nice to get rid of, but I think this should maybe more of a warning thing than a hard barrier.
And if somebody ignores the warnings/suggestions for no good reason, that's what reviews are for :-). Gabe On Tue, Oct 22, 2019 at 12:37 PM Bobby Bruce <[email protected]> wrote: > It seems like there may be some value in adding "systemc" as an official > tag, but, from what I can see, most of the other incorrect tags are mapped > pretty cleanly to existing ones. > > Would anyone have any objection to adding "systemc" as a tag in > MAINTAINERS? > > In regards to server-side hooks: it sounds like it'd be good but a > client-side hook also has value, so I'm still ok with either. > > Kind regards, > Bobby > > -- > Dr. Bobby R. Bruce > Room 2235, > Kemper Hall, UC Davis > Davis, > CA, 95616 > ________________________________ > From: gem5-dev <[email protected]> on behalf of Daniel Carvalho < > [email protected]> > Sent: Saturday, October 19, 2019 6:38 AM > To: gem5 Developer List <[email protected]> > Subject: Re: [gem5-dev] gem5 tags rework > > Thank you for the info, Nikos. Here are the updated values (2400 commits, > around May of 2017, a few days after the MAINTAINERS file creation): > > {'systemc': 466, 'arch-arm': 251, 'mem-cache': 193, 'cpu': 142, 'mem': > 128, 'dev-arm': 127, 'base': 108, 'arm': 97, 'sim': 84, 'tests': 83, 'dev': > 68, 'mem-ruby': 63, 'python': 62, 'arch': 60, 'x86': 59, 'scons': 57, > 'configs': 53, 'sim-se': 52, 'config': 45, 'misc': 44, 'arch-riscv': 43, > 'util': 32, 'sparc': 26, 'cpu-o3': 23, 'riscv': 20, 'arch-x86': 19, 'ext': > 18, 'stats': 14, 'ruby': 14, 'alpha': 13, 'kvm': 13, 'mips': 13, > 'system-arm': 12, 'power': 12, 'learning_gem5': 11, 'syscall_emul': 11, > 'gpu-compute': 10, 'fastmodel': 10, 'style': 7, 'ps2': 7, 'tlm': 5, 'kern': > 4, 'arch-mips': 3, 'null': 3, 'cpu-minor': 3, 'gpu': 3, 'net': 3, > 'arch-power': 3, 'hsail': 3, 'arch-hsail': 2, 'cpu-tester': 2, 'pwr': 2, > 'hsail-x86': 2, 'virtio': 2, 'isa': 2, 'arch-alpha': 1, 'o3': 1, > 'arch-sparc': 1, 'testlib': 1, 'probe': 1, 'gdb': 1, 'sconstruct': 1, > 'vnc': 1, 'cpu-kvm': 1, 'm5': 1, 'testers': 1, 'test': 1, 'hmc': 1, 'docs': > 1, 'mem-garnet': 1, 'arch-generic': 1, 'learning-gem5': 1, 'tarch': 1, > 'pl011': 1} > > > Matches: > > {'arch-arm': 251, 'mem-cache': 193, 'cpu': 142, 'mem': 128, 'base': 108, > 'sim': 84, 'tests': 83, 'dev': 68, 'mem-ruby': 63, 'python': 62, 'arch': > 60, 'scons': 57, 'configs': 53, 'sim-se': 51, 'misc': 44, 'arch-riscv': 43, > 'util': 31, 'cpu-o3': 23, 'arch-x86': 19, 'ext': 18, 'stats': 14, > 'system-arm': 12, 'gpu-compute': 10, 'arch-mips': 3, 'arch-power': 3, > 'cpu-minor': 3, 'arch-hsail': 2, 'arch-alpha': 1, 'cpu-kvm': 1, > 'arch-sparc': 1, 'mem-garnet': 1, 'learning-gem5': 1} > > Unused tags: > > ['dev-virtio', 'system-alpha', 'sim-power', 'system', 'cpu-simple'] > > Although the pattern remains, and there is still a significant amount of > non-'arch-' tags, having the 'arch-' prefix would solve the 'power' > ambiguity issue, and would comply with this recent decision, and the > directory names. Therefore, I suggest keeping the prefix. If this is > adopted, IIRC, 'ruby' would be the only tag that does not comply with > directory names, therefore 'mem-ruby' should be enforced instead. > > > A server side check is still not being done (just started reading about > the hooks), but it is doable, and way more desirable than a client side, > since it can be easily skipped. > > > Regards, > Daniel > Em sábado, 19 de outubro de 2019 13:47:20 GMT+2, Nikos Nikoleris < > [email protected]> escreveu: > > Hi Daniel, > > If I remember correctly, the rules on commit tags changed about 2.5 > years ago (see also git log MAINTAINERS) along with the migration from > mercurial to git. Previously tags such as arch-arm and arch-x86 were > simply arm and x86. In addition tags such as dev-arm, systemc were added > more recently as the result of the creation (or refactoring) of a new > subsystem. > > I think you might have to limit the window in your analysis to see if > and what tags we use consistently. > > It would be nice if we could do this check and other style checks > through a commit hook or a server side check and not to have to rely on > reviews. > > Regards, > > Nikos > > > On 19/10/2019 06:54, Daniel Carvalho wrote: > > Hello all, > > > > I have recently proposed adding a git hook ( > https://gem5-review.googlesource.com/c/public/gem5/+/21739) to make sure > that commit messages follow the guidelines in CONTRIBUTING, with the > uttermost goal being to make sure that the gem5 tags used are valid. I have > also checked if the previous commits would have been valid if this hook had > been applied, and just a tiny percentage of them would, > > Although typos were frequent, the main issue was that the tags in > MAINTAINERS do not correspond to reality, and many other tags were being > used instead. > > > > I have done a quick check on all used tags since around 2013 (last 5000 > commits) to check which and how many times each tag has been used. I have > manually removed some noise (fixed simple tag typos, and ignored grotesque > outliers, which were less than 0.1% of the tags), and all tags were treated > as lowercase. This is the result: > > > > {'mem': 575, 'systemc': 466, 'cpu': 368, 'arm': 347, 'ruby': 327, > 'arch-arm': 251, 'sim': 214, 'stats': 194, 'mem-cache': 193, 'config': 193, > 'dev': 192, 'base': 187, 'x86': 172, 'tests': 144, 'scons': 142, 'dev-arm': > 127, 'misc': 113, 'arch': 93, 'kvm': 79, 'python': 73, 'util': 70, > 'configs': 67, 'mem-ruby': 63, 'syscall_emul': 53, 'sim-se': 52, 'style': > 48, 'ext': 48, 'gpu-compute': 46, 'arch-riscv': 43, 'riscv': 35, 'sparc': > 32, 'power': 26, 'cpu-o3': 23, 'alpha': 21, 'arch-x86': 19, 'mips': 18, > 'slicc': 17, 'hsail': 16, 'regressions': 13, 'system-arm': 12, 'o3': 11, > 'learning_gem5': 11, 'syscall-emul': 10, 'fastmodel': 10, 'isa': 8, '': 7, > 'ps2': 7, 'kern': 7, 'test': 6, 'dist': 5, 'tlm': 5, 'pwr': 5, 'cache': 4, > 'energy': 4, 'gpu': 4, 'probe': 3, 'virtio': 3, 'arch-mips': 3, 'cpu/o3': > 3, 'net': 3, 'cpu-minor': 3, 'syscall emulation': 3, 'revert "cpu': 3, > 'arch-power': 3, 'null': 3, 'build': 3, 'copyright': 2, 'swig': 2, > 'arch-sparc': 2, 'x86 regressions': 2, 'cpu-tester': 2, 'loader': 2, > 'arch-hsail': 2, 'cpuid': 2, 'minor': 2, 'o3 cpu': 2, 'syscall': 2, > 'proto': 2, 'vnc': 2, 'hsail-x86': 2, 'arch-alpha': 1, 'base simple cpu': > 1, 'testlib': 1, 'gdb': 1, 'pci': 1, 'docs': 1, 'tarch': 1, 'x86 cpuid': 1, > 'testers': 1, 'debug': 1, 'util/regress': 1, 'branch predictor': 1, > 'devices': 1, 'arch/x86': 1, 'kmi': 1, 'x86 isa': 1, 'cpu. arch': 1, 'sym': > 1, 'rcs scripts': 1, 'regress': 1, 'ide': 1, 'cache recorder': 1, 'o3 iew': > 1, 'unittest': 1, 'pseudo inst': 1, 'build opts': 1, 'pl011': 1, 'hmc': 1, > 'decoder': 1, 'learning-gem5': 1, 'o3cpu': 1, 'sconstruct': 1, 'cpu-kvm': > 1, 'ruby sequencer': 1, 'ext lib': 1, 'mem-garnet': 1, 'arch-generic': 1, > 'options': 1} > > > > Matching against the current tags existing in maintainers (not including > dev-arm, which shall be added soon), this is the list: > > > > {'mem': 575, 'cpu': 367, 'arch-arm': 251, 'sim': 214, 'stats': 194, > 'mem-cache': 193, 'dev': 192, 'base': 187, 'tests': 144, 'scons': 142, > 'misc': 113, 'arch': 93, 'python': 73, 'util': 70, 'configs': 67, > 'mem-ruby': 63, 'sim-se': 51, 'ext': 48, 'gpu-compute': 46, 'arch-riscv': > 43, 'cpu-o3': 23, 'arch-x86': 19, 'system-arm': 12, 'arch-mips': 3, > 'arch-power': 3, 'cpu-minor': 3, 'arch-hsail': 2, 'arch-sparc': 2, > 'arch-alpha': 1, 'cpu-kvm': 1, 'mem-garnet': 1, 'learning-gem5': 1} > > > > Therefore these gem5 tags have not been used in a long time: > > > > {'dev-virtio', 'system-alpha', 'sim-power', 'system', 'cpu-simple'} > > > > 'dev-virtio' has been used as 'virtio' (3) > > 'system-alpha' has been used as 'alpha' (21) and 'arch-alpha' (1) > > > > 'power' (26) has both been used as 'sim-power' and 'arch-power', which > are completely different > > > > 'system' and 'cpu-simple' seem to have been substituted by other tags > > > > ============================================= > > > > > > Further analysis: > > > > Except for arm and riscv, which had tied results, there seems to exist a > preference to not include "arch-" in ISA specific commits, so I think they > should all be replaced, i.e., 'arch-x86' becomes 'x86', 'arch-hsail' > becomes 'hsail' and so on, but I also agree with the counterargument that > these tags reflect the path to the dir, and therefore 'arch-' must be > present. > > > > The same applies to "mem-ruby", which is constantly used as "ruby". > > Maybe 'learning-gem5' should be move to 'misc' due to their low > frequency. Also, move 'mem-garnet' to 'misc', since although it belongs to > 'mem' I do not know if the maintainer is familiar with it. > > The usage of 'misc' has been fair, overall, although sometimes a new tag > 'style' has been used instead, and other times other tags should have been > applied instead. > > 'dev-virtio' is very rare, so it could be incorporated by 'dev'. > > 'cpu-minor' is very rare, so it could be incorporated by 'cpu' > > > > There is no maintainer for 'cpu-o3'. Does it really need to be apart > from 'cpu'? > > > > I do not know what to think of the 'system', 'system-alpha', and > 'system-arm' tags. > > ============================================= > > > > > > Given all that information, I propose remodeling the list of tags. This > is what I am thinking: > > > > [alpha, arch, arm, base, configs, cpu, cpu-o3, dev, dev-arm, ext, > gpu-compute, hsail, kvm, learning-gem5, mem, mem-cache, mips, misc, power, > python, riscv, ruby, scons, sim, sim-se, sim-power, slicc, sparc, stats, > system, system-arm, systemc, tests, util, x86] > > > > Of course, this is mostly from a statistical point of view, so it may > not reflect the knowledge nor will of the maintainers, so I ask you: what > do you think? > > > > > > Regards, > > Daniel > > _______________________________________________ > > gem5-dev mailing list > > [email protected] > > http://m5sim.org/mailman/listinfo/gem5-dev > > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
