From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53854) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKwY6-0007NV-G6 for qemu-devel@nongnu.org; Sat, 14 Sep 2013 16:34:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VKwXy-0001IJ-2U for qemu-devel@nongnu.org; Sat, 14 Sep 2013 16:34:34 -0400 Received: from mail-pa0-x230.google.com ([2607:f8b0:400e:c03::230]:59559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKwXx-0001IF-RJ for qemu-devel@nongnu.org; Sat, 14 Sep 2013 16:34:26 -0400 Received: by mail-pa0-f48.google.com with SMTP id kp13so3844958pab.21 for ; Sat, 14 Sep 2013 13:34:24 -0700 (PDT) Sender: Richard Henderson Message-ID: <5234C84D.9090900@twiddle.net> Date: Sat, 14 Sep 2013 13:34:21 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1379015124-21055-1-git-send-email-sw@weilnetz.de> <52340DAA.1070807@weilnetz.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] tci: Detect function argument alignment List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Stefan Weil , Michael Walle , qemu-devel , Jia Liu 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~