[v1] target/alpha: Access CPUState::cpu_index via helper

[PATCH] target/alpha: Access CPUState::cpu_index via helper

Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago

CPUState::cpu_index is a target agnostic field, meant
for common code (i.e. accel/ and system/ folders).

Target specific code should use the CPUClass::get_arch_id()
helper, even if there is a 1:1 mapping.

In preparation of generic changes around CPU indexing,
introduce the whoami helper to access the generic
CPUState::cpu_index field.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/alpha/helper.h     | 1 +
 target/alpha/sys_helper.c | 5 +++++
 target/alpha/translate.c  | 3 +--
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/alpha/helper.h b/target/alpha/helper.h
index d60f2087031..604af4213c9 100644
--- a/target/alpha/helper.h
+++ b/target/alpha/helper.h
@@ -93,6 +93,7 @@ DEF_HELPER_FLAGS_2(tbis, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(tb_flush, TCG_CALL_NO_RWG, void, env)
 
 DEF_HELPER_1(halt, void, i64)
+DEF_HELPER_1(whoami, i64, env)
 
 DEF_HELPER_FLAGS_0(get_vmtime, TCG_CALL_NO_RWG, i64)
 DEF_HELPER_FLAGS_0(get_walltime, TCG_CALL_NO_RWG, i64)
diff --git a/target/alpha/sys_helper.c b/target/alpha/sys_helper.c
index 51e32544287..a757a558900 100644
--- a/target/alpha/sys_helper.c
+++ b/target/alpha/sys_helper.c
@@ -73,3 +73,8 @@ void helper_set_alarm(CPUAlphaState *env, uint64_t expire)
         timer_del(cpu->alarm_timer);
     }
 }
+
+uint64_t HELPER(whoami)(CPUAlphaState *env)
+{
+    return env_cpu(env)->cpu_index;
+}
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index cebab0318cf..3e01bb36efa 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -1128,8 +1128,7 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int palcode)
             break;
         case 0x3C:
             /* WHAMI */
-            tcg_gen_ld32s_i64(ctx->ir[IR_V0], tcg_env,
-                -offsetof(AlphaCPU, env) + offsetof(CPUState, cpu_index));
+            gen_helper_whoami(ctx->ir[IR_V0], tcg_env);
             break;
 
         case 0x3E:
-- 
2.51.0

Re: [PATCH] target/alpha: Access CPUState::cpu_index via helper

Posted by Richard Henderson 4 months, 2 weeks ago

On 9/24/25 10:01, Philippe Mathieu-Daudé wrote:
> CPUState::cpu_index is a target agnostic field, meant
> for common code (i.e. accel/ and system/ folders).
> 
> Target specific code should use the CPUClass::get_arch_id()
> helper, even if there is a 1:1 mapping.
> 
> In preparation of generic changes around CPU indexing,
> introduce the whoami helper to access the generic
> CPUState::cpu_index field.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/alpha/helper.h     | 1 +
>   target/alpha/sys_helper.c | 5 +++++
>   target/alpha/translate.c  | 3 +--
>   3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/target/alpha/helper.h b/target/alpha/helper.h
> index d60f2087031..604af4213c9 100644
> --- a/target/alpha/helper.h
> +++ b/target/alpha/helper.h
> @@ -93,6 +93,7 @@ DEF_HELPER_FLAGS_2(tbis, TCG_CALL_NO_RWG, void, env, i64)
>   DEF_HELPER_FLAGS_1(tb_flush, TCG_CALL_NO_RWG, void, env)
>   
>   DEF_HELPER_1(halt, void, i64)
> +DEF_HELPER_1(whoami, i64, env)

The PALcode function name doesn't contain the 'O'.

Hooray for 1970's abbreviations still hanging on:
http://www.bitsavers.org/pdf/dec/pdp11/1160/AA-C815A-TC_1160_MIcroprogramming_Tools_V1.0_Nov77.pdf

I think WHAMI was a pdp10 instruction even before that, but I can't find docs.  :-)

>   
>   DEF_HELPER_FLAGS_0(get_vmtime, TCG_CALL_NO_RWG, i64)
>   DEF_HELPER_FLAGS_0(get_walltime, TCG_CALL_NO_RWG, i64)
> diff --git a/target/alpha/sys_helper.c b/target/alpha/sys_helper.c
> index 51e32544287..a757a558900 100644
> --- a/target/alpha/sys_helper.c
> +++ b/target/alpha/sys_helper.c
> @@ -73,3 +73,8 @@ void helper_set_alarm(CPUAlphaState *env, uint64_t expire)
>           timer_del(cpu->alarm_timer);
>       }
>   }
> +
> +uint64_t HELPER(whoami)(CPUAlphaState *env)
> +{
> +    return env_cpu(env)->cpu_index;
> +}

It's a correct transformation of the current code,
though hard to evaluate without further context.

Aside from the extra 'O',
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH] target/alpha: Access CPUState::cpu_index via helper

Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago

On 24/9/25 21:30, Richard Henderson wrote:
> On 9/24/25 10:01, Philippe Mathieu-Daudé wrote:
>> CPUState::cpu_index is a target agnostic field, meant
>> for common code (i.e. accel/ and system/ folders).
>>
>> Target specific code should use the CPUClass::get_arch_id()
>> helper, even if there is a 1:1 mapping.
>>
>> In preparation of generic changes around CPU indexing,
>> introduce the whoami helper to access the generic
>> CPUState::cpu_index field.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/alpha/helper.h     | 1 +
>>   target/alpha/sys_helper.c | 5 +++++
>>   target/alpha/translate.c  | 3 +--
>>   3 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/alpha/helper.h b/target/alpha/helper.h
>> index d60f2087031..604af4213c9 100644
>> --- a/target/alpha/helper.h
>> +++ b/target/alpha/helper.h
>> @@ -93,6 +93,7 @@ DEF_HELPER_FLAGS_2(tbis, TCG_CALL_NO_RWG, void, env, 
>> i64)
>>   DEF_HELPER_FLAGS_1(tb_flush, TCG_CALL_NO_RWG, void, env)
>>   DEF_HELPER_1(halt, void, i64)
>> +DEF_HELPER_1(whoami, i64, env)
> 
> The PALcode function name doesn't contain the 'O'.
> 
> Hooray for 1970's abbreviations still hanging on:
> http://www.bitsavers.org/pdf/dec/pdp11/1160/AA-C815A- 
> TC_1160_MIcroprogramming_Tools_V1.0_Nov77.pdf
> 
> I think WHAMI was a pdp10 instruction even before that, but I can't find 
> docs.  :-)

I read a bit on PDP-11, but never on PDP-10. Thanks, this was
interesting! I understand 8 extra bits in names were expensive back
then =)

> 
>>   DEF_HELPER_FLAGS_0(get_vmtime, TCG_CALL_NO_RWG, i64)
>>   DEF_HELPER_FLAGS_0(get_walltime, TCG_CALL_NO_RWG, i64)
>> diff --git a/target/alpha/sys_helper.c b/target/alpha/sys_helper.c
>> index 51e32544287..a757a558900 100644
>> --- a/target/alpha/sys_helper.c
>> +++ b/target/alpha/sys_helper.c
>> @@ -73,3 +73,8 @@ void helper_set_alarm(CPUAlphaState *env, uint64_t 
>> expire)
>>           timer_del(cpu->alarm_timer);
>>       }
>>   }
>> +
>> +uint64_t HELPER(whoami)(CPUAlphaState *env)
>> +{
>> +    return env_cpu(env)->cpu_index;
>> +}
> 
> It's a correct transformation of the current code,

This also makes the code simpler to review for those not born with
ability to compile TCG mentally :P And ease adding tracing & co
(the performance gain for not calling an helper is not relevant here).

> though hard to evaluate without further context.

I'll post v2 removing the extra 'O' for completeness, but don't mind
to keep carying locally until more effort is posted.

> 
> Aside from the extra 'O',
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

Re: [PATCH] target/alpha: Access CPUState::cpu_index via helper

Posted by Anton Johansson via 4 months, 2 weeks ago

On 24/09/25, Philippe Mathieu-Daudé wrote:
> CPUState::cpu_index is a target agnostic field, meant
> for common code (i.e. accel/ and system/ folders).
> 
> Target specific code should use the CPUClass::get_arch_id()
> helper, even if there is a 1:1 mapping.
> 
> In preparation of generic changes around CPU indexing,
> introduce the whoami helper to access the generic
> CPUState::cpu_index field.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/alpha/helper.h     | 1 +
>  target/alpha/sys_helper.c | 5 +++++
>  target/alpha/translate.c  | 3 +--
>  3 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>