Re: [PATCH v6 08/26] tcg: Build ffi data structures for helpers

[Richard Henderson]

On 5/31/21 11:55 AM, Philippe Mathieu-Daudé wrote:
> Hi Richard,
>
> On 5/3/21 1:57 AM, Richard Henderson wrote:
> > Add libffi as a build requirement for TCI.
> > Add libffi to the dockerfiles to satisfy that requirement.
> >
> > Construct an ffi_cif structure for each unique typemask.
> > Record the result in a separate hash table for later lookup;
> > this allows helper_table to stay const.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   meson.build                                   |  9 ++-
> >   tcg/tcg.c                                     | 58 +++++++++++++++++++
> >   tests/docker/dockerfiles/alpine.docker        |  1 +
> >   tests/docker/dockerfiles/centos7.docker       |  1 +
> >   tests/docker/dockerfiles/centos8.docker       |  1 +
> >   tests/docker/dockerfiles/debian10.docker      |  1 +
> >   .../dockerfiles/fedora-i386-cross.docker      |  1 +
> >   .../dockerfiles/fedora-win32-cross.docker     |  1 +
> >   .../dockerfiles/fedora-win64-cross.docker     |  1 +
> >   tests/docker/dockerfiles/fedora.docker        |  1 +
> >   tests/docker/dockerfiles/ubuntu.docker        |  1 +
> >   tests/docker/dockerfiles/ubuntu1804.docker    |  1 +
> >   tests/docker/dockerfiles/ubuntu2004.docker    |  1 +
> >   13 files changed, 77 insertions(+), 1 deletion(-)
>
> > @@ -1135,6 +1152,47 @@ void tcg_context_init(TCGContext *s)
> >                               (gpointer)&all_helpers[i]);
> >       }
> >   
> > +#ifdef CONFIG_TCG_INTERPRETER
> > +    /* g_direct_hash/equal for direct comparisons on uint32_t.  */
>
> Why not use g_int_hash() then?

g_int_hash takes a pointer to an int; this stores the int directly as the hash 
key.


r~

> Otherwise,
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> > +    ffi_table = g_hash_table_new(NULL, NULL);
> > +    for (i = 0; i < ARRAY_SIZE(all_helpers); ++i) {
> > +        struct {
> > +            ffi_cif cif;
> > +            ffi_type *args[];
> > +        } *ca;
> > +        uint32_t typemask = all_helpers[i].typemask;
> > +        gpointer hash = (gpointer)(uintptr_t)typemask;
> > +        ffi_status status;
> > +        int nargs;
> > +
> > +        if (g_hash_table_lookup(ffi_table, hash)) {
> > +            continue;
> > +        }
> > +
> > +        /* Ignoring the return type, find the last non-zero field. */
> > +        nargs = 32 - clz32(typemask >> 3);
> > +        nargs = DIV_ROUND_UP(nargs, 3);
> > +
> > +        ca = g_malloc0(sizeof(*ca) + nargs * sizeof(ffi_type *));
> > +        ca->cif.rtype = typecode_to_ffi[typemask & 7];
> > +        ca->cif.nargs = nargs;
> > +
> > +        if (nargs != 0) {
> > +            ca->cif.arg_types = ca->args;
> > +            for (i = 0; i < nargs; ++i) {
> > +                int typecode = extract32(typemask, (i + 1) * 3, 3);
> > +                ca->args[i] = typecode_to_ffi[typecode];
> > +            }
> > +        }
> > +
> > +        status = ffi_prep_cif(&ca->cif, FFI_DEFAULT_ABI, nargs,
> > +                              ca->cif.rtype, ca->cif.arg_types);
> > +        assert(status == FFI_OK);
> > +
> > +        g_hash_table_insert(ffi_table, hash, (gpointer)&ca->cif);
> > +    }
> > +#endif
> > +
> >       tcg_target_init(s);
> >       process_op_defs(s);