Re: [PATCH v5 22/57] tcg: Build ffi data structures for helpers

[Richard Henderson]

On 3/16/21 4:35 PM, Philippe Mathieu-Daudé wrote:
> Hi Richard,
>
> On 3/11/21 3:39 PM, Richard Henderson wrote:
> > We will shortly use libffi for tci, as that is the only
> > portable way of calling arbitrary functions.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   meson.build                            |   9 +-
> >   include/exec/helper-ffi.h              | 115 +++++++++++++++++++++++++
> >   include/exec/helper-tcg.h              |  24 ++++--
> >   target/hppa/helper.h                   |   2 +
> >   target/i386/ops_sse_header.h           |   6 ++
> >   target/m68k/helper.h                   |   1 +
> >   target/ppc/helper.h                    |   3 +
> >   tcg/tcg.c                              |  20 +++++
> >   tests/docker/dockerfiles/fedora.docker |   1 +
> >   9 files changed, 172 insertions(+), 9 deletions(-)
> >   create mode 100644 include/exec/helper-ffi.h
>
> > diff --git a/include/exec/helper-tcg.h b/include/exec/helper-tcg.h
> > index 27870509a2..a71b848576 100644
> > --- a/include/exec/helper-tcg.h
> > +++ b/include/exec/helper-tcg.h
> > @@ -10,50 +10,57 @@
> >      to get all the macros expanded first.  */
> >   #define str(s) #s
> >   
> > +#ifdef CONFIG_TCG_INTERPRETER
> > +# define DO_CIF(NAME)  .cif = &cif_##NAME,
>
> The comma in the macro bugs me... What about adding it as last field?

Hmm, I think I can rearrange things so that happens.

The first iteration(s) of these two patches Knew Things about the layout of 
TCGHelperInfo, and the cif pointer needed to be immediately after the function 
pointer.

But in the post-uint32_t conversion part of this patch set that's no longer 
true.

So I think I can bodge things here pre-uint32_t conversion too.

> > +++ b/tests/docker/dockerfiles/fedora.docker
> > @@ -32,6 +32,7 @@ ENV PACKAGES \
> >       libcurl-devel \
> >       libepoxy-devel \
> >       libfdt-devel \
> > +    libffi-devel \
> >       libiscsi-devel \
> >       libjpeg-devel \
> >       libpmem-devel \
>
> What happened to the other Docker images?
>
> Otherwise the rest LGTM.

This is the one we use for the gitlab build-tci job.  Or rather, we did.  It 
looks like build-tci was changed to debian-all-test-cross recently...


r~