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 2011/2/24 01:20 AM, David Gilbert wrote:
> On 23 February 2011 16:56, Chung-Lin Tang <cltang@linaro.org> wrote:
>> On 2011/2/24 12:20 AM, David Gilbert wrote:
>>> On 23 February 2011 13:25, Chung-Lin Tang <cltang@linaro.org> wrote:
>>>> On 2011/2/23 09:11 PM, David Gilbert wrote:
>>>>> In ARM's ffi_prep_cif_machdep I have:
>>>>>
>>>>> +#ifdef __ARM_PCS_VFP
>>>>> +  /* VFP uses the standard ABI for variadic functions */
>>>>> +  if ((cif->abi == FFI_VFP) && (cif->nfixedargs != 0))
>>>>> +    cif->abi = FFI_SYSV;
>>>>> +#endif
>>>>>
>>>>> and that's the only active change that's needed.
>>>>
>>>> I suggest removing the #ifdef __ARM_PCS_VFP #ifdef.  We always have
>>>> support for both FFI_SYSV and FFI_VFP compiled in, so it doesn't make
>>>> sense to leave this bit of logic out when we are under __ARM_PCS.
>>>
>>> I could do, although cif->nfixedargs is also not present except in the
>>> VFP case, so
>>> it would still need an ifdef.
>>>
>>> I did that since I thought it best not to change the layout of the CIF
>>> structure for
>>> existing ARM builds.
>>
>> Sigh...you're right about the layout issue :(
>>
>> Still I think it's best we keep the full FFI_SYSV/VFP capabilities in,
>> or we'll have the weird case of "FFI_VFP with buggy variadics" under softfp.
> 
> 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".

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

Chung-Lin


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