| Message ID | 20190419061429.17695-1-ehabkost@redhat.com |
|---|---|
| Headers | show |
| Series | Delete 16 *_cpu_class_by_name() functions | expand |
Eduardo Habkost <ehabkost@redhat.com> writes: > This series adds a new CPUClass::class_name_format field, which > allows us to delete 16 of the 21 *_cpu_class_by_name() functions > that exist today. Which five remain, and why?
On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > This series adds a new CPUClass::class_name_format field, which > > allows us to delete 16 of the 21 *_cpu_class_by_name() functions > > that exist today. > > Which five remain, and why? alpha_cpu_class_by_name: * Translates aliases based on alpha_cpu_aliases; * Falls back to "ev67" unconditionally (there's a "TODO: remove match everything nonsense" comment). cris_cpu_class_by_name: * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY. ppc_cpu_class_by_name: * Supports lookup by PVR if CPU model is a 8 digit hex number; * Converts CPU model to lowercase. superh_cpu_class_by_name: * Translates "any" alias to TYPE_SH7750R_CPU. sparc_cpu_class_by_name: * Replaces whitespaces with '-' on CPU model name.
Eduardo Habkost <ehabkost@redhat.com> writes: > On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote: >> Eduardo Habkost <ehabkost@redhat.com> writes: >> >> > This series adds a new CPUClass::class_name_format field, which >> > allows us to delete 16 of the 21 *_cpu_class_by_name() functions >> > that exist today. >> >> Which five remain, and why? > > alpha_cpu_class_by_name: > * Translates aliases based on alpha_cpu_aliases; > * Falls back to "ev67" unconditionally > (there's a "TODO: remove match everything nonsense" comment). > > cris_cpu_class_by_name: > * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY. > > ppc_cpu_class_by_name: > * Supports lookup by PVR if CPU model is a 8 digit hex number; > * Converts CPU model to lowercase. > > superh_cpu_class_by_name: > * Translates "any" alias to TYPE_SH7750R_CPU. > > sparc_cpu_class_by_name: > * Replaces whitespaces with '-' on CPU model name. I'm of course asking because I wonder whether we can dumb down this CPU naming business to something simpler and more regular. Let's review what we have. For all <TARGET> in target/*: * arm i386 lm32 m68k mips moxie openrisc riscv s390x s390x tricore unicore32 xtensa CPU type name format is <TARGET>_CPU_TYPE_NAME("%s"), which boils down to: - arm lm32 m68k moxie riscv s390x tricore unicore32 xtensa "%s-<TARGET>-cpu" - openrisc "%s-or1k-cpu" - i386 "%s-x86_64-cpu" #ifdef TARGET_X86_64 "%s-i386-cpu" #else - mips "%s-mips64-cpu" #ifdef TARGET_MIPS64 "%s-mips-cpu" #else The %s gets replaced by the user's cpu model. * hppa microblaze nios2 tilegx CPU type name format is <TARGET>-cpu. The user's cpu model seems silently ignored. * alpha cris ppc sh4 sparc No format, using ->class_by_name() - alpha CPU type name format is "%s-alpha-cpu". alpha_cpu_class_by_name() recognizes the full name, the full name without "-alpha-cpu" suffix, and a bunch of aliases. - cris CPU type name format is "%s-cris-cpu". cris_cpu_class_by_name() recognizes the name without the "-cris-cpu" suffix, plus "any" as alias for "crisv32-cris-cpu" #ifdef CONFIG_USER_ONLY (this is the default CPU type for machine "axis-dev88"; the other machine "none" has no default). - ppc CPU type name format is "%s-powerpc64-cpu" #ifdef TARGET_PPC64 "%s-powerpc-cpu" #else ppc_cpu_class_by_name() recognizes the name without the suffix, plus the CPU type's PVR (8 digit hex number), plus a bunch of (case insensitive) aliases. - sh4 CPU type name format is "%s-superh-cpu". superh_cpu_class_by_name() recognizes the name without the suffix, plus "any" as alias for "sh7750r-superh-cpu" (this is the default CPU type for machine "shix"; machines "r2d" defaults to "sh7751r", and "none" has no default). - sparc CPU type name format is "%s-sparc64-cpu" #ifdef TARGET_SPARC64 "%s-sparc-cpu" #else sparc_cpu_class_by_name() recognizes the name without the suffix, mapping any spaces in the user's cpu model to '-'. Observations: * The CPU type name format is generally "%s-T-cpu", where T is either <TARGET> or <TARGET>64. Exceptions: - openrisc, sh4 uses or1k, superh instead. Looks pointless to me. - i386 uses x86_64 instead of i38664. Makes sense. - hppa, microblaze, nios2 and tilegx use CPU type name format "T-cpu", ignoring the user's cpu model. These exceptions looks pointless to me. * The user's CPU model is generally the "%s" part of the format. Exceptions: - alpha additionaly recognizes full type names. If that's useful for alpha (I'm not sure it is), why isn't it useful for all other targets? - cris and sh4 additionaly recognize an "any" alias, cris only #ifdef CONFIG_USER_ONLY. Until PATCH 4, arm also recognizes an "any" alias #ifdef CONFIG_USER_ONLY. PATCH 4 drops that, because it's redundant with the "any" CPU, which is a copy instead of an alias. Sure we want to do have different targets do "any" in different ways? See aliases below. - ppc additionaly recognizes PVR aliases and additional (case insensitive) aliases. Feels overengineered to me. See aliases below. - sparc additionally recognizes aliases with ' ' instead of '-'. Feels pointless to me. See aliases below. * What about deprecating pointless exceptions? * Aliases We have several targets roll their own CPU name aliases code. Assuming aliases are here to stay (i.e. we're not deprecating all of them): what about letting each CPU type specify a set of aliases, so we can recognize them in generic code?
On Wed, May 08, 2019 at 10:34:44AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote: > >> Eduardo Habkost <ehabkost@redhat.com> writes: > >> > >> > This series adds a new CPUClass::class_name_format field, which > >> > allows us to delete 16 of the 21 *_cpu_class_by_name() functions > >> > that exist today. > >> > >> Which five remain, and why? > > > > alpha_cpu_class_by_name: > > * Translates aliases based on alpha_cpu_aliases; > > * Falls back to "ev67" unconditionally > > (there's a "TODO: remove match everything nonsense" comment). > > > > cris_cpu_class_by_name: > > * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY. > > > > ppc_cpu_class_by_name: > > * Supports lookup by PVR if CPU model is a 8 digit hex number; > > * Converts CPU model to lowercase. > > > > superh_cpu_class_by_name: > > * Translates "any" alias to TYPE_SH7750R_CPU. > > > > sparc_cpu_class_by_name: > > * Replaces whitespaces with '-' on CPU model name. > > I'm of course asking because I wonder whether we can dumb down this CPU > naming business to something simpler and more regular. We can, but that's not on my list of priorities. Any volunteers? > [...] > * Aliases > > We have several targets roll their own CPU name aliases code. > Assuming aliases are here to stay (i.e. we're not deprecating all of > them): what about letting each CPU type specify a set of aliases, so > we can recognize them in generic code? Yes. I considered adding alias support to generic code, but decided to do this one step at a time.
Eduardo Habkost <ehabkost@redhat.com> writes: > On Wed, May 08, 2019 at 10:34:44AM +0200, Markus Armbruster wrote: >> Eduardo Habkost <ehabkost@redhat.com> writes: >> >> > On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote: >> >> Eduardo Habkost <ehabkost@redhat.com> writes: >> >> >> >> > This series adds a new CPUClass::class_name_format field, which >> >> > allows us to delete 16 of the 21 *_cpu_class_by_name() functions >> >> > that exist today. >> >> >> >> Which five remain, and why? >> > >> > alpha_cpu_class_by_name: >> > * Translates aliases based on alpha_cpu_aliases; >> > * Falls back to "ev67" unconditionally >> > (there's a "TODO: remove match everything nonsense" comment). >> > >> > cris_cpu_class_by_name: >> > * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY. >> > >> > ppc_cpu_class_by_name: >> > * Supports lookup by PVR if CPU model is a 8 digit hex number; >> > * Converts CPU model to lowercase. >> > >> > superh_cpu_class_by_name: >> > * Translates "any" alias to TYPE_SH7750R_CPU. >> > >> > sparc_cpu_class_by_name: >> > * Replaces whitespaces with '-' on CPU model name. >> >> I'm of course asking because I wonder whether we can dumb down this CPU >> naming business to something simpler and more regular. > > We can, but that's not on my list of priorities. Any volunteers? Fair enough. Except for... >> > [...] >> Observations: >> >> * The CPU type name format is generally "%s-T-cpu", where T is either >> <TARGET> or <TARGET>64. >> >> Exceptions: >> >> - openrisc, sh4 uses or1k, superh instead. Looks pointless to me. >> >> - i386 uses x86_64 instead of i38664. Makes sense. >> >> - hppa, microblaze, nios2 and tilegx use CPU type name format "T-cpu", >> ignoring the user's cpu model. These exceptions looks pointless to >> me. >> >> * The user's CPU model is generally the "%s" part of the format. >> >> Exceptions: >> >> - alpha additionaly recognizes full type names. If that's useful for >> alpha (I'm not sure it is), why isn't it useful for all other >> targets? >> >> - cris and sh4 additionaly recognize an "any" alias, cris only #ifdef >> CONFIG_USER_ONLY. >> >> Until PATCH 4, arm also recognizes an "any" alias #ifdef >> CONFIG_USER_ONLY. PATCH 4 drops that, because it's redundant with >> the "any" CPU, which is a copy instead of an alias. Sure we want to >> do have different targets do "any" in different ways? >> >> See aliases below. >> >> - ppc additionaly recognizes PVR aliases and additional (case >> insensitive) aliases. Feels overengineered to me. See aliases >> below. >> >> - sparc additionally recognizes aliases with ' ' instead of '-'. >> Feels pointless to me. See aliases below. ... this, perhaps: >> * What about deprecating pointless exceptions? Deprecating unwanted stuff now is likely to make a later cleanup so much easier. >> * Aliases >> >> We have several targets roll their own CPU name aliases code. >> Assuming aliases are here to stay (i.e. we're not deprecating all of >> them): what about letting each CPU type specify a set of aliases, so >> we can recognize them in generic code? > > Yes. I considered adding alias support to generic code, but > decided to do this one step at a time. Okay. Consider adding suitable TODO comments.
On Wed, 08 May 2019 10:34:44 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote: > >> Eduardo Habkost <ehabkost@redhat.com> writes: > >> > >> > This series adds a new CPUClass::class_name_format field, which > >> > allows us to delete 16 of the 21 *_cpu_class_by_name() functions > >> > that exist today. > >> > >> Which five remain, and why? > > > > alpha_cpu_class_by_name: > > * Translates aliases based on alpha_cpu_aliases; > > * Falls back to "ev67" unconditionally > > (there's a "TODO: remove match everything nonsense" comment). > > > > cris_cpu_class_by_name: > > * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY. > > > > ppc_cpu_class_by_name: > > * Supports lookup by PVR if CPU model is a 8 digit hex number; > > * Converts CPU model to lowercase. > > > > superh_cpu_class_by_name: > > * Translates "any" alias to TYPE_SH7750R_CPU. > > > > sparc_cpu_class_by_name: > > * Replaces whitespaces with '-' on CPU model name. > > I'm of course asking because I wonder whether we can dumb down this CPU > naming business to something simpler and more regular. > > Let's review what we have. > > For all <TARGET> in target/*: > > * arm i386 lm32 m68k mips moxie openrisc riscv s390x s390x tricore > unicore32 xtensa > > CPU type name format is <TARGET>_CPU_TYPE_NAME("%s"), which boils down > to: > > - arm lm32 m68k moxie riscv s390x tricore unicore32 xtensa > "%s-<TARGET>-cpu" > > - openrisc > "%s-or1k-cpu" > > - i386 > "%s-x86_64-cpu" #ifdef TARGET_X86_64 > "%s-i386-cpu" #else > > - mips > "%s-mips64-cpu" #ifdef TARGET_MIPS64 > "%s-mips-cpu" #else > > The %s gets replaced by the user's cpu model. > > * hppa microblaze nios2 tilegx > > CPU type name format is <TARGET>-cpu. The user's cpu model seems > silently ignored. > > * alpha cris ppc sh4 sparc > > No format, using ->class_by_name() > > - alpha > > CPU type name format is "%s-alpha-cpu". > > alpha_cpu_class_by_name() recognizes the full name, the full name > without "-alpha-cpu" suffix, and a bunch of aliases. > > - cris > > CPU type name format is "%s-cris-cpu". > > cris_cpu_class_by_name() recognizes the name without the "-cris-cpu" > suffix, plus "any" as alias for "crisv32-cris-cpu" #ifdef > CONFIG_USER_ONLY (this is the default CPU type for machine > "axis-dev88"; the other machine "none" has no default). > > - ppc > > CPU type name format is > "%s-powerpc64-cpu" #ifdef TARGET_PPC64 > "%s-powerpc-cpu" #else > > ppc_cpu_class_by_name() recognizes the name without the suffix, plus > the CPU type's PVR (8 digit hex number), plus a bunch of (case > insensitive) aliases. > > - sh4 > > CPU type name format is "%s-superh-cpu". > > superh_cpu_class_by_name() recognizes the name without the suffix, > plus "any" as alias for "sh7750r-superh-cpu" (this is the default > CPU type for machine "shix"; machines "r2d" defaults to "sh7751r", > and "none" has no default). > > - sparc > > CPU type name format is > "%s-sparc64-cpu" #ifdef TARGET_SPARC64 > "%s-sparc-cpu" #else > > sparc_cpu_class_by_name() recognizes the name without the suffix, > mapping any spaces in the user's cpu model to '-'. > > Observations: > > * The CPU type name format is generally "%s-T-cpu", where T is either > <TARGET> or <TARGET>64. > > Exceptions: > > - openrisc, sh4 uses or1k, superh instead. Looks pointless to me. > > - i386 uses x86_64 instead of i38664. Makes sense. > > - hppa, microblaze, nios2 and tilegx use CPU type name format "T-cpu", > ignoring the user's cpu model. These exceptions looks pointless to > me. > > * The user's CPU model is generally the "%s" part of the format. > > Exceptions: > > - alpha additionaly recognizes full type names. If that's useful for > alpha (I'm not sure it is), why isn't it useful for all other > targets? > > - cris and sh4 additionaly recognize an "any" alias, cris only #ifdef > CONFIG_USER_ONLY. > > Until PATCH 4, arm also recognizes an "any" alias #ifdef > CONFIG_USER_ONLY. PATCH 4 drops that, because it's redundant with > the "any" CPU, which is a copy instead of an alias. Sure we want to > do have different targets do "any" in different ways? > > See aliases below. > > - ppc additionaly recognizes PVR aliases and additional (case > insensitive) aliases. Feels overengineered to me. See aliases > below. > > - sparc additionally recognizes aliases with ' ' instead of '-'. > Feels pointless to me. See aliases below. > > * What about deprecating pointless exceptions? Last time I was refactoring all this CPU name thingy, there wasn't way to deprecate anything so I had to keep it. But I'd support deprecation in there now. The same applies to aliases, sometimes its code even too complicated (ppc). I'd ditch it if possible. (added in loop PPC folks for opinions) > > * Aliases > > We have several targets roll their own CPU name aliases code. > Assuming aliases are here to stay (i.e. we're not deprecating all of > them): what about letting each CPU type specify a set of aliases, so > we can recognize them in generic code? >