[0/7] Delete 16 *_cpu_class_by_name() functions
mbox series

Message ID 20190419061429.17695-1-ehabkost@redhat.com
Headers show
Series
  • Delete 16 *_cpu_class_by_name() functions
Related show

Message

Eduardo Habkost April 19, 2019, 6:14 a.m. UTC
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.

Eduardo Habkost (7):
  cpu: Change return type of cpu_class_by_name() to CPUClass
  riscv: Don't split CPU model string
  arm: Don't split CPU model string
  arm: Remove special case for "any" CPU model
  cpu: Let architectures set CPU class name format
  cpu: Set class name format for some architectures
  cpu: Set fixed class name on some architectures

 include/qom/cpu.h         | 14 +++++++++++++-
 target/s390x/internal.h   |  1 -
 exec.c                    |  8 +++-----
 qom/cpu.c                 | 20 +++++++++++++++++---
 target/arm/cpu.c          | 30 +-----------------------------
 target/hppa/cpu.c         |  8 ++------
 target/i386/cpu.c         | 11 +----------
 target/lm32/cpu.c         | 17 +----------------
 target/m68k/cpu.c         | 17 +----------------
 target/microblaze/cpu.c   |  8 ++------
 target/mips/cpu.c         | 13 +------------
 target/moxie/cpu.c        | 17 +----------------
 target/nios2/cpu.c        |  8 ++------
 target/openrisc/cpu.c     | 17 +----------------
 target/riscv/cpu.c        | 20 +-------------------
 target/s390x/cpu.c        |  2 +-
 target/s390x/cpu_models.c | 20 +++++---------------
 target/tilegx/cpu.c       |  8 ++------
 target/tricore/cpu.c      | 17 +----------------
 target/unicore32/cpu.c    | 17 +----------------
 target/xtensa/cpu.c       | 17 +----------------
 21 files changed, 58 insertions(+), 232 deletions(-)

Comments

Markus Armbruster May 6, 2019, 11:53 a.m. UTC | #1
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?
Eduardo Habkost May 6, 2019, 7:53 p.m. UTC | #2
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.
Markus Armbruster May 8, 2019, 8:34 a.m. UTC | #3
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?
Eduardo Habkost May 8, 2019, 7:46 p.m. UTC | #4
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.
Markus Armbruster May 9, 2019, 5:55 a.m. UTC | #5
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.
Igor Mammedov May 9, 2019, 3:46 p.m. UTC | #6
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?
>