This is the mail archive of the libffi-discuss@sourceware.org mailing list for the libffi project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add variadic support


On 23 February 2011 17:39, Chung-Lin Tang <cltang@linaro.org> 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; ?since 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 way.
>
> 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 program.

>>> 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_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.
>>
>
> 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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]