| Message ID | 20181025172057.20414-7-cota@braap.org |
|---|---|
| State | New |
| Headers | show |
| Series | Plugin support | expand |
Emilio G. Cota <cota@braap.org> writes: > This will allow us to add TCG helpers at run-time. > > While at it, rename tcg_find_helper to tcg_helper_find for consistency > with the added tcg_helper_foo functions. > > Signed-off-by: Emilio G. Cota <cota@braap.org> > --- > tcg/tcg.c | 59 +++++++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 49 insertions(+), 10 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index e85133ef05..65da3c5dbf 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -33,6 +33,7 @@ > #include "qemu/error-report.h" > #include "qemu/cutils.h" > #include "qemu/host-utils.h" > +#include "qemu/xxhash.h" > #include "qemu/timer.h" > > /* Note: the long term plan is to reduce the dependencies on the QEMU > @@ -879,13 +880,46 @@ typedef struct TCGHelperInfo { > static const TCGHelperInfo all_helpers[] = { > #include "exec/helper-tcg.h" > }; > -static GHashTable *helper_table; > +static struct qht helper_table; > +static bool helper_table_inited; Having a flag for initialisation seems a little excessive considering we've moved that initialisation into tcg_context_init() which has to be called before we do anything TCG related. > static int indirect_reg_alloc_order[ARRAY_SIZE(tcg_target_reg_alloc_order)]; > static void process_op_defs(TCGContext *s); > static TCGTemp *tcg_global_reg_new_internal(TCGContext *s, TCGType type, > TCGReg reg, const char *name); > > +static inline uint32_t tcg_helper_func_hash(const void *func) > +{ > + return qemu_xxhash2((uint64_t)func); > +} > + > +static bool tcg_helper_cmp(const void *ap, const void *bp) > +{ > + const TCGHelperInfo *a = ap; > + const TCGHelperInfo *b = bp; > + > + return a->func == b->func && > + a->flags == b->flags && > + a->sizemask == b->sizemask && > + !strcmp(a->name, b->name); > +} > + > +static bool tcg_helper_lookup_cmp(const void *obj, const void *func) > +{ > + const TCGHelperInfo *info = obj; > + > + return info->func == func; > +} > + > +static void tcg_helper_insert(const TCGHelperInfo *info) > +{ > + uint32_t hash = tcg_helper_func_hash(info->func); > + bool inserted; > + > + inserted = qht_insert(&helper_table, (void *)info, hash, NULL); > + g_assert(inserted); > +} > + > void tcg_context_init(TCGContext *s) > { > int op, total_args, n, i; > @@ -919,13 +953,13 @@ void tcg_context_init(TCGContext *s) > } > > /* Register helpers. */ > - /* Use g_direct_hash/equal for direct pointer comparisons on func. */ > - helper_table = g_hash_table_new(NULL, NULL); > + qht_init(&helper_table, tcg_helper_cmp, ARRAY_SIZE(all_helpers), > + QHT_MODE_AUTO_RESIZE); > > for (i = 0; i < ARRAY_SIZE(all_helpers); ++i) { > - g_hash_table_insert(helper_table, (gpointer)all_helpers[i].func, > - (gpointer)&all_helpers[i]); > + tcg_helper_insert(&all_helpers[i]); > } > + helper_table_inited = true; so I think we can drop this and... > > tcg_target_init(s); > process_op_defs(s); > @@ -1620,9 +1654,10 @@ void tcg_gen_callN(void *func, TCGTemp *ret, int nargs, TCGTemp **args) > int i, real_args, nb_rets, pi; > unsigned sizemask, flags; > TCGHelperInfo *info; > + uint32_t hash = tcg_helper_func_hash(func); > TCGOp *op; > > - info = g_hash_table_lookup(helper_table, (gpointer)func); > + info = qht_lookup_custom(&helper_table, func, hash, tcg_helper_lookup_cmp); > flags = info->flags; > sizemask = info->sizemask; > > @@ -1825,11 +1860,15 @@ static char *tcg_get_arg_str(TCGContext *s, char *buf, > } > > /* Find helper name. */ > -static inline const char *tcg_find_helper(TCGContext *s, uintptr_t val) > +static inline const char *tcg_helper_find(TCGContext *s, uintptr_t val) > { > const char *ret = NULL; > - if (helper_table) { > - TCGHelperInfo *info = g_hash_table_lookup(helper_table, (gpointer)val); > + if (helper_table_inited) { change this to a assert(helper_table.cmp) if you really want to. > + uint32_t hash = tcg_helper_func_hash((void *)val); > + TCGHelperInfo *info; > + > + info = qht_lookup_custom(&helper_table, (void *)val, hash, > + tcg_helper_lookup_cmp); > if (info) { > ret = info->name; > } > @@ -1919,7 +1958,7 @@ void tcg_dump_ops(TCGContext *s) > > /* function name, flags, out args */ > col += qemu_log(" %s %s,$0x%" TCG_PRIlx ",$%d", def->name, > - tcg_find_helper(s, op->args[nb_oargs + nb_iargs]), > + tcg_helper_find(s, op->args[nb_oargs + nb_iargs]), > op->args[nb_oargs + nb_iargs + 1], nb_oargs); > for (i = 0; i < nb_oargs; i++) { > col += qemu_log(",%s", tcg_get_arg_str(s, buf, sizeof(buf), Otherwise: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> -- Alex Bennée
Emilio G. Cota <cota@braap.org> writes: > This will allow us to add TCG helpers at run-time. > > While at it, rename tcg_find_helper to tcg_helper_find for consistency > with the added tcg_helper_foo functions. > > Signed-off-by: Emilio G. Cota <cota@braap.org> > --- > tcg/tcg.c | 59 +++++++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 49 insertions(+), 10 deletions(-) > > diff --git a/tcg/tcg.c b/tcg/tcg.c > index e85133ef05..65da3c5dbf 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -33,6 +33,7 @@ > #include "qemu/error-report.h" > #include "qemu/cutils.h" > #include "qemu/host-utils.h" > +#include "qemu/xxhash.h" > #include "qemu/timer.h" > > /* Note: the long term plan is to reduce the dependencies on the QEMU > @@ -879,13 +880,46 @@ typedef struct TCGHelperInfo { > static const TCGHelperInfo all_helpers[] = { > #include "exec/helper-tcg.h" > }; > -static GHashTable *helper_table; > +static struct qht helper_table; > +static bool helper_table_inited; > > static int indirect_reg_alloc_order[ARRAY_SIZE(tcg_target_reg_alloc_order)]; > static void process_op_defs(TCGContext *s); > static TCGTemp *tcg_global_reg_new_internal(TCGContext *s, TCGType type, > TCGReg reg, const char *name); > > +static inline uint32_t tcg_helper_func_hash(const void *func) > +{ > + return qemu_xxhash2((uint64_t)func); > +} I needed to do this: modified tcg/tcg.c @@ -884,7 +884,7 @@ static TCGTemp *tcg_global_reg_new_internal(TCGContext *s, TCGType type, static inline uint32_t tcg_helper_func_hash(const void *func) { - return qemu_xxhash2((uint64_t)func); + return qemu_xxhash2((uint64_t)(uintptr_t)func); } To prevent the compiler complaining about: tcg.c:887:25: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] > + > +static bool tcg_helper_cmp(const void *ap, const void *bp) > +{ > + const TCGHelperInfo *a = ap; > + const TCGHelperInfo *b = bp; > + > + return a->func == b->func && > + a->flags == b->flags && > + a->sizemask == b->sizemask && > + !strcmp(a->name, b->name); > +} > + > +static bool tcg_helper_lookup_cmp(const void *obj, const void *func) > +{ > + const TCGHelperInfo *info = obj; > + > + return info->func == func; > +} > + > +static void tcg_helper_insert(const TCGHelperInfo *info) > +{ > + uint32_t hash = tcg_helper_func_hash(info->func); > + bool inserted; > + > + inserted = qht_insert(&helper_table, (void *)info, hash, NULL); > + g_assert(inserted); > +} > + > void tcg_context_init(TCGContext *s) > { > int op, total_args, n, i; > @@ -919,13 +953,13 @@ void tcg_context_init(TCGContext *s) > } > > /* Register helpers. */ > - /* Use g_direct_hash/equal for direct pointer comparisons on func. */ > - helper_table = g_hash_table_new(NULL, NULL); > + qht_init(&helper_table, tcg_helper_cmp, ARRAY_SIZE(all_helpers), > + QHT_MODE_AUTO_RESIZE); > > for (i = 0; i < ARRAY_SIZE(all_helpers); ++i) { > - g_hash_table_insert(helper_table, (gpointer)all_helpers[i].func, > - (gpointer)&all_helpers[i]); > + tcg_helper_insert(&all_helpers[i]); > } > + helper_table_inited = true; > > tcg_target_init(s); > process_op_defs(s); > @@ -1620,9 +1654,10 @@ void tcg_gen_callN(void *func, TCGTemp *ret, int nargs, TCGTemp **args) > int i, real_args, nb_rets, pi; > unsigned sizemask, flags; > TCGHelperInfo *info; > + uint32_t hash = tcg_helper_func_hash(func); > TCGOp *op; > > - info = g_hash_table_lookup(helper_table, (gpointer)func); > + info = qht_lookup_custom(&helper_table, func, hash, tcg_helper_lookup_cmp); > flags = info->flags; > sizemask = info->sizemask; > > @@ -1825,11 +1860,15 @@ static char *tcg_get_arg_str(TCGContext *s, char *buf, > } > > /* Find helper name. */ > -static inline const char *tcg_find_helper(TCGContext *s, uintptr_t val) > +static inline const char *tcg_helper_find(TCGContext *s, uintptr_t val) > { > const char *ret = NULL; > - if (helper_table) { > - TCGHelperInfo *info = g_hash_table_lookup(helper_table, (gpointer)val); > + if (helper_table_inited) { > + uint32_t hash = tcg_helper_func_hash((void *)val); > + TCGHelperInfo *info; > + > + info = qht_lookup_custom(&helper_table, (void *)val, hash, > + tcg_helper_lookup_cmp); > if (info) { > ret = info->name; > } > @@ -1919,7 +1958,7 @@ void tcg_dump_ops(TCGContext *s) > > /* function name, flags, out args */ > col += qemu_log(" %s %s,$0x%" TCG_PRIlx ",$%d", def->name, > - tcg_find_helper(s, op->args[nb_oargs + nb_iargs]), > + tcg_helper_find(s, op->args[nb_oargs + nb_iargs]), > op->args[nb_oargs + nb_iargs + 1], nb_oargs); > for (i = 0; i < nb_oargs; i++) { > col += qemu_log(",%s", tcg_get_arg_str(s, buf, sizeof(buf), -- Alex Bennée
On Wed, Nov 14, 2018 at 14:41:53 +0000, Alex Bennée wrote: > Emilio G. Cota <cota@braap.org> writes: (snip) > > -static GHashTable *helper_table; > > +static struct qht helper_table; > > +static bool helper_table_inited; > > Having a flag for initialisation seems a little excessive considering > we've moved that initialisation into tcg_context_init() which has to be > called before we do anything TCG related. (snip) > > + helper_table_inited = true; > > so I think we can drop this and... (snip) > > +static inline const char *tcg_helper_find(TCGContext *s, uintptr_t val) > > { > > const char *ret = NULL; > > - if (helper_table) { > > - TCGHelperInfo *info = g_hash_table_lookup(helper_table, (gpointer)val); > > + if (helper_table_inited) { > > change this to a assert(helper_table.cmp) if you really want to. I like this suggestion. The only caller of tcg_helper_find is tcg_dump_ops, which is unlikely to be called on an uninitialized TCGContext. I have added this to v2, without the assert. (snip) > Otherwise: > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Thanks! E.
On Wed, Nov 14, 2018 at 16:11:35 +0000, Alex Bennée wrote: > > Emilio G. Cota <cota@braap.org> writes: (snip) > I needed to do this: > > modified tcg/tcg.c > @@ -884,7 +884,7 @@ static TCGTemp *tcg_global_reg_new_internal(TCGContext *s, TCGType type, > > static inline uint32_t tcg_helper_func_hash(const void *func) > { > - return qemu_xxhash2((uint64_t)func); > + return qemu_xxhash2((uint64_t)(uintptr_t)func); > } > > To prevent the compiler complaining about: > > tcg.c:887:25: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] Fixed, thanks. E.
diff --git a/tcg/tcg.c b/tcg/tcg.c index e85133ef05..65da3c5dbf 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -33,6 +33,7 @@ #include "qemu/error-report.h" #include "qemu/cutils.h" #include "qemu/host-utils.h" +#include "qemu/xxhash.h" #include "qemu/timer.h" /* Note: the long term plan is to reduce the dependencies on the QEMU @@ -879,13 +880,46 @@ typedef struct TCGHelperInfo { static const TCGHelperInfo all_helpers[] = { #include "exec/helper-tcg.h" }; -static GHashTable *helper_table; +static struct qht helper_table; +static bool helper_table_inited; static int indirect_reg_alloc_order[ARRAY_SIZE(tcg_target_reg_alloc_order)]; static void process_op_defs(TCGContext *s); static TCGTemp *tcg_global_reg_new_internal(TCGContext *s, TCGType type, TCGReg reg, const char *name); +static inline uint32_t tcg_helper_func_hash(const void *func) +{ + return qemu_xxhash2((uint64_t)func); +} + +static bool tcg_helper_cmp(const void *ap, const void *bp) +{ + const TCGHelperInfo *a = ap; + const TCGHelperInfo *b = bp; + + return a->func == b->func && + a->flags == b->flags && + a->sizemask == b->sizemask && + !strcmp(a->name, b->name); +} + +static bool tcg_helper_lookup_cmp(const void *obj, const void *func) +{ + const TCGHelperInfo *info = obj; + + return info->func == func; +} + +static void tcg_helper_insert(const TCGHelperInfo *info) +{ + uint32_t hash = tcg_helper_func_hash(info->func); + bool inserted; + + inserted = qht_insert(&helper_table, (void *)info, hash, NULL); + g_assert(inserted); +} + void tcg_context_init(TCGContext *s) { int op, total_args, n, i; @@ -919,13 +953,13 @@ void tcg_context_init(TCGContext *s) } /* Register helpers. */ - /* Use g_direct_hash/equal for direct pointer comparisons on func. */ - helper_table = g_hash_table_new(NULL, NULL); + qht_init(&helper_table, tcg_helper_cmp, ARRAY_SIZE(all_helpers), + QHT_MODE_AUTO_RESIZE); for (i = 0; i < ARRAY_SIZE(all_helpers); ++i) { - g_hash_table_insert(helper_table, (gpointer)all_helpers[i].func, - (gpointer)&all_helpers[i]); + tcg_helper_insert(&all_helpers[i]); } + helper_table_inited = true; tcg_target_init(s); process_op_defs(s); @@ -1620,9 +1654,10 @@ void tcg_gen_callN(void *func, TCGTemp *ret, int nargs, TCGTemp **args) int i, real_args, nb_rets, pi; unsigned sizemask, flags; TCGHelperInfo *info; + uint32_t hash = tcg_helper_func_hash(func); TCGOp *op; - info = g_hash_table_lookup(helper_table, (gpointer)func); + info = qht_lookup_custom(&helper_table, func, hash, tcg_helper_lookup_cmp); flags = info->flags; sizemask = info->sizemask; @@ -1825,11 +1860,15 @@ static char *tcg_get_arg_str(TCGContext *s, char *buf, } /* Find helper name. */ -static inline const char *tcg_find_helper(TCGContext *s, uintptr_t val) +static inline const char *tcg_helper_find(TCGContext *s, uintptr_t val) { const char *ret = NULL; - if (helper_table) { - TCGHelperInfo *info = g_hash_table_lookup(helper_table, (gpointer)val); + if (helper_table_inited) { + uint32_t hash = tcg_helper_func_hash((void *)val); + TCGHelperInfo *info; + + info = qht_lookup_custom(&helper_table, (void *)val, hash, + tcg_helper_lookup_cmp); if (info) { ret = info->name; } @@ -1919,7 +1958,7 @@ void tcg_dump_ops(TCGContext *s) /* function name, flags, out args */ col += qemu_log(" %s %s,$0x%" TCG_PRIlx ",$%d", def->name, - tcg_find_helper(s, op->args[nb_oargs + nb_iargs]), + tcg_helper_find(s, op->args[nb_oargs + nb_iargs]), op->args[nb_oargs + nb_iargs + 1], nb_oargs); for (i = 0; i < nb_oargs; i++) { col += qemu_log(",%s", tcg_get_arg_str(s, buf, sizeof(buf),
This will allow us to add TCG helpers at run-time. While at it, rename tcg_find_helper to tcg_helper_find for consistency with the added tcg_helper_foo functions. Signed-off-by: Emilio G. Cota <cota@braap.org> --- tcg/tcg.c | 59 +++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 10 deletions(-)