Re: [PATCH] target/hppa: Speed up hppa_is_pa20()

[Richard Henderson]

On 12/27/24 23:23, Helge Deller wrote:
> Although the hppa_is_pa20() helper is costly due to string comparisms in
> object_dynamic_cast(), it is called quite often during memory lookups
> and at each start of a block of instruction translations.
> Speed hppa_is_pa20() up by calling object_dynamic_cast() only once at
> CPU creation and store the result in the is_pa20 of struct CPUArchState.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index a31dc32a9f..08ac1ec068 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -281,6 +281,7 @@ static TranslateFn 
> *machine_HP_common_init_cpus(MachineState *machine)
>       /* Create CPUs.  */
>       for (unsigned int i = 0; i < smp_cpus; i++) {
>           cpu[i] = HPPA_CPU(cpu_create(machine->cpu_type));
> +        cpu[i]->env.is_pa20 = object_dynamic_cast(OBJECT(cpu[i]), 
> TYPE_HPPA64_CPU);
>       }

This belongs in hppa_cpu_initfn, since it's internal to the workings of the cpu.
Otherwise,

> diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
> index e45ba50a59..c37a701f44 100644
> --- a/target/hppa/cpu.h
> +++ b/target/hppa/cpu.h
> @@ -208,6 +208,7 @@ typedef struct CPUArchState {
>       uint64_t fr[32];
>       uint64_t sr[8];          /* stored shifted into place for gva */
>   
> +    bool is_pa20;

This placement will interact badly with your reset function.
Probably better to put it at the end of ArchCPU.


r~