Re: [Qemu-devel] [PATCH] tci: Detect function argument alignment

[Richard Henderson]

Michael Walle and Jia Liu, see below wrt minor lm32 and openrisc mistakes.

On 09/14/2013 02:51 AM, Peter Maydell wrote:
>> I had a look on libffi now and don't see how it could solve my problem.
>> As far as I could see, libffi must be ported to new architectures, so
>> its use would restrict the portability of TCI.
> 
> Yes, but it's somebody else's problem to port it, not ours.
> I present it mostly as an alternative to doing it the hard way.

In particular, libffi is used by gcc within its libjava implementation.
Which tends to mean that every (non-embedded) target to which gcc is
ported also gets a port of libffi.

It is tempting to want to avoid the extra external build dependency, but
we have so many now that working too hard to avoid another seems silly.


>> There is a drawback of that solution: it needs modifications in the TCG
>> opcode generation which would no longer be identical to all other TCG
>> targets (or I'd have to search the given address of the helper function
>> in a lookup table which would cost too much time).
> 
> Nobody's running TCI for the performance benefit :-) Use a hash
> table, there's one in glib and it won't have much overhead at all for lookups.

Indeed.  And one can minimize that lookup cost by doing that when building
the TB, as opposed to when we interpret it.

As far as actually interfacing with libffi, one builds ffi_cif structures
that describe the arguments and return value.  Given the contents of the
target-foo/helper.h file, we ought to be able to statically build data
structures of the ffi_type* inputs, and use those to generate all of the
ffi_cif structures within tcg_target_init.

We probably need to clean up the interface from target-foo to tcg.c a bit
to facilitate this.  In the process, we'd be able to reduce some startup
overhead wrt cpu_translate_init.  We currently use

    /* register helpers */
#define GEN_HELPER 2
#include "helper.h"

which expands to, in the case of i386, 514 calls to tcg_register_helper.
We ought to be able to build a static table within tcg.o instead.

It also means that one wouldn't be able to *forget* to register helpers
on the target side.  The lm32 and openrisc target have forgotten that.

That's not to say that we don't still need changes to tcg.c to allow
ffi_call to actually be used.  In particular, getting the data into the
avalue array is non-trivial.  As is getting the results out of rvalue.
One needs to store the argument data such that one knows where the
argument boundaries are.  I.e., at minimum one still needs to know the
sizes of each argument.  That minimum could be passing along the "sizemask"
value used during tcg_gen_callN.  Currently we discard that value there
in tcg_gen_callN, but that's not to say we couldn't store it in the
INDEX_op_call opcode.  And pass it along to tcg_gen_opc as args[1].

Actually, with just that last change we wouldn't necessarily have to
clean up the target-foo/helper.h interface first.  We'd have just enough
info to build the ffi_cif on demand during translation of the INDEX_op_call.


r~