Re: [PATCH v3 33/60] target/arm: Store cpregs key in the hash table directly

[Peter Maydell]

On Sun, 17 Apr 2022 at 19:09, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Cast the uint32_t key into a gpointer directly, which
> allows us to avoid allocating storage for each key.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpu.c     |  4 ++--
>  target/arm/gdbstub.c |  2 +-
>  target/arm/helper.c  | 45 ++++++++++++++++++++------------------------
>  3 files changed, 23 insertions(+), 28 deletions(-)
>


> @@ -231,11 +228,9 @@ static void add_cpreg_to_list(gpointer key, gpointer 
> opaque)
>  static void count_cpreg(gpointer key, gpointer opaque)
>  {
>      ARMCPU *cpu = opaque;
> -    uint64_t regidx;
>      const ARMCPRegInfo *ri;
>
> -    regidx = *(uint32_t *)key;
> -    ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
> +    ri = g_hash_table_lookup(cpu->cp_regs, key);

Here we turn a get_arm_cp_reginfo() call into a direct
g_hash_table_lookup()...

>      if (!(ri->type & (ARM_CP_NO_RAW|ARM_CP_ALIAS))) {
>          cpu->cpreg_array_len++;

> @@ -5915,16 +5910,17 @@ static void 
> define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu)
>
>      for (i = 0; i < ARRAY_SIZE(aliases); i++) {
>          const struct E2HAlias *a = &aliases[i];
> -        ARMCPRegInfo *src_reg, *dst_reg, *new_reg;
> -        uint32_t *new_key;
> +        const ARMCPRegInfo *dst_reg;
> +        ARMCPRegInfo *src_reg;
> +        ARMCPRegInfo *new_reg;
>          bool ok;
>
>          if (a->feature && !a->feature(&cpu->isar)) {
>              continue;
>          }
>
> -        src_reg = g_hash_table_lookup(cpu->cp_regs, &a->src_key);
> -        dst_reg = g_hash_table_lookup(cpu->cp_regs, &a->dst_key);
> +        src_reg = (ARMCPRegInfo *)get_arm_cp_reginfo(cpu->cp_regs, 
> a->src_key);
> +        dst_reg = get_arm_cp_reginfo(cpu->cp_regs, a->dst_key);

...but here we turn g_hash_table_lookup() calls into
get_arm_cp_reginfo() calls (necessitating an ugly cast-away-const).
Is there a rationale for when we should use which function ?

>          g_assert(src_reg != NULL);
>          g_assert(dst_reg != NULL);

thanks
-- PMM