From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4664 invoked by alias); 25 Feb 2011 12:56:58 -0000 Received: (qmail 4654 invoked by uid 22791); 25 Feb 2011 12:56:57 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,TW_BF X-Spam-Check-By: sourceware.org Received: from mail-qw0-f41.google.com (HELO mail-qw0-f41.google.com) (209.85.216.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 25 Feb 2011 12:56:52 +0000 Received: by qwd7 with SMTP id 7so1589512qwd.0 for ; Fri, 25 Feb 2011 04:56:50 -0800 (PST) MIME-Version: 1.0 Received: by 10.224.67.198 with SMTP id s6mr1983177qai.138.1298638610217; Fri, 25 Feb 2011 04:56:50 -0800 (PST) Received: by 10.224.205.134 with HTTP; Fri, 25 Feb 2011 04:56:50 -0800 (PST) In-Reply-To: <4D654655.5000503@linaro.org> References: <20110222154022.GA29862@davesworkthinkpad> <4D650ADE.6040609@linaro.org> <4D653C3D.2050107@linaro.org> <4D654655.5000503@linaro.org> Date: Fri, 25 Feb 2011 12:56:00 -0000 Message-ID: Subject: Re: [PATCH] Add variadic support From: David Gilbert To: Chung-Lin Tang Cc: Anthony Green , libffi-discuss@sourceware.org, Marcus.Shawcroft@arm.com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact libffi-discuss-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libffi-discuss-owner@sourceware.org X-SW-Source: 2011/txt/msg00088.txt.bz2 On 23 February 2011 17:39, Chung-Lin Tang wrote: > On 2011/2/24 01:20 AM, David Gilbert wrote: >> Wouldn't it make more sense to ban FFI_VFP under softfp? > > Well, that would be simple :) > > The current code paths already have support for both ABIs; =A0since this > is a "foreign function interface", I thought it made sense to support > all ABIs on that platform, for the most flexibility, so I wrote it that w= ay. > > If we were to make softfp/hardfp a libffi build-time configuration, > we're already wasting some runtime cost now. In fact, we would have no > need for 'FFI_VFP' at all; FFI_SYSV would just mean "the ABI". Yes, you're right - I hadn't thought of calling VFP code from a SYSV progra= m. >>> I suggest moving the nfixedargs field into the ARM FFI_EXTRA_CIF_FIELDS, >>> placed after the current VFP fields; this means doing away with the >>> FFI_TARGET_SPECIFIC_VARIADIC symbol, and probably having a >>> machine-specific ffi_prep_cif_var_machdep() function, analogous to the >>> current ffi_prep_cif_machdep(). >> >> That would mean changing every back end to add that function or again > > Okay, then we should keep the preprocessor symbol then; we can then use > the basic ffi_prep_cif() as the default ffi_prep_cif_var() implementation. > >> ifdefing for it, and the belief is that storing the nfixedargs value >> is likely to >> be useful to some backends who have to do different calling conventions = for >> variadics anyway. > > The keyword is "some" :) > Some means it is not universally needed for supporting variadics, thus > probably should be in FFI_EXTRA_CIF_FIELDS. OK, I can see we can do this if I switch over to making a ffi_prep_cif_var_machdep, which isn't too bad. > On 2011/2/24, Chung-Lin Tang wrote: >>>> I suggest moving the nfixedargs field into the ARM FFI_EXTRA_CIF_FIELD= S, >>>> >> placed after the current VFP fields; this means doing away with the >>>> >> FFI_TARGET_SPECIFIC_VARIADIC symbol, and probably having a >>>> >> machine-specific ffi_prep_cif_var_machdep() function, analogous to = the >>>> >> current ffi_prep_cif_machdep(). >>> > >>> > That would mean changing every back end to add that function or again >> Okay, then we should keep the preprocessor symbol then; we can then use >> the basic ffi_prep_cif() as the default ffi_prep_cif_var() implementatio= n. >> > > I just wanted to add that, instead of a new ffi_prep_cif_var_machdep() > function for all backends to add, thus needing a preprocessor symbol to > set a 'default' implementation (to avoid changing every port), we can do > this: add a flag value, passed by cif->flags, into > ffi_prep_cif_machdep() to indicate variadic function processing. We'll > just need a new flag value CPP symbol, but avoid an ugly #ifndef in the > machine-independent code. > > I'm suggesting this because I see cif->flags being overwritten anyways > by the backend ffi_prep_cif_machdep() implementations I see, so it > should be valid to pass in values for this use. The flag doesn't provide the space for the generic code to pass the extra information (the number of fixed args) to ffi_prep_cif_machdep_var or the equivalent code in ffi_prep_cif_machdep. I think the way to move it in the direction of what you're suggesting is: 1) as per my code have an ffi_prep_cif and ffi_prep_cif_var that both call ffi_prep_cif_core 2) Explicitly pass ffi_prep_cif_core the extra parameter rather than storing it in cif. 3) Still keep an ifdef to say if the backend has explicit variadic support, if that ifdef is set then ffi_prep_cif_core will call ffi_prep_cif_machdep_var when appropriate. 4) ffi_prep_cif_machdep_var will get the argument count passed as explicit parameters, then it can process them as it feels fit including storing them in an EXTRA_CIF_FIELD if the backend wants to. Thoughts? Dave > > Chung-Lin >