qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration
@ 2013-09-15  0:03 Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 1/8] tcg: Delete tcg_helper_get_name declaration Richard Henderson
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien, sw

The subject came up in the contect of review of other patches
this weekend, and it was pretty easy to whip up.


r~


Richard Henderson (8):
  tcg: Delete tcg_helper_get_name declaration
  tcg: Use a GHashTable for tcg_find_helper
  target-m68k: Rename helpers.h to helper.h
  tcg: Move helper registration into tcg_context_init
  tcg: Remove stray semi-colons from target-*/helper.h
  tcg: Put target helper data into an array.
  tcg: Add tcg-runtime.c helpers to all_helpers
  tcg: Merge tcg_register_helper into tcg_context_init

 include/exec/def-helper.h           |   3 +-
 target-alpha/helper.h               |   2 +-
 target-alpha/translate.c            |   4 --
 target-arm/helper.h                 |   8 +--
 target-arm/translate.c              |   3 -
 target-cris/helper.h                |   8 +--
 target-cris/translate.c             |   3 -
 target-i386/translate.c             |   4 --
 target-m68k/helper.c                |   2 +-
 target-m68k/{helpers.h => helper.h} |   0
 target-m68k/op_helper.c             |   2 +-
 target-m68k/translate.c             |   7 +--
 target-microblaze/translate.c       |   2 -
 target-mips/helper.h                |  12 ++--
 target-mips/translate.c             |   4 --
 target-openrisc/translate.c         |   2 -
 target-ppc/helper.h                 |  10 ++--
 target-ppc/translate.c              |   4 --
 target-s390x/translate.c            |   4 --
 target-sh4/translate.c              |   4 --
 target-sparc/helper.h               |  18 +++---
 target-sparc/translate.c            |   5 --
 target-unicore32/translate.c        |   3 -
 target-xtensa/translate.c           |   2 -
 tcg/tcg.c                           | 114 ++++++++++++++++--------------------
 tcg/tcg.h                           |  12 +---
 26 files changed, 85 insertions(+), 157 deletions(-)
 rename target-m68k/{helpers.h => helper.h} (100%)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 1/8] tcg: Delete tcg_helper_get_name declaration
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  2013-09-15  6:14   ` Stefan Weil
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 2/8] tcg: Use a GHashTable for tcg_find_helper Richard Henderson
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Richard Henderson, aurelien, sw

The function was deleted in 4dc81f2822187f4503d4bdb76785cafa5b28db0b.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tcg/tcg.h b/tcg/tcg.h
index 902c751..20543f6 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -681,7 +681,6 @@ TCGArg *tcg_optimize(TCGContext *s, uint16_t *tcg_opc_ptr, TCGArg *args,
 
 /* only used for debugging purposes */
 void tcg_register_helper(void *func, const char *name);
-const char *tcg_helper_get_name(TCGContext *s, void *func);
 void tcg_dump_ops(TCGContext *s);
 
 void dump_ops(const uint16_t *opc_buf, const TCGArg *opparam_buf);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 2/8] tcg: Use a GHashTable for tcg_find_helper
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 1/8] tcg: Delete tcg_helper_get_name declaration Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  2013-09-15  6:32   ` Stefan Weil
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 3/8] target-m68k: Rename helpers.h to helper.h Richard Henderson
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Richard Henderson, aurelien, sw

Slightly changes the interface, in that we now return name
instead of a TCGHelperInfo structure, which goes away.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 74 ++++++++++++++++-----------------------------------------------
 tcg/tcg.h | 10 +--------
 2 files changed, 19 insertions(+), 65 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index fd7fb6b..98b1c37 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -623,20 +623,15 @@ int tcg_check_temp_count(void)
 void tcg_register_helper(void *func, const char *name)
 {
     TCGContext *s = &tcg_ctx;
-    int n;
-    if ((s->nb_helpers + 1) > s->allocated_helpers) {
-        n = s->allocated_helpers;
-        if (n == 0) {
-            n = 4;
-        } else {
-            n *= 2;
-        }
-        s->helpers = realloc(s->helpers, n * sizeof(TCGHelperInfo));
-        s->allocated_helpers = n;
+    GHashTable *table = s->helpers;
+
+    if (table == NULL) {
+        /* Use g_direct_hash/equal for direct pointer comparisons on func.  */
+        table = g_hash_table_new(NULL, NULL);
+        s->helpers = table;
     }
-    s->helpers[s->nb_helpers].func = (uintptr_t)func;
-    s->helpers[s->nb_helpers].name = name;
-    s->nb_helpers++;
+
+    g_hash_table_insert(table, (gpointer)func, (gpointer)name);
 }
 
 /* Note: we convert the 64 bit args to 32 bit and do some alignment
@@ -851,47 +846,14 @@ char *tcg_get_arg_str_i64(TCGContext *s, char *buf, int buf_size, TCGv_i64 arg)
     return tcg_get_arg_str_idx(s, buf, buf_size, GET_TCGV_I64(arg));
 }
 
-static int helper_cmp(const void *p1, const void *p2)
+/* Find helper name.  */
+static inline const char *tcg_find_helper(TCGContext *s, uintptr_t val)
 {
-    const TCGHelperInfo *th1 = p1;
-    const TCGHelperInfo *th2 = p2;
-    if (th1->func < th2->func)
-        return -1;
-    else if (th1->func == th2->func)
-        return 0;
-    else
-        return 1;
-}
-
-/* find helper definition (Note: A hash table would be better) */
-static TCGHelperInfo *tcg_find_helper(TCGContext *s, uintptr_t val)
-{
-    int m, m_min, m_max;
-    TCGHelperInfo *th;
-    uintptr_t v;
-
-    if (unlikely(!s->helpers_sorted)) {
-        qsort(s->helpers, s->nb_helpers, sizeof(TCGHelperInfo), 
-              helper_cmp);
-        s->helpers_sorted = 1;
-    }
-
-    /* binary search */
-    m_min = 0;
-    m_max = s->nb_helpers - 1;
-    while (m_min <= m_max) {
-        m = (m_min + m_max) >> 1;
-        th = &s->helpers[m];
-        v = th->func;
-        if (v == val)
-            return th;
-        else if (val < v) {
-            m_max = m - 1;
-        } else {
-            m_min = m + 1;
-        }
+    const char *ret = NULL;
+    if (s->helpers) {
+	ret = g_hash_table_lookup(s->helpers, (gpointer)val);
     }
-    return NULL;
+    return ret;
 }
 
 static const char * const cond_name[] =
@@ -976,7 +938,7 @@ void tcg_dump_ops(TCGContext *s)
             }
         } else if (c == INDEX_op_movi_i32 || c == INDEX_op_movi_i64) {
             tcg_target_ulong val;
-            TCGHelperInfo *th;
+            const char *name;
 
             nb_oargs = def->nb_oargs;
             nb_iargs = def->nb_iargs;
@@ -984,9 +946,9 @@ void tcg_dump_ops(TCGContext *s)
             qemu_log(" %s %s,$", def->name,
                      tcg_get_arg_str_idx(s, buf, sizeof(buf), args[0]));
             val = args[1];
-            th = tcg_find_helper(s, val);
-            if (th) {
-                qemu_log("%s", th->name);
+            name = tcg_find_helper(s, val);
+            if (name) {
+                qemu_log("%s", name);
             } else {
                 if (c == INDEX_op_movi_i32) {
                     qemu_log("0x%x", (uint32_t)val);
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 20543f6..8c5eb42 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -405,11 +405,6 @@ typedef struct TCGTemp {
     const char *name;
 } TCGTemp;
 
-typedef struct TCGHelperInfo {
-    uintptr_t func;
-    const char *name;
-} TCGHelperInfo;
-
 typedef struct TCGContext TCGContext;
 
 struct TCGContext {
@@ -447,10 +442,7 @@ struct TCGContext {
     uint8_t *code_ptr;
     TCGTemp temps[TCG_MAX_TEMPS]; /* globals first, temps after */
 
-    TCGHelperInfo *helpers;
-    int nb_helpers;
-    int allocated_helpers;
-    int helpers_sorted;
+    GHashTable *helpers;
 
 #ifdef CONFIG_PROFILER
     /* profiling info */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 3/8] target-m68k: Rename helpers.h to helper.h
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 1/8] tcg: Delete tcg_helper_get_name declaration Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 2/8] tcg: Use a GHashTable for tcg_find_helper Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  2013-09-15  6:35   ` Stefan Weil
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 4/8] tcg: Move helper registration into tcg_context_init Richard Henderson
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Paul Brook, aurelien, sw

This brings the m68k target in line with all other targets.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-m68k/helper.c                | 2 +-
 target-m68k/{helpers.h => helper.h} | 0
 target-m68k/op_helper.c             | 2 +-
 target-m68k/translate.c             | 6 +++---
 4 files changed, 5 insertions(+), 5 deletions(-)
 rename target-m68k/{helpers.h => helper.h} (100%)

diff --git a/target-m68k/helper.c b/target-m68k/helper.c
index 00a7a08..a508896 100644
--- a/target-m68k/helper.c
+++ b/target-m68k/helper.c
@@ -21,7 +21,7 @@
 #include "cpu.h"
 #include "exec/gdbstub.h"
 
-#include "helpers.h"
+#include "helper.h"
 
 #define SIGNBIT (1u << 31)
 
diff --git a/target-m68k/helpers.h b/target-m68k/helper.h
similarity index 100%
rename from target-m68k/helpers.h
rename to target-m68k/helper.h
diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index 30f7d8b..bbbfd7f 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -17,7 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "cpu.h"
-#include "helpers.h"
+#include "helper.h"
 
 #if defined(CONFIG_USER_ONLY)
 
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 0be0a96..f31e48d 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -23,9 +23,9 @@
 #include "tcg-op.h"
 #include "qemu/log.h"
 
-#include "helpers.h"
+#include "helper.h"
 #define GEN_HELPER 1
-#include "helpers.h"
+#include "helper.h"
 
 //#define DEBUG_DISPATCH 1
 
@@ -110,7 +110,7 @@ void m68k_tcg_init(void)
     store_dummy = tcg_global_mem_new(TCG_AREG0, -8, "NULL");
 
 #define GEN_HELPER 2
-#include "helpers.h"
+#include "helper.h"
 }
 
 static inline void qemu_assert(int cond, const char *msg)
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 4/8] tcg: Move helper registration into tcg_context_init
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
                   ` (2 preceding siblings ...)
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 3/8] target-m68k: Rename helpers.h to helper.h Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 5/8] tcg: Remove stray semi-colons from target-*/helper.h Richard Henderson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, Jia Liu, sw, Alexander Graf, Blue Swirl,
	Max Filippov, open list:PowerPC, Paul Brook, Edgar E. Iglesias,
	Guan Xuetao, aurelien, Richard Henderson

No longer needs to be done on a per-target basis.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-alpha/translate.c      | 4 ----
 target-arm/translate.c        | 3 ---
 target-cris/translate.c       | 3 ---
 target-i386/translate.c       | 4 ----
 target-m68k/translate.c       | 3 ---
 target-microblaze/translate.c | 2 --
 target-mips/translate.c       | 4 ----
 target-openrisc/translate.c   | 2 --
 target-ppc/translate.c        | 4 ----
 target-s390x/translate.c      | 4 ----
 target-sh4/translate.c        | 4 ----
 target-sparc/translate.c      | 5 -----
 target-unicore32/translate.c  | 3 ---
 target-xtensa/translate.c     | 2 --
 tcg/tcg.c                     | 8 +++++++-
 15 files changed, 7 insertions(+), 48 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 28ce436..9cb8084 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -140,10 +140,6 @@ void alpha_translate_init(void)
                                      offsetof(CPUAlphaState, usp), "usp");
 #endif
 
-    /* register helpers */
-#define GEN_HELPER 2
-#include "helper.h"
-
     done_init = 1;
 }
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 998bde2..5f003e7 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -115,9 +115,6 @@ void arm_translate_init(void)
 #endif
 
     a64_translate_init();
-
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 static inline TCGv_i32 load_cpu_offset(int offset)
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 617e1b4..5faa44c 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3480,9 +3480,6 @@ void cris_initialize_tcg(void)
 {
     int i;
 
-#define GEN_HELPER 2
-#include "helper.h"
-
     cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
     cc_x = tcg_global_mem_new(TCG_AREG0,
                               offsetof(CPUCRISState, cc_x), "cc_x");
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 6d87900..439fc5a 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -8242,10 +8242,6 @@ void optimize_flags_init(void)
     cpu_regs[R_EDI] = tcg_global_mem_new_i32(TCG_AREG0,
                                              offsetof(CPUX86State, regs[R_EDI]), "edi");
 #endif
-
-    /* register helpers */
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 /* generate intermediate code in gen_opc_buf and gen_opparam_buf for
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index f31e48d..f54b94a 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -108,9 +108,6 @@ void m68k_tcg_init(void)
 
     NULL_QREG = tcg_global_mem_new(TCG_AREG0, -4, "NULL");
     store_dummy = tcg_global_mem_new(TCG_AREG0, -8, "NULL");
-
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 static inline void qemu_assert(int cond, const char *msg)
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 0673176..1b937b3 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -2024,8 +2024,6 @@ void mb_tcg_init(void)
                           offsetof(CPUMBState, sregs[i]),
                           special_regnames[i]);
     }
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 void restore_state_to_opc(CPUMBState *env, TranslationBlock *tb, int pc_pos)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index ad43d59..0d8db66 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -15886,10 +15886,6 @@ void mips_tcg_init(void)
                                        offsetof(CPUMIPSState, active_fpu.fcr31),
                                        "fcr31");
 
-    /* register helpers */
-#define GEN_HELPER 2
-#include "helper.h"
-
     inited = 1;
 }
 
diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
index 723b77d..8908a2e 100644
--- a/target-openrisc/translate.c
+++ b/target-openrisc/translate.c
@@ -110,8 +110,6 @@ void openrisc_translate_init(void)
                                       offsetof(CPUOpenRISCState, gpr[i]),
                                       regnames[i]);
     }
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 /* Writeback SR_F transaltion-space to execution-space.  */
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 2da7bc7..45ec840 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -175,10 +175,6 @@ void ppc_translate_init(void)
     cpu_access_type = tcg_global_mem_new_i32(TCG_AREG0,
                                              offsetof(CPUPPCState, access_type), "access_type");
 
-    /* register helpers */
-#define GEN_HELPER 2
-#include "helper.h"
-
     done_init = 1;
 }
 
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index afe90eb..bc99a37 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -188,10 +188,6 @@ void s390x_translate_init(void)
                                       offsetof(CPUS390XState, fregs[i].d),
                                       cpu_reg_names[i + 16]);
     }
-
-    /* register helpers */
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 static TCGv_i64 load_reg(int reg)
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index c06b29f..2272eb0 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -143,10 +143,6 @@ void sh4_translate_init(void)
                                               offsetof(CPUSH4State, fregs[i]),
                                               fregnames[i]);
 
-    /* register helpers */
-#define GEN_HELPER 2
-#include "helper.h"
-
     done_init = 1;
 }
 
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 36615f1..dce64c3 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5456,11 +5456,6 @@ void gen_intermediate_code_init(CPUSPARCState *env)
                                                 offsetof(CPUSPARCState, fpr[i]),
                                                 fregnames[i]);
         }
-
-        /* register helpers */
-
-#define GEN_HELPER 2
-#include "helper.h"
     }
 }
 
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index 1246895..4572890 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -74,9 +74,6 @@ void uc32_translate_init(void)
         cpu_R[i] = tcg_global_mem_new_i32(TCG_AREG0,
                                 offsetof(CPUUniCore32State, regs[i]), regnames[i]);
     }
-
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 static int num_temps;
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 24343bd..06641bb 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -238,8 +238,6 @@ void xtensa_translate_init(void)
                     uregnames[i].name);
         }
     }
-#define GEN_HELPER 2
-#include "helper.h"
 }
 
 static inline bool option_bits_enabled(DisasContext *dc, uint64_t opt)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 98b1c37..59251c0 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -254,6 +254,8 @@ void tcg_pool_reset(TCGContext *s)
     s->pool_current = NULL;
 }
 
+#include "helper.h"
+
 void tcg_context_init(TCGContext *s)
 {
     int op, total_args, n;
@@ -284,7 +286,11 @@ void tcg_context_init(TCGContext *s)
         sorted_args += n;
         args_ct += n;
     }
-    
+
+    /* Register helpers.  */
+#define GEN_HELPER 2
+#include "helper.h"
+
     tcg_target_init(s);
 }
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 5/8] tcg: Remove stray semi-colons from target-*/helper.h
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
                   ` (3 preceding siblings ...)
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 4/8] tcg: Move helper registration into tcg_context_init Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  2013-09-15  7:03   ` Stefan Weil
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 6/8] tcg: Put target helper data into an array Richard Henderson
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: peter.maydell, sw, Alexander Graf, Blue Swirl, open list:PowerPC,
	Paul Brook, Edgar E. Iglesias, aurelien, Richard Henderson

During GEN_HELPER=1, these are actually stray top-level semi-colons
which are technically invalid ISO C, but GCC accepts as an extension.
If we added enough __extension__ markers that we could dare use
-Wpedantic, we'd see

  warning: ISO C does not allow extra ‘;’ outside of a function

This will become a hard error in the next patch, wherein those ; will
appear in the middle of a data structure.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-alpha/helper.h |  2 +-
 target-arm/helper.h   |  8 ++++----
 target-cris/helper.h  |  8 ++++----
 target-mips/helper.h  | 12 ++++++------
 target-ppc/helper.h   | 10 +++++-----
 target-sparc/helper.h | 18 +++++++++---------
 6 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/target-alpha/helper.h b/target-alpha/helper.h
index 732b701..5a0e78c 100644
--- a/target-alpha/helper.h
+++ b/target-alpha/helper.h
@@ -114,7 +114,7 @@ DEF_HELPER_FLAGS_1(tbia, TCG_CALL_NO_RWG, void, env)
 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(halt, void, i64)
 
 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-arm/helper.h b/target-arm/helper.h
index 63ae13a..cac9564 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -247,10 +247,10 @@ DEF_HELPER_3(neon_qshl_u32, i32, env, i32, i32)
 DEF_HELPER_3(neon_qshl_s32, i32, env, i32, i32)
 DEF_HELPER_3(neon_qshl_u64, i64, env, i64, i64)
 DEF_HELPER_3(neon_qshl_s64, i64, env, i64, i64)
-DEF_HELPER_3(neon_qshlu_s8, i32, env, i32, i32);
-DEF_HELPER_3(neon_qshlu_s16, i32, env, i32, i32);
-DEF_HELPER_3(neon_qshlu_s32, i32, env, i32, i32);
-DEF_HELPER_3(neon_qshlu_s64, i64, env, i64, i64);
+DEF_HELPER_3(neon_qshlu_s8, i32, env, i32, i32)
+DEF_HELPER_3(neon_qshlu_s16, i32, env, i32, i32)
+DEF_HELPER_3(neon_qshlu_s32, i32, env, i32, i32)
+DEF_HELPER_3(neon_qshlu_s64, i64, env, i64, i64)
 DEF_HELPER_3(neon_qrshl_u8, i32, env, i32, i32)
 DEF_HELPER_3(neon_qrshl_s8, i32, env, i32, i32)
 DEF_HELPER_3(neon_qrshl_u16, i32, env, i32, i32)
diff --git a/target-cris/helper.h b/target-cris/helper.h
index 8e8365c..0ac31f5 100644
--- a/target-cris/helper.h
+++ b/target-cris/helper.h
@@ -4,14 +4,14 @@ DEF_HELPER_2(raise_exception, void, env, i32)
 DEF_HELPER_2(tlb_flush_pid, void, env, i32)
 DEF_HELPER_2(spc_write, void, env, i32)
 DEF_HELPER_3(dump, void, i32, i32, i32)
-DEF_HELPER_1(rfe, void, env);
-DEF_HELPER_1(rfn, void, env);
+DEF_HELPER_1(rfe, void, env)
+DEF_HELPER_1(rfn, void, env)
 
 DEF_HELPER_3(movl_sreg_reg, void, env, i32, i32)
 DEF_HELPER_3(movl_reg_sreg, void, env, i32, i32)
 
-DEF_HELPER_FLAGS_1(lz, TCG_CALL_NO_SE, i32, i32);
-DEF_HELPER_FLAGS_4(btst, TCG_CALL_NO_SE, i32, env, i32, i32, i32);
+DEF_HELPER_FLAGS_1(lz, TCG_CALL_NO_SE, i32, i32)
+DEF_HELPER_FLAGS_4(btst, TCG_CALL_NO_SE, i32, env, i32, i32, i32)
 
 DEF_HELPER_FLAGS_4(evaluate_flags_muls, TCG_CALL_NO_SE, i32, env, i32, i32, i32)
 DEF_HELPER_FLAGS_4(evaluate_flags_mulu, TCG_CALL_NO_SE, i32, env, i32, i32, i32)
diff --git a/target-mips/helper.h b/target-mips/helper.h
index ed75e2c..1a8b86d 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -148,7 +148,7 @@ DEF_HELPER_2(mtc0_taghi, void, env, tl)
 DEF_HELPER_2(mtc0_datahi, void, env, tl)
 
 /* MIPS MT functions */
-DEF_HELPER_2(mftgpr, tl, env, i32);
+DEF_HELPER_2(mftgpr, tl, env, i32)
 DEF_HELPER_2(mftlo, tl, env, i32)
 DEF_HELPER_2(mfthi, tl, env, i32)
 DEF_HELPER_2(mftacx, tl, env, i32)
@@ -165,11 +165,11 @@ DEF_HELPER_1(evpe, tl, env)
 #endif /* !CONFIG_USER_ONLY */
 
 /* microMIPS functions */
-DEF_HELPER_4(lwm, void, env, tl, tl, i32);
-DEF_HELPER_4(swm, void, env, tl, tl, i32);
+DEF_HELPER_4(lwm, void, env, tl, tl, i32)
+DEF_HELPER_4(swm, void, env, tl, tl, i32)
 #ifdef TARGET_MIPS64
-DEF_HELPER_4(ldm, void, env, tl, tl, i32);
-DEF_HELPER_4(sdm, void, env, tl, tl, i32);
+DEF_HELPER_4(ldm, void, env, tl, tl, i32)
+DEF_HELPER_4(sdm, void, env, tl, tl, i32)
 #endif
 
 DEF_HELPER_2(fork, void, tl, tl)
@@ -615,7 +615,7 @@ DEF_HELPER_FLAGS_4(dmsubu, 0, void, tl, tl, i32, env)
 DEF_HELPER_FLAGS_1(bitrev, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_3(insv, 0, tl, env, tl, tl)
 #if defined(TARGET_MIPS64)
-DEF_HELPER_FLAGS_3(dinsv, 0, tl, env, tl, tl);
+DEF_HELPER_FLAGS_3(dinsv, 0, tl, env, tl, tl)
 #endif
 
 /* DSP Compare-Pick Sub-class insns */
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 56814b5..6d282bb 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -168,8 +168,8 @@ DEF_HELPER_3(vslo, void, avr, avr, avr)
 DEF_HELPER_3(vsro, void, avr, avr, avr)
 DEF_HELPER_3(vaddcuw, void, avr, avr, avr)
 DEF_HELPER_3(vsubcuw, void, avr, avr, avr)
-DEF_HELPER_2(lvsl, void, avr, tl);
-DEF_HELPER_2(lvsr, void, avr, tl);
+DEF_HELPER_2(lvsl, void, avr, tl)
+DEF_HELPER_2(lvsr, void, avr, tl)
 DEF_HELPER_4(vaddsbs, void, env, avr, avr, avr)
 DEF_HELPER_4(vaddshs, void, env, avr, avr, avr)
 DEF_HELPER_4(vaddsws, void, env, avr, avr, avr)
@@ -220,7 +220,7 @@ DEF_HELPER_5(vmsumuhs, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vmsumshm, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vmsumshs, void, env, avr, avr, avr, avr)
 DEF_HELPER_4(vmladduhm, void, avr, avr, avr, avr)
-DEF_HELPER_2(mtvscr, void, env, avr);
+DEF_HELPER_2(mtvscr, void, env, avr)
 DEF_HELPER_3(lvebx, void, env, avr, tl)
 DEF_HELPER_3(lvehx, void, env, avr, tl)
 DEF_HELPER_3(lvewx, void, env, avr, tl)
@@ -349,7 +349,7 @@ DEF_HELPER_2(load_slb_vsid, tl, env, tl)
 DEF_HELPER_FLAGS_1(slbia, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(slbie, TCG_CALL_NO_RWG, void, env, tl)
 #endif
-DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl);
+DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
 DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
 
 DEF_HELPER_FLAGS_1(602_mfrom, TCG_CALL_NO_RWG_SE, tl, tl)
@@ -367,7 +367,7 @@ DEF_HELPER_3(divo, tl, env, tl, tl)
 DEF_HELPER_3(divs, tl, env, tl, tl)
 DEF_HELPER_3(divso, tl, env, tl, tl)
 
-DEF_HELPER_2(load_dcr, tl, env, tl);
+DEF_HELPER_2(load_dcr, tl, env, tl)
 DEF_HELPER_3(store_dcr, void, env, tl, tl)
 
 DEF_HELPER_2(load_dump_spr, void, env, i32)
diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index 15f7328..2a771b2 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -103,7 +103,7 @@ DEF_HELPER_3(fmuls, f32, env, f32, f32)
 DEF_HELPER_3(fdivs, f32, env, f32, f32)
 
 DEF_HELPER_3(fsmuld, f64, env, f32, f32)
-DEF_HELPER_3(fdmulq, void, env, f64, f64);
+DEF_HELPER_3(fdmulq, void, env, f64, f64)
 
 DEF_HELPER_FLAGS_1(fnegs, TCG_CALL_NO_RWG_SE, f32, f32)
 DEF_HELPER_2(fitod, f64, env, s32)
@@ -156,22 +156,22 @@ DEF_HELPER_FLAGS_3(bshuffle, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
     DEF_HELPER_FLAGS_2(f ## name ## 32s, TCG_CALL_NO_RWG_SE, \
                        i32, i32, i32)
 
-VIS_HELPER(padd);
-VIS_HELPER(psub);
+VIS_HELPER(padd)
+VIS_HELPER(psub)
 #define VIS_CMPHELPER(name)                                              \
     DEF_HELPER_FLAGS_2(f##name##16, TCG_CALL_NO_RWG_SE,      \
                        i64, i64, i64)                                    \
     DEF_HELPER_FLAGS_2(f##name##32, TCG_CALL_NO_RWG_SE,      \
                        i64, i64, i64)
-VIS_CMPHELPER(cmpgt);
-VIS_CMPHELPER(cmpeq);
-VIS_CMPHELPER(cmple);
-VIS_CMPHELPER(cmpne);
+VIS_CMPHELPER(cmpgt)
+VIS_CMPHELPER(cmpeq)
+VIS_CMPHELPER(cmple)
+VIS_CMPHELPER(cmpne)
 #endif
 #undef F_HELPER_0_1
 #undef VIS_HELPER
 #undef VIS_CMPHELPER
-DEF_HELPER_1(compute_psr, void, env);
-DEF_HELPER_1(compute_C_icc, i32, env);
+DEF_HELPER_1(compute_psr, void, env)
+DEF_HELPER_1(compute_C_icc, i32, env)
 
 #include "exec/def-helper.h"
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 6/8] tcg: Put target helper data into an array.
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
                   ` (4 preceding siblings ...)
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 5/8] tcg: Remove stray semi-colons from target-*/helper.h Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 7/8] tcg: Add tcg-runtime.c helpers to all_helpers Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 8/8] tcg: Merge tcg_register_helper into tcg_context_init Richard Henderson
  7 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Richard Henderson, aurelien, sw

One call inside of a loop to tcg_register_helper instead of hundreds
of sequential calls.

Presumably more icache and branch prediction friendly; resulting binary
size mostly unchanged on x86_64, as we're trading 32-bit rip-relative
references in .text for full 64-bit pointers in .rodata.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 include/exec/def-helper.h |  3 +--
 tcg/tcg.c                 | 17 ++++++++++++++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/exec/def-helper.h b/include/exec/def-helper.h
index 022a9ce..73d51f9 100644
--- a/include/exec/def-helper.h
+++ b/include/exec/def-helper.h
@@ -240,8 +240,7 @@ static inline void glue(gen_helper_, name)(dh_retvar_decl(ret) \
 #elif GEN_HELPER == 2
 /* Register helpers.  */
 
-#define DEF_HELPER_FLAGS_0(name, flags, ret) \
-tcg_register_helper(HELPER(name), #name);
+#define DEF_HELPER_FLAGS_0(name, flags, ret)  { HELPER(name), #name },
 
 #define DEF_HELPER_FLAGS_1(name, flags, ret, t1) \
 DEF_HELPER_FLAGS_0(name, flags, ret)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 59251c0..9ace8fc 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -256,9 +256,19 @@ void tcg_pool_reset(TCGContext *s)
 
 #include "helper.h"
 
+typedef struct TCGHelperInfo {
+    void *func;
+    const char *name;
+} TCGHelperInfo;
+
+static const TCGHelperInfo all_helpers[] = {
+#define GEN_HELPER 2
+#include "helper.h"
+};
+
 void tcg_context_init(TCGContext *s)
 {
-    int op, total_args, n;
+    int op, total_args, n, i;
     TCGOpDef *def;
     TCGArgConstraint *args_ct;
     int *sorted_args;
@@ -288,8 +298,9 @@ void tcg_context_init(TCGContext *s)
     }
 
     /* Register helpers.  */
-#define GEN_HELPER 2
-#include "helper.h"
+    for (i = 0; i < ARRAY_SIZE(all_helpers); ++i) {
+        tcg_register_helper(all_helpers[i].func, all_helpers[i].name);
+    }
 
     tcg_target_init(s);
 }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 7/8] tcg: Add tcg-runtime.c helpers to all_helpers
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
                   ` (5 preceding siblings ...)
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 6/8] tcg: Put target helper data into an array Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 8/8] tcg: Merge tcg_register_helper into tcg_context_init Richard Henderson
  7 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Richard Henderson, aurelien, sw

For the few targets that actually use these, we'd not report
them symbolicly in the tcg opcode logs.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 9ace8fc..bf4edfd 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -264,6 +264,22 @@ typedef struct TCGHelperInfo {
 static const TCGHelperInfo all_helpers[] = {
 #define GEN_HELPER 2
 #include "helper.h"
+
+    /* Include tcg-runtime.c functions.  */
+    { tcg_helper_div_i32, "div_i32" },
+    { tcg_helper_rem_i32, "rem_i32" },
+    { tcg_helper_divu_i32, "divu_i32" },
+    { tcg_helper_remu_i32, "remu_i32" },
+
+    { tcg_helper_shl_i64, "shl_i64" },
+    { tcg_helper_shr_i64, "shr_i64" },
+    { tcg_helper_sar_i64, "sar_i64" },
+    { tcg_helper_div_i64, "div_i64" },
+    { tcg_helper_rem_i64, "rem_i64" },
+    { tcg_helper_divu_i64, "divu_i64" },
+    { tcg_helper_remu_i64, "remu_i64" },
+    { tcg_helper_mulsh_i64, "mulsh_i64" },
+    { tcg_helper_muluh_i64, "muluh_i64" },
 };
 
 void tcg_context_init(TCGContext *s)
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 8/8] tcg: Merge tcg_register_helper into tcg_context_init
  2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
                   ` (6 preceding siblings ...)
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 7/8] tcg: Add tcg-runtime.c helpers to all_helpers Richard Henderson
@ 2013-09-15  0:03 ` Richard Henderson
  7 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2013-09-15  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Richard Henderson, aurelien, sw

Eliminates the repeated checks for having created
the s->helpers hash table.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 21 ++++++---------------
 tcg/tcg.h |  1 -
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index bf4edfd..f401044 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -288,6 +288,7 @@ void tcg_context_init(TCGContext *s)
     TCGOpDef *def;
     TCGArgConstraint *args_ct;
     int *sorted_args;
+    GHashTable *helper_table;
 
     memset(s, 0, sizeof(*s));
     s->nb_globals = 0;
@@ -314,8 +315,12 @@ void tcg_context_init(TCGContext *s)
     }
 
     /* Register helpers.  */
+    /* Use g_direct_hash/equal for direct pointer comparisons on func.  */
+    s->helpers = helper_table = g_hash_table_new(NULL, NULL);
+
     for (i = 0; i < ARRAY_SIZE(all_helpers); ++i) {
-        tcg_register_helper(all_helpers[i].func, all_helpers[i].name);
+        g_hash_table_insert(helper_table, (gpointer)all_helpers[i].func,
+                            (gpointer)all_helpers[i].name);
     }
 
     tcg_target_init(s);
@@ -653,20 +658,6 @@ int tcg_check_temp_count(void)
 }
 #endif
 
-void tcg_register_helper(void *func, const char *name)
-{
-    TCGContext *s = &tcg_ctx;
-    GHashTable *table = s->helpers;
-
-    if (table == NULL) {
-        /* Use g_direct_hash/equal for direct pointer comparisons on func.  */
-        table = g_hash_table_new(NULL, NULL);
-        s->helpers = table;
-    }
-
-    g_hash_table_insert(table, (gpointer)func, (gpointer)name);
-}
-
 /* Note: we convert the 64 bit args to 32 bit and do some alignment
    and endian swap. Maybe it would be better to do the alignment
    and endian swap in tcg_reg_alloc_call(). */
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 8c5eb42..f67fdb6 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -672,7 +672,6 @@ TCGArg *tcg_optimize(TCGContext *s, uint16_t *tcg_opc_ptr, TCGArg *args,
                      TCGOpDef *tcg_op_def);
 
 /* only used for debugging purposes */
-void tcg_register_helper(void *func, const char *name);
 void tcg_dump_ops(TCGContext *s);
 
 void dump_ops(const uint16_t *opc_buf, const TCGArg *opparam_buf);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/8] tcg: Delete tcg_helper_get_name declaration
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 1/8] tcg: Delete tcg_helper_get_name declaration Richard Henderson
@ 2013-09-15  6:14   ` Stefan Weil
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Weil @ 2013-09-15  6:14 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-devel, aurelien

Am 15.09.2013 02:03, schrieb Richard Henderson:
> The function was deleted in 4dc81f2822187f4503d4bdb76785cafa5b28db0b.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/tcg.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 902c751..20543f6 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -681,7 +681,6 @@ TCGArg *tcg_optimize(TCGContext *s, uint16_t *tcg_opc_ptr, TCGArg *args,
>  
>  /* only used for debugging purposes */
>  void tcg_register_helper(void *func, const char *name);
> -const char *tcg_helper_get_name(TCGContext *s, void *func);
>  void tcg_dump_ops(TCGContext *s);
>  
>  void dump_ops(const uint16_t *opc_buf, const TCGArg *opparam_buf);

Reviewed-by: Stefan Weil <sw@weilnetz.de>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 2/8] tcg: Use a GHashTable for tcg_find_helper
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 2/8] tcg: Use a GHashTable for tcg_find_helper Richard Henderson
@ 2013-09-15  6:32   ` Stefan Weil
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Weil @ 2013-09-15  6:32 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-devel, aurelien

Am 15.09.2013 02:03, schrieb Richard Henderson:
> Slightly changes the interface, in that we now return name
> instead of a TCGHelperInfo structure, which goes away.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/tcg.c | 74 ++++++++++++++++-----------------------------------------------
>  tcg/tcg.h | 10 +--------
>  2 files changed, 19 insertions(+), 65 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index fd7fb6b..98b1c37 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -623,20 +623,15 @@ int tcg_check_temp_count(void)
>  void tcg_register_helper(void *func, const char *name)
>  {
>      TCGContext *s = &tcg_ctx;
> -    int n;
> -    if ((s->nb_helpers + 1) > s->allocated_helpers) {
> -        n = s->allocated_helpers;
> -        if (n == 0) {
> -            n = 4;
> -        } else {
> -            n *= 2;
> -        }
> -        s->helpers = realloc(s->helpers, n * sizeof(TCGHelperInfo));
> -        s->allocated_helpers = n;
> +    GHashTable *table = s->helpers;
> +
> +    if (table == NULL) {
> +        /* Use g_direct_hash/equal for direct pointer comparisons on func.  */
> +        table = g_hash_table_new(NULL, NULL);
> +        s->helpers = table;
>      }
> -    s->helpers[s->nb_helpers].func = (uintptr_t)func;
> -    s->helpers[s->nb_helpers].name = name;
> -    s->nb_helpers++;
> +
> +    g_hash_table_insert(table, (gpointer)func, (gpointer)name);
>  }
>  
>  /* Note: we convert the 64 bit args to 32 bit and do some alignment
> @@ -851,47 +846,14 @@ char *tcg_get_arg_str_i64(TCGContext *s, char *buf, int buf_size, TCGv_i64 arg)
>      return tcg_get_arg_str_idx(s, buf, buf_size, GET_TCGV_I64(arg));
>  }
>  
> -static int helper_cmp(const void *p1, const void *p2)
> +/* Find helper name.  */
> +static inline const char *tcg_find_helper(TCGContext *s, uintptr_t val)
>  {
> -    const TCGHelperInfo *th1 = p1;
> -    const TCGHelperInfo *th2 = p2;
> -    if (th1->func < th2->func)
> -        return -1;
> -    else if (th1->func == th2->func)
> -        return 0;
> -    else
> -        return 1;
> -}
> -
> -/* find helper definition (Note: A hash table would be better) */
> -static TCGHelperInfo *tcg_find_helper(TCGContext *s, uintptr_t val)
> -{
> -    int m, m_min, m_max;
> -    TCGHelperInfo *th;
> -    uintptr_t v;
> -
> -    if (unlikely(!s->helpers_sorted)) {
> -        qsort(s->helpers, s->nb_helpers, sizeof(TCGHelperInfo), 
> -              helper_cmp);
> -        s->helpers_sorted = 1;
> -    }
> -
> -    /* binary search */
> -    m_min = 0;
> -    m_max = s->nb_helpers - 1;
> -    while (m_min <= m_max) {
> -        m = (m_min + m_max) >> 1;
> -        th = &s->helpers[m];
> -        v = th->func;
> -        if (v == val)
> -            return th;
> -        else if (val < v) {
> -            m_max = m - 1;
> -        } else {
> -            m_min = m + 1;
> -        }
> +    const char *ret = NULL;
> +    if (s->helpers) {
> +	ret = g_hash_table_lookup(s->helpers, (gpointer)val);

Please replace tab by spaces here.

>      }
> -    return NULL;
> +    return ret;
>  }
>  
>  static const char * const cond_name[] =
> @@ -976,7 +938,7 @@ void tcg_dump_ops(TCGContext *s)
>              }
>          } else if (c == INDEX_op_movi_i32 || c == INDEX_op_movi_i64) {
>              tcg_target_ulong val;
> -            TCGHelperInfo *th;
> +            const char *name;
>  
>              nb_oargs = def->nb_oargs;
>              nb_iargs = def->nb_iargs;
> @@ -984,9 +946,9 @@ void tcg_dump_ops(TCGContext *s)
>              qemu_log(" %s %s,$", def->name,
>                       tcg_get_arg_str_idx(s, buf, sizeof(buf), args[0]));
>              val = args[1];
> -            th = tcg_find_helper(s, val);
> -            if (th) {
> -                qemu_log("%s", th->name);
> +            name = tcg_find_helper(s, val);
> +            if (name) {
> +                qemu_log("%s", name);
>              } else {
>                  if (c == INDEX_op_movi_i32) {
>                      qemu_log("0x%x", (uint32_t)val);
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 20543f6..8c5eb42 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -405,11 +405,6 @@ typedef struct TCGTemp {
>      const char *name;
>  } TCGTemp;
>  
> -typedef struct TCGHelperInfo {
> -    uintptr_t func;
> -    const char *name;
> -} TCGHelperInfo;
> -
>  typedef struct TCGContext TCGContext;
>  
>  struct TCGContext {
> @@ -447,10 +442,7 @@ struct TCGContext {
>      uint8_t *code_ptr;
>      TCGTemp temps[TCG_MAX_TEMPS]; /* globals first, temps after */
>  
> -    TCGHelperInfo *helpers;
> -    int nb_helpers;
> -    int allocated_helpers;
> -    int helpers_sorted;
> +    GHashTable *helpers;
>  
>  #ifdef CONFIG_PROFILER
>      /* profiling info */

With fixed tab (see above):

Reviewed-by: Stefan Weil <sw@weilnetz.de>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 3/8] target-m68k: Rename helpers.h to helper.h
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 3/8] target-m68k: Rename helpers.h to helper.h Richard Henderson
@ 2013-09-15  6:35   ` Stefan Weil
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Weil @ 2013-09-15  6:35 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-devel, aurelien, Paul Brook

Am 15.09.2013 02:03, schrieb Richard Henderson:
> This brings the m68k target in line with all other targets.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-m68k/helper.c                | 2 +-
>  target-m68k/{helpers.h => helper.h} | 0
>  target-m68k/op_helper.c             | 2 +-
>  target-m68k/translate.c             | 6 +++---
>  4 files changed, 5 insertions(+), 5 deletions(-)
>  rename target-m68k/{helpers.h => helper.h} (100%)
>
> diff --git a/target-m68k/helper.c b/target-m68k/helper.c
> index 00a7a08..a508896 100644
> --- a/target-m68k/helper.c
> +++ b/target-m68k/helper.c
> @@ -21,7 +21,7 @@
>  #include "cpu.h"
>  #include "exec/gdbstub.h"
>  
> -#include "helpers.h"
> +#include "helper.h"
>  
>  #define SIGNBIT (1u << 31)
>  
> diff --git a/target-m68k/helpers.h b/target-m68k/helper.h
> similarity index 100%
> rename from target-m68k/helpers.h
> rename to target-m68k/helper.h
> diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
> index 30f7d8b..bbbfd7f 100644
> --- a/target-m68k/op_helper.c
> +++ b/target-m68k/op_helper.c
> @@ -17,7 +17,7 @@
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
>  #include "cpu.h"
> -#include "helpers.h"
> +#include "helper.h"
>  
>  #if defined(CONFIG_USER_ONLY)
>  
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index 0be0a96..f31e48d 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> @@ -23,9 +23,9 @@
>  #include "tcg-op.h"
>  #include "qemu/log.h"
>  
> -#include "helpers.h"
> +#include "helper.h"
>  #define GEN_HELPER 1
> -#include "helpers.h"
> +#include "helper.h"
>  
>  //#define DEBUG_DISPATCH 1
>  
> @@ -110,7 +110,7 @@ void m68k_tcg_init(void)
>      store_dummy = tcg_global_mem_new(TCG_AREG0, -8, "NULL");
>  
>  #define GEN_HELPER 2
> -#include "helpers.h"
> +#include "helper.h"
>  }
>  
>  static inline void qemu_assert(int cond, const char *msg)

Reviewed-by: Stefan Weil <sw@weilnetz.de>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 5/8] tcg: Remove stray semi-colons from target-*/helper.h
  2013-09-15  0:03 ` [Qemu-devel] [PATCH 5/8] tcg: Remove stray semi-colons from target-*/helper.h Richard Henderson
@ 2013-09-15  7:03   ` Stefan Weil
  2013-09-15 10:44     ` Peter Maydell
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Weil @ 2013-09-15  7:03 UTC (permalink / raw)
  To: Richard Henderson
  Cc: peter.maydell, Alexander Graf, qemu-devel, Blue Swirl, PowerPC,
	Paul Brook, Edgar E. Iglesias, aurelien

Am 15.09.2013 02:03, schrieb Richard Henderson:
> During GEN_HELPER=1, these are actually stray top-level semi-colons
> which are technically invalid ISO C, but GCC accepts as an extension.
> If we added enough __extension__ markers that we could dare use
> -Wpedantic, we'd see
>
>   warning: ISO C does not allow extra ‘;’ outside of a function
>
> This will become a hard error in the next patch, wherein those ; will
> appear in the middle of a data structure.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---

Instead of removing the semicolons from the DEF_HELPER_x lines,
I'd prefer removing them from the DEF_HELPER_FLAGS_x definitions.

Code formatters and static code analyzers (maybe humans, too) prefer
lines which look like valid C syntax, therefore

DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64);

is better for such tools than

DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64)

The compiler will also complain if someone adds a new DEF_HELPER_FLAGS_x
without semicolon in the first case, but it won't complain if someone adds
it with semicolon in the second case.

Regards,
Stefan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 5/8] tcg: Remove stray semi-colons from target-*/helper.h
  2013-09-15  7:03   ` Stefan Weil
@ 2013-09-15 10:44     ` Peter Maydell
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2013-09-15 10:44 UTC (permalink / raw)
  To: Stefan Weil
  Cc: Alexander Graf, QEMU Developers, Blue Swirl, open list:PowerPC,
	Paul Brook, Edgar E. Iglesias, Aurelien Jarno, Richard Henderson

On 15 September 2013 08:03, Stefan Weil <sw@weilnetz.de> wrote:
> Instead of removing the semicolons from the DEF_HELPER_x lines,
> I'd prefer removing them from the DEF_HELPER_FLAGS_x definitions.
>
> Code formatters and static code analyzers (maybe humans, too) prefer
> lines which look like valid C syntax, therefore
>
> DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64);
>
> is better for such tools than
>
> DEF_HELPER_FLAGS_1(ctpop, TCG_CALL_NO_RWG_SE, i64, i64)

I agree it looks nicer, but for this kind of multipurpose macro definition
where one of the redefinitions is used for things like creating a
data structure (eg Richard's patch 6/8 in this series) it just doesn't
work, because the required separator for entries in an array is a comma,
not a semicolon.

It's just an unavoidable cost of doing this with the preprocessor
rather than (say) a custom little language which we parsed with
a python or perl script.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-09-15 10:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-15  0:03 [Qemu-devel] [PATCH 0/8] tcg: Tidy helpers registration Richard Henderson
2013-09-15  0:03 ` [Qemu-devel] [PATCH 1/8] tcg: Delete tcg_helper_get_name declaration Richard Henderson
2013-09-15  6:14   ` Stefan Weil
2013-09-15  0:03 ` [Qemu-devel] [PATCH 2/8] tcg: Use a GHashTable for tcg_find_helper Richard Henderson
2013-09-15  6:32   ` Stefan Weil
2013-09-15  0:03 ` [Qemu-devel] [PATCH 3/8] target-m68k: Rename helpers.h to helper.h Richard Henderson
2013-09-15  6:35   ` Stefan Weil
2013-09-15  0:03 ` [Qemu-devel] [PATCH 4/8] tcg: Move helper registration into tcg_context_init Richard Henderson
2013-09-15  0:03 ` [Qemu-devel] [PATCH 5/8] tcg: Remove stray semi-colons from target-*/helper.h Richard Henderson
2013-09-15  7:03   ` Stefan Weil
2013-09-15 10:44     ` Peter Maydell
2013-09-15  0:03 ` [Qemu-devel] [PATCH 6/8] tcg: Put target helper data into an array Richard Henderson
2013-09-15  0:03 ` [Qemu-devel] [PATCH 7/8] tcg: Add tcg-runtime.c helpers to all_helpers Richard Henderson
2013-09-15  0:03 ` [Qemu-devel] [PATCH 8/8] tcg: Merge tcg_register_helper into tcg_context_init Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).