Re: [PATCH v4 01/33] target/alpha: Tidy up alpha_cpu_class_by_name()

[Gavin Shan <gshan@redhat.com>]

On 11/7/23 00:22, Igor Mammedov wrote:
> On Thu,  2 Nov 2023 10:24:28 +1000
> Gavin Shan <gshan@redhat.com> wrote:
> 
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> For target/alpha, the default CPU model name is "ev67". The default
>> CPU model is used when no matching CPU model is found. The conditions
>> to fall back to the default CPU model can be combined so that the code
>> looks a bit simplified.
> 
> default cpu should be specified by board not by target internals.
> 

Yes, MachineClass::default_cpu_type used to specify the default CPU type.
I will improve the changelog in next revision to avoid the confusion.

>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   target/alpha/cpu.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
>> index 51b7d8d1bf..c7ae4d6a41 100644
>> --- a/target/alpha/cpu.c
>> +++ b/target/alpha/cpu.c
>> @@ -142,13 +142,10 @@ static ObjectClass *alpha_cpu_class_by_name(const char *cpu_model)
>>       typename = g_strdup_printf(ALPHA_CPU_TYPE_NAME("%s"), cpu_model);
>>       oc = object_class_by_name(typename);
>>       g_free(typename);
>> -    if (oc != NULL && object_class_is_abstract(oc)) {
>> -        oc = NULL;
>> -    }
>>   
>>       /* TODO: remove match everything nonsense */
> 
> Let's do ^^^^^ instead of just shifting code around.
> It will break users that specify junk as input, but it's clear
> users error so garbage in => error out.
> 

Ok. The whole chunk of code to fall back to 'ev67' will be dropped
in next revision.

> 
>> -    /* Default to ev67; no reason not to emulate insns by default. */
>> -    if (!oc) {
>> +    if (!oc || object_class_is_abstract(oc)) {
>> +        /* Default to ev67, no reason not to emulate insns by default */
>>           oc = object_class_by_name(ALPHA_CPU_TYPE_NAME("ev67"));
>>       }
>>   

Thanks,
Gavin