| Message ID | 1351652644-18687-3-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On 2012-10-31 14:03, Andreas Färber wrote: > +static const AlphaCPUInfo alpha_cpus[] = { > + { .name = "ev4", .initfn = ev4_cpu_initfn }, > + { .name = "ev5", .initfn = ev5_cpu_initfn }, > + { .name = "ev56", .initfn = ev56_cpu_initfn }, > + { .name = "pca56", .initfn = pca56_cpu_initfn }, > + { .name = "ev6", .initfn = ev6_cpu_initfn }, > + { .name = "ev67", .initfn = ev67_cpu_initfn }, > + { .name = "ev68", .initfn = ev68_cpu_initfn }, > + { .name = "21064", .initfn = alpha_21064_cpu_initfn }, > + { .name = "21164", .initfn = alpha_21164_cpu_initfn }, > + { .name = "21164a", .initfn = alpha_21164a_cpu_initfn }, > + { .name = "21164pc", .initfn = alpha_21164pc_cpu_initfn }, > + { .name = "21264", .initfn = alpha_21264_cpu_initfn }, > + { .name = "21264a", .initfn = alpha_21264a_cpu_initfn }, > +}; The "2*" names are aliases of the "ev*" names. There's no need for so much duplication. And for that matter, "ev68" is no different from "ev67" at the level for which we emulate. In hw, it was more cache and a faster multiply implementation. r~
Am 31.10.2012 05:57, schrieb Richard Henderson: > On 2012-10-31 14:03, Andreas Färber wrote: >> +static const AlphaCPUInfo alpha_cpus[] = { >> + { .name = "ev4", .initfn = ev4_cpu_initfn }, >> + { .name = "ev5", .initfn = ev5_cpu_initfn }, >> + { .name = "ev56", .initfn = ev56_cpu_initfn }, >> + { .name = "pca56", .initfn = pca56_cpu_initfn }, >> + { .name = "ev6", .initfn = ev6_cpu_initfn }, >> + { .name = "ev67", .initfn = ev67_cpu_initfn }, >> + { .name = "ev68", .initfn = ev68_cpu_initfn }, >> + { .name = "21064", .initfn = alpha_21064_cpu_initfn }, >> + { .name = "21164", .initfn = alpha_21164_cpu_initfn }, >> + { .name = "21164a", .initfn = alpha_21164a_cpu_initfn }, >> + { .name = "21164pc", .initfn = alpha_21164pc_cpu_initfn }, >> + { .name = "21264", .initfn = alpha_21264_cpu_initfn }, >> + { .name = "21264a", .initfn = alpha_21264a_cpu_initfn }, >> +}; > > The "2*" names are aliases of the "ev*" names. There's no need for so > much duplication. And for that matter, "ev68" is no different from "ev67" > at the level for which we emulate. In hw, it was more cache and a faster > multiply implementation. Clearly I know little to nothing about Alpha CPU models. :) Regarding ev68, we'll need to carry it for backwards compatibility; can we assume that the Alpha ISA is dead? Then I could drop this shrinking array and make, e.g., ev68 a trivial subclass of ev67. The name scheme we are heading towards now looks like <name>-alpha-cpu. Did I understand you correctly that we would want, e.g., ev4-alpha-cpu as type and have "21064" map to it? Or the other way around? Andreas
On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote: [...] > +static void alpha_cpu_register(const AlphaCPUInfo *info) > +{ > + TypeInfo type_info = { > + .name = info->name, > + .parent = TYPE_ALPHA_CPU, > + .instance_init = info->initfn, > + }; > + > + type_register_static(&type_info); You should use type_register() instead of type_register_static(), here.
Am 06.12.2012 16:29, schrieb Eduardo Habkost: > On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote: > [...] >> +static void alpha_cpu_register(const AlphaCPUInfo *info) >> +{ >> + TypeInfo type_info = { >> + .name = info->name, >> + .parent = TYPE_ALPHA_CPU, >> + .instance_init = info->initfn, >> + }; >> + >> + type_register_static(&type_info); > > You should use type_register() instead of type_register_static(), here. I still don't understand why. (CC'ing Anthony, Paolo, Peter) The TypeInfo argument is in no way retained inside qom/object.c:type_register_internal(). Therefore the lifetime of TypeInfo should be completely irrelevant for the static/non-static decision and the documentation should be fixed IMO. Is there a reason to do it differently? What would we want to do with TypeInfo after transfer of its field values to TypeImpl? FWIW if, as suggested earlier, we don't loop over Alpha CPU models in favor of a handful of trivial static TypeInfos, it becomes irrelevant for this patch but I'd still like to understand for the remaining architectures. Regards, Andreas
On Thu, Dec 06, 2012 at 04:51:31PM +0100, Andreas Färber wrote: > Am 06.12.2012 16:29, schrieb Eduardo Habkost: > > On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote: > > [...] > >> +static void alpha_cpu_register(const AlphaCPUInfo *info) > >> +{ > >> + TypeInfo type_info = { > >> + .name = info->name, > >> + .parent = TYPE_ALPHA_CPU, > >> + .instance_init = info->initfn, > >> + }; > >> + > >> + type_register_static(&type_info); > > > > You should use type_register() instead of type_register_static(), here. > > I still don't understand why. (CC'ing Anthony, Paolo, Peter) > > The TypeInfo argument is in no way retained inside > qom/object.c:type_register_internal(). > Therefore the lifetime of TypeInfo should be completely irrelevant for > the static/non-static decision and the documentation should be fixed IMO. > Is there a reason to do it differently? What would we want to do with > TypeInfo after transfer of its field values to TypeImpl? The current implementation doesn't matter. It can change at any minute. The interface, on the other hand, is documented as: type_register_static: @info: The #TypeInfo of the new type. @info and all of the strings it points to should exist for the life time that the type is registered.
Am 06.12.2012 17:09, schrieb Eduardo Habkost: > On Thu, Dec 06, 2012 at 04:51:31PM +0100, Andreas Färber wrote: >> Am 06.12.2012 16:29, schrieb Eduardo Habkost: >>> On Wed, Oct 31, 2012 at 04:03:59AM +0100, Andreas Färber wrote: >>> [...] >>>> +static void alpha_cpu_register(const AlphaCPUInfo *info) >>>> +{ >>>> + TypeInfo type_info = { >>>> + .name = info->name, >>>> + .parent = TYPE_ALPHA_CPU, >>>> + .instance_init = info->initfn, >>>> + }; >>>> + >>>> + type_register_static(&type_info); >>> >>> You should use type_register() instead of type_register_static(), here. >> >> I still don't understand why. (CC'ing Anthony, Paolo, Peter) >> >> The TypeInfo argument is in no way retained inside >> qom/object.c:type_register_internal(). >> Therefore the lifetime of TypeInfo should be completely irrelevant for >> the static/non-static decision and the documentation should be fixed IMO. >> Is there a reason to do it differently? What would we want to do with >> TypeInfo after transfer of its field values to TypeImpl? > > The current implementation doesn't matter. It can change at any minute. The > interface, on the other hand, is documented as: > > type_register_static: > @info: The #TypeInfo of the new type. > > @info and all of the strings it points to should exist for the life time > that the type is registered. Both implementation and documentation can be changed. My question is, why does the documentation say this and where does Anthony (or Paolo) want to go with the current implementation that makes this necessary. Like, if we switched to C++, we would drop both registration functions completely. Andreas
On 2012-12-06 04:11, Andreas Färber wrote: >> The "2*" names are aliases of the "ev*" names. There's no need for so >> much duplication. And for that matter, "ev68" is no different from "ev67" >> at the level for which we emulate. In hw, it was more cache and a faster >> multiply implementation. > > Clearly I know little to nothing about Alpha CPU models. :) > Regarding ev68, we'll need to carry it for backwards compatibility; can > we assume that the Alpha ISA is dead? Then I could drop this shrinking > array and make, e.g., ev68 a trivial subclass of ev67. Yes, the ISA is dead. We're also nearing the end of "extended" hardware support for Alpha systems from HP. > The name scheme we are heading towards now looks like <name>-alpha-cpu. > Did I understand you correctly that we would want, e.g., ev4-alpha-cpu > as type and have "21064" map to it? Correct. That corresponds more closely to how the compiler tools are configured (e.g. alphaev67-linux, not alpha21164-linux). r~
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c index 11a19eb..e1a5739 100644 --- a/target-alpha/cpu.c +++ b/target-alpha/cpu.c @@ -23,6 +23,145 @@ #include "qemu-common.h" +/* Models */ + +static void ev4_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_2106x; +} + +static void ev5_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21164; +} + +static void ev56_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21164; + env->amask = AMASK_BWX; +} + +static void pca56_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21164; + env->amask = AMASK_BWX | AMASK_MVI; +} + +static void ev6_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21264; + env->amask = AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP; +} + +static void ev67_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21264; + env->amask = AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI | AMASK_TRAP | + AMASK_PREFETCH; +} + +static void ev68_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21264; + env->amask = AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI | AMASK_TRAP | + AMASK_PREFETCH; +} + +static void alpha_21064_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_2106x; +} + +static void alpha_21164_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21164; +} + +static void alpha_21164a_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21164; + env->amask = AMASK_BWX; +} + +static void alpha_21164pc_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21164; + env->amask = AMASK_BWX | AMASK_MVI; +} + +static void alpha_21264_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21264; + env->amask = AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP; +} + +static void alpha_21264a_cpu_initfn(Object *obj) +{ + AlphaCPU *cpu = ALPHA_CPU(obj); + CPUAlphaState *env = &cpu->env; + + env->implver = IMPLVER_21264; + env->amask = AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI | AMASK_TRAP | + AMASK_PREFETCH; +} + +typedef struct AlphaCPUInfo { + const char *name; + void (*initfn)(Object *obj); +} AlphaCPUInfo; + +static const AlphaCPUInfo alpha_cpus[] = { + { .name = "ev4", .initfn = ev4_cpu_initfn }, + { .name = "ev5", .initfn = ev5_cpu_initfn }, + { .name = "ev56", .initfn = ev56_cpu_initfn }, + { .name = "pca56", .initfn = pca56_cpu_initfn }, + { .name = "ev6", .initfn = ev6_cpu_initfn }, + { .name = "ev67", .initfn = ev67_cpu_initfn }, + { .name = "ev68", .initfn = ev68_cpu_initfn }, + { .name = "21064", .initfn = alpha_21064_cpu_initfn }, + { .name = "21164", .initfn = alpha_21164_cpu_initfn }, + { .name = "21164a", .initfn = alpha_21164a_cpu_initfn }, + { .name = "21164pc", .initfn = alpha_21164pc_cpu_initfn }, + { .name = "21264", .initfn = alpha_21264_cpu_initfn }, + { .name = "21264a", .initfn = alpha_21264a_cpu_initfn }, +}; + static void alpha_cpu_initfn(Object *obj) { AlphaCPU *cpu = ALPHA_CPU(obj); @@ -41,18 +180,34 @@ static void alpha_cpu_initfn(Object *obj) env->fen = 1; } +static void alpha_cpu_register(const AlphaCPUInfo *info) +{ + TypeInfo type_info = { + .name = info->name, + .parent = TYPE_ALPHA_CPU, + .instance_init = info->initfn, + }; + + type_register_static(&type_info); +} + static const TypeInfo alpha_cpu_type_info = { .name = TYPE_ALPHA_CPU, .parent = TYPE_CPU, .instance_size = sizeof(AlphaCPU), .instance_init = alpha_cpu_initfn, - .abstract = false, + .abstract = true, .class_size = sizeof(AlphaCPUClass), }; static void alpha_cpu_register_types(void) { + int i; + type_register_static(&alpha_cpu_type_info); + for (i = 0; i < ARRAY_SIZE(alpha_cpus); i++) { + alpha_cpu_register(&alpha_cpus[i]); + } } type_init(alpha_cpu_register_types) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index f707d8d..6ee031d 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -3493,56 +3493,21 @@ void gen_intermediate_code_pc (CPUAlphaState *env, struct TranslationBlock *tb) gen_intermediate_code_internal(env, tb, 1); } -struct cpu_def_t { - const char *name; - int implver, amask; -}; - -static const struct cpu_def_t cpu_defs[] = { - { "ev4", IMPLVER_2106x, 0 }, - { "ev5", IMPLVER_21164, 0 }, - { "ev56", IMPLVER_21164, AMASK_BWX }, - { "pca56", IMPLVER_21164, AMASK_BWX | AMASK_MVI }, - { "ev6", IMPLVER_21264, AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP }, - { "ev67", IMPLVER_21264, (AMASK_BWX | AMASK_FIX | AMASK_CIX - | AMASK_MVI | AMASK_TRAP | AMASK_PREFETCH), }, - { "ev68", IMPLVER_21264, (AMASK_BWX | AMASK_FIX | AMASK_CIX - | AMASK_MVI | AMASK_TRAP | AMASK_PREFETCH), }, - { "21064", IMPLVER_2106x, 0 }, - { "21164", IMPLVER_21164, 0 }, - { "21164a", IMPLVER_21164, AMASK_BWX }, - { "21164pc", IMPLVER_21164, AMASK_BWX | AMASK_MVI }, - { "21264", IMPLVER_21264, AMASK_BWX | AMASK_FIX | AMASK_MVI | AMASK_TRAP }, - { "21264a", IMPLVER_21264, (AMASK_BWX | AMASK_FIX | AMASK_CIX - | AMASK_MVI | AMASK_TRAP | AMASK_PREFETCH), } -}; - -CPUAlphaState * cpu_alpha_init (const char *cpu_model) +CPUAlphaState *cpu_alpha_init(const char *cpu_model) { AlphaCPU *cpu; CPUAlphaState *env; - int implver, amask, i, max; + const char *typename = cpu_model; - cpu = ALPHA_CPU(object_new(TYPE_ALPHA_CPU)); + if (object_class_by_name(typename) == NULL) { + /* Default to ev67; no reason not to emulate insns by default. */ + typename = "ev67"; + } + cpu = ALPHA_CPU(object_new(typename)); env = &cpu->env; alpha_translate_init(); - /* Default to ev67; no reason not to emulate insns by default. */ - implver = IMPLVER_21264; - amask = (AMASK_BWX | AMASK_FIX | AMASK_CIX | AMASK_MVI - | AMASK_TRAP | AMASK_PREFETCH); - - max = ARRAY_SIZE(cpu_defs); - for (i = 0; i < max; i++) { - if (strcmp (cpu_model, cpu_defs[i].name) == 0) { - implver = cpu_defs[i].implver; - amask = cpu_defs[i].amask; - break; - } - } - env->implver = implver; - env->amask = amask; env->cpu_model_str = cpu_model; qemu_init_vcpu(env);
Make TYPE_ALPHA_CPU abstract and default to creating type "ev67". Signed-off-by: Andreas Färber <afaerber@suse.de> --- target-alpha/cpu.c | 157 +++++++++++++++++++++++++++++++++++++++++++++- target-alpha/translate.c | 49 +++------------ 2 Dateien geändert, 163 Zeilen hinzugefügt(+), 43 Zeilen entfernt(-)