Re: [PATCH 3/3] tcg: Move ffi_cif pointer into TCGHelperInfo

[Philippe Mathieu-Daudé]

On 22/11/22 19:08, Philippe Mathieu-Daudé wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
>
> Instead of requiring a separate hash table lookup,
> put a pointer to the CIF into TCGHelperInfo.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20221111074101.2069454-27-richard.henderson@linaro.org>
> [PMD: Split from bigger patch]
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tcg/tcg-internal.h |  7 +++++++
>   tcg/tcg.c          | 26 ++++++++++++++------------
>   2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/tcg/tcg-internal.h b/tcg/tcg-internal.h
> index c7e87e193d..6e50aeba3a 100644
> --- a/tcg/tcg-internal.h
> +++ b/tcg/tcg-internal.h
> @@ -25,6 +25,10 @@
>   #ifndef TCG_INTERNAL_H
>   #define TCG_INTERNAL_H
>   
> +#ifdef CONFIG_TCG_INTERPRETER
> +#include <ffi.h>
> +#endif
> +
>   #define TCG_HIGHWATER 1024
>   
>   /*
> @@ -57,6 +61,9 @@ typedef struct TCGCallArgumentLoc {
>   typedef struct TCGHelperInfo {
>       void *func;
>       const char *name;
> +#ifdef CONFIG_TCG_INTERPRETER
> +    ffi_cif *cif;
> +#endif
>       unsigned typemask           : 32;
>       unsigned flags              : 8;
>       unsigned nr_in              : 8;
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 9b24b4d863..d6a3036412 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c

I forgot this hunk:

-- >8 --
@@ -62,10 +62,6 @@
 #include "tcg/tcg-ldst.h"
 #include "tcg-internal.h"

-#ifdef CONFIG_TCG_INTERPRETER
-#include <ffi.h>
-#endif
-

---

> @@ -552,8 +552,6 @@ static TCGHelperInfo all_helpers[] = {
>   static GHashTable *helper_table;
>   
>   #ifdef CONFIG_TCG_INTERPRETER
> -static GHashTable *ffi_table;
> -
>   static ffi_type *typecode_to_ffi(int argmask)
>   {
>       switch (argmask) {
> @@ -576,9 +574,11 @@ static ffi_type *typecode_to_ffi(int argmask)
>   static void init_ffi_layouts(void)
>   {
>       /* g_direct_hash/equal for direct comparisons on uint32_t.  */
> -    ffi_table = g_hash_table_new(NULL, NULL);
> +    GHashTable *ffi_table = g_hash_table_new(NULL, NULL);
> +
>       for (int i = 0; i < ARRAY_SIZE(all_helpers); ++i) {
> -        uint32_t typemask = all_helpers[i].typemask;
> +        TCGHelperInfo *info = &all_helpers[i];
> +        unsigned typemask = info->typemask;
>           gpointer hash = (gpointer)(uintptr_t)typemask;
>           struct {
>               ffi_cif cif;
> @@ -586,8 +586,11 @@ static void init_ffi_layouts(void)
>           } *ca;
>           ffi_status status;
>           int nargs;
> +        ffi_cif *cif;
>   
> -        if (g_hash_table_lookup(ffi_table, hash)) {
> +        cif = g_hash_table_lookup(ffi_table, hash);
> +        if (cif) {
> +            info->cif = cif;
>               continue;
>           }
>   
> @@ -611,8 +614,12 @@ static void init_ffi_layouts(void)
>                                 ca->cif.rtype, ca->cif.arg_types);
>           assert(status == FFI_OK);
>   
> -        g_hash_table_insert(ffi_table, hash, (gpointer)&ca->cif);
> +        cif = &ca->cif;
> +        info->cif = cif;
> +        g_hash_table_insert(ffi_table, hash, (gpointer)cif);
>       }
> +
> +    g_hash_table_destroy(ffi_table);
>   }
>   #endif /* CONFIG_TCG_INTERPRETER */
>   
> @@ -4413,12 +4420,7 @@ static void tcg_reg_alloc_call(TCGContext *s, TCGOp *op)
>       }
>   
>   #ifdef CONFIG_TCG_INTERPRETER
> -    {
> -        gpointer hash = (gpointer)(uintptr_t)info->typemask;
> -        ffi_cif *cif = g_hash_table_lookup(ffi_table, hash);
> -        assert(cif != NULL);
> -        tcg_out_call(s, tcg_call_func(op), cif);
> -    }
> +    tcg_out_call(s, tcg_call_func(op), info->cif);
>   #else
>       tcg_out_call(s, tcg_call_func(op));
>   #endif