diff mbox series

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

Message ID 20220417174426.711829-34-richard.henderson@linaro.org
State New
Headers show
Series target/arm: Cleanups, new features, new cpus | expand

Commit Message

Richard Henderson April 17, 2022, 5:43 p.m. UTC 
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(-)

Comments

Peter Maydell April 22, 2022, 10:46 a.m. UTC | #1 
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
diff mbox series

Patch

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 92fc75b2bf..af13b34697 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1080,8 +1080,8 @@  static void arm_cpu_initfn(Object *obj)
     ARMCPU *cpu = ARM_CPU(obj);
 
     cpu_set_cpustate_pointers(cpu);
-    cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
-                                         g_free, cpreg_hashtable_data_destroy);
+    cpu->cp_regs = g_hash_table_new_full(g_direct_hash, g_direct_equal,
+                                         NULL, cpreg_hashtable_data_destroy);
 
     QLIST_INIT(&cpu->pre_el_change_hooks);
     QLIST_INIT(&cpu->el_change_hooks);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index f01a126108..f5b35cd55f 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -273,7 +273,7 @@  static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml,
 static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
                                         gpointer p)
 {
-    uint32_t ri_key = *(uint32_t *)key;
+    uint32_t ri_key = (uintptr_t)key;
     ARMCPRegInfo *ri = value;
     RegisterSysregXmlParam *param = (RegisterSysregXmlParam *)p;
     GString *s = param->s;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index aee195400b..db9e75a42d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -215,11 +215,8 @@  bool write_list_to_cpustate(ARMCPU *cpu)
 static void add_cpreg_to_list(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);
+    uint32_t regidx = (uintptr_t)key;
+    const ARMCPRegInfo *ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
 
     if (!(ri->type & (ARM_CP_NO_RAW|ARM_CP_ALIAS))) {
         cpu->cpreg_indexes[cpu->cpreg_array_len] = cpreg_to_kvm_id(regidx);
@@ -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);
 
     if (!(ri->type & (ARM_CP_NO_RAW|ARM_CP_ALIAS))) {
         cpu->cpreg_array_len++;
@@ -244,8 +239,8 @@  static void count_cpreg(gpointer key, gpointer opaque)
 
 static gint cpreg_key_compare(gconstpointer a, gconstpointer b)
 {
-    uint64_t aidx = cpreg_to_kvm_id(*(uint32_t *)a);
-    uint64_t bidx = cpreg_to_kvm_id(*(uint32_t *)b);
+    uint64_t aidx = cpreg_to_kvm_id((uintptr_t)a);
+    uint64_t bidx = cpreg_to_kvm_id((uintptr_t)b);
 
     if (aidx > bidx) {
         return 1;
@@ -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);
         g_assert(src_reg != NULL);
         g_assert(dst_reg != NULL);
 
@@ -5937,7 +5933,6 @@  static void define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu)
 
         /* Create alias before redirection so we dup the right data. */
         new_reg = g_memdup(src_reg, sizeof(ARMCPRegInfo));
-        new_key = g_memdup(&a->new_key, sizeof(uint32_t));
 
         new_reg->name = a->new_name;
         new_reg->type |= ARM_CP_ALIAS;
@@ -5956,10 +5951,11 @@  static void define_arm_vh_e2h_redirects_aliases(ARMCPU *cpu)
 
 #undef E
 
-        ok = g_hash_table_insert(cpu->cp_regs, new_key, new_reg);
+        ok = g_hash_table_insert(cpu->cp_regs,
+                                 (gpointer)(uintptr_t)a->new_key, new_reg);
         g_assert(ok);
 
-        src_reg->opaque = dst_reg;
+        src_reg->opaque = (void *)dst_reg;
         src_reg->orig_readfn = src_reg->readfn ?: raw_read;
         src_reg->orig_writefn = src_reg->writefn ?: raw_write;
         if (!src_reg->raw_readfn) {
@@ -8522,7 +8518,7 @@  static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
     /* Private utility function for define_one_arm_cp_reg_with_opaque():
      * add a single reginfo struct to the hash table.
      */
-    uint32_t *key = g_new(uint32_t, 1);
+    uint32_t key;
     ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo));
     int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0;
     int ns = (secstate & ARM_CP_SECSTATE_NS) ? 1 : 0;
@@ -8589,10 +8585,10 @@  static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
         if (r->cp == 0 || r->state == ARM_CP_STATE_BOTH) {
             r2->cp = CP_REG_ARM64_SYSREG_CP;
         }
-        *key = ENCODE_AA64_CP_REG(r2->cp, r2->crn, crm,
-                                  r2->opc0, opc1, opc2);
+        key = ENCODE_AA64_CP_REG(r2->cp, r2->crn, crm,
+                                 r2->opc0, opc1, opc2);
     } else {
-        *key = ENCODE_CP_REG(r2->cp, is64, ns, r2->crn, crm, opc1, opc2);
+        key = ENCODE_CP_REG(r2->cp, is64, ns, r2->crn, crm, opc1, opc2);
     }
     if (opaque) {
         r2->opaque = opaque;
@@ -8634,8 +8630,7 @@  static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
      * requested.
      */
     if (!(r->type & ARM_CP_OVERRIDE)) {
-        ARMCPRegInfo *oldreg;
-        oldreg = g_hash_table_lookup(cpu->cp_regs, key);
+        const ARMCPRegInfo *oldreg = get_arm_cp_reginfo(cpu->cp_regs, key);
         if (oldreg && !(oldreg->type & ARM_CP_OVERRIDE)) {
             fprintf(stderr, "Register redefined: cp=%d %d bit "
                     "crn=%d crm=%d opc1=%d opc2=%d, "
@@ -8645,7 +8640,7 @@  static void add_cpreg_to_hashtable(ARMCPU *cpu, const ARMCPRegInfo *r,
             g_assert_not_reached();
         }
     }
-    g_hash_table_insert(cpu->cp_regs, key, r2);
+    g_hash_table_insert(cpu->cp_regs, (gpointer)(uintptr_t)key, r2);
 }
 
 
@@ -8875,7 +8870,7 @@  void modify_arm_cp_regs_with_len(ARMCPRegInfo *regs, size_t regs_len,
 
 const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp)
 {
-    return g_hash_table_lookup(cpregs, &encoded_cp);
+    return g_hash_table_lookup(cpregs, (gpointer)(uintptr_t)encoded_cp);
 }
 
 void arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri,