diff mbox series

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

Message ID 20231102002500.1750692-2-gshan@redhat.com
State New
Headers show
Series [v4,01/33] target/alpha: Tidy up alpha_cpu_class_by_name() | expand

Commit Message

Gavin Shan Nov. 2, 2023, 12:24 a.m. UTC 
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.

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(-)

Comments

Igor Mammedov Nov. 6, 2023, 2:22 p.m. UTC | #1 
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.

> 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.


> -    /* 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"));
>      }
>
Gavin Shan Nov. 13, 2023, 11:23 p.m. UTC | #2 
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
diff mbox series

Patch

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 */
-    /* 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"));
     }