[libffi,ARM] VFP hard-float calling convention support

On 2010/11/20 20:40, Mikael Pettersson wrote:
> On Thu, 28 Oct 2010 14:11:12 -0400, Anthony Green wrote:
>   >
>   >  I'm committing this patch from Chung-Lin Tang, who provided the
>   >  following description:
>   >
>   >  "this patch implements VFP hard-float calling conventions, as a new ABI
>   >  for ARM. Asides from VFP calling convention bits, some additional
>   >  optimizations have been added to the current softfp assembly code
>   >  paths too.
> ...
>   >  2010-10-28  Chung-Lin Tang<cltang@codesourcery.com>
> ...
>
> This commit, r166032, caused a build error on ARM:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46508
>
> In short, you can't unconditionally use VFP instructions because the
> target CPU may not support them and the assembler will (rightfully)
> error out if you try.
>
> /Mikael

I think I missed this earlier because my toolchain happened to be 
configured with --with-fpu=vfp.

Here's a patch that moves the VFP related functions together, and adds a 
'.fpu vfp' directive in front of them. The VFP hard-float support is 
always compiled in for more runtime flexibility, however VFP insns will 
only be executed if you use that hard-float functionality (which will be 
assumed you know you have a VFP unit)

Also added two missing conditional UNWINDs in the closure routines.

Anthony, is this patch okay?

Thanks,
Chung-Lin

Chung-Lin Tang writes:
 > On 2010/11/20 20:40, Mikael Pettersson wrote:
 > > On Thu, 28 Oct 2010 14:11:12 -0400, Anthony Green wrote:
 > >   >
 > >   >  I'm committing this patch from Chung-Lin Tang, who provided the
 > >   >  following description:
 > >   >
 > >   >  "this patch implements VFP hard-float calling conventions, as a new ABI
 > >   >  for ARM. Asides from VFP calling convention bits, some additional
 > >   >  optimizations have been added to the current softfp assembly code
 > >   >  paths too.
 > > ...
 > >   >  2010-10-28  Chung-Lin Tang<cltang@codesourcery.com>
 > > ...
 > >
 > > This commit, r166032, caused a build error on ARM:
 > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46508
 > >
 > > In short, you can't unconditionally use VFP instructions because the
 > > target CPU may not support them and the assembler will (rightfully)
 > > error out if you try.
 > >
 > > /Mikael
 > 
 > I think I missed this earlier because my toolchain happened to be 
 > configured with --with-fpu=vfp.
 > 
 > Here's a patch that moves the VFP related functions together, and adds a 
 > '.fpu vfp' directive in front of them. The VFP hard-float support is 
 > always compiled in for more runtime flexibility, however VFP insns will 
 > only be executed if you use that hard-float functionality (which will be 
 > assumed you know you have a VFP unit)
 > 
 > Also added two missing conditional UNWINDs in the closure routines.
 > 
 > Anthony, is this patch okay?
 > 
 > Thanks,
 > Chung-Lin

This patch works for me.  Tested on armv5tel-linux-gnueabi w/ java
and thus libffi: bootstrap succeeds and there were no test suite
regressions compared to a previous run where I had reverted r166032.

Thanks,

/Mikael

 > diff --git a/ChangeLog b/ChangeLog
 > index b4a0a93..00aebba 100644
 > --- a/ChangeLog
 > +++ b/ChangeLog
 > @@ -1,3 +1,10 @@
 > +2010-11-30  Chung-Lin Tang  <cltang@codesourcery.com>
 > +
 > +	* src/arm/sysv.S (ffi_closure_SYSV): Add UNWIND to .pad directive.
 > +	(ffi_closure_VFP): Same.
 > +	(ffi_call_VFP): Move down to before ffi_closure_VFP. Add '.fpu vfp'
 > +	directive.
 > +
 >  2010-11-22  Jacek Caban <jacek@codeweavers.com>
 >  
 >  	* configure.ac: Check for symbol underscores on mingw-w64.
 > diff --git a/src/arm/sysv.S b/src/arm/sysv.S
 > index 7bce727..72f0ee0 100644
 > --- a/src/arm/sysv.S
 > +++ b/src/arm/sysv.S
 > @@ -229,6 +229,83 @@ LSYM(Lepilogue):
 >          .size    CNAME(ffi_call_SYSV),.ffi_call_SYSV_end-CNAME(ffi_call_SYSV)
 >  
 >  
 > +/*
 > +	unsigned int FFI_HIDDEN
 > +	ffi_closure_SYSV_inner (closure, respp, args)
 > +	     ffi_closure *closure;
 > +	     void **respp;
 > +  	     void *args;
 > +*/
 > +
 > +ARM_FUNC_START ffi_closure_SYSV
 > +	UNWIND .pad #16
 > +	add	ip, sp, #16
 > +	stmfd	sp!, {ip, lr}
 > +	UNWIND .save	{r0, lr}
 > +	add	r2, sp, #8
 > +	UNWIND .pad #16
 > +	sub	sp, sp, #16
 > +	str	sp, [sp, #8]
 > +	add	r1, sp, #8
 > +	bl	ffi_closure_SYSV_inner
 > +	cmp	r0, #FFI_TYPE_INT
 > +	beq	.Lretint
 > +
 > +	cmp	r0, #FFI_TYPE_FLOAT
 > +#if defined(__SOFTFP__) || defined(__ARM_EABI__)
 > +	beq	.Lretint
 > +#else
 > +	beq	.Lretfloat
 > +#endif
 > +
 > +	cmp	r0, #FFI_TYPE_DOUBLE
 > +#if defined(__SOFTFP__) || defined(__ARM_EABI__)
 > +	beq	.Lretlonglong
 > +#else
 > +	beq	.Lretdouble
 > +#endif
 > +
 > +	cmp	r0, #FFI_TYPE_LONGDOUBLE
 > +#if defined(__SOFTFP__) || defined(__ARM_EABI__)
 > +	beq	.Lretlonglong
 > +#else
 > +	beq	.Lretlongdouble
 > +#endif
 > +
 > +	cmp	r0, #FFI_TYPE_SINT64
 > +	beq	.Lretlonglong
 > +.Lclosure_epilogue:
 > +	add	sp, sp, #16
 > +	ldmfd	sp, {sp, pc}
 > +.Lretint:
 > +	ldr	r0, [sp]
 > +	b	.Lclosure_epilogue
 > +.Lretlonglong:
 > +	ldr	r0, [sp]
 > +	ldr	r1, [sp, #4]
 > +	b	.Lclosure_epilogue
 > +
 > +#if !defined(__SOFTFP__) && !defined(__ARM_EABI__)
 > +.Lretfloat:
 > +	ldfs	f0, [sp]
 > +	b	.Lclosure_epilogue
 > +.Lretdouble:
 > +	ldfd	f0, [sp]
 > +	b	.Lclosure_epilogue
 > +.Lretlongdouble:
 > +	ldfd	f0, [sp]
 > +	b	.Lclosure_epilogue
 > +#endif
 > +
 > +.ffi_closure_SYSV_end:
 > +	UNWIND .fnend
 > +        .size    CNAME(ffi_closure_SYSV),.ffi_closure_SYSV_end-CNAME(ffi_closure_SYSV)
 > +
 > +
 > +/* Below are VFP hard-float ABI call and closure implementations.
 > +   Add VFP FPU directive here. */
 > +	.fpu	vfp
 > +
 >  	@ r0:   fn
 >  	@ r1:   &ecif
 >  	@ r2:   cif->bytes
 > @@ -321,79 +398,6 @@ LSYM(Lepilogue_vfp):
 >  .ffi_call_VFP_end:
 >  	UNWIND .fnend
 >          .size    CNAME(ffi_call_VFP),.ffi_call_VFP_end-CNAME(ffi_call_VFP)
 > -	
 > -	
 > -/*
 > -	unsigned int FFI_HIDDEN
 > -	ffi_closure_SYSV_inner (closure, respp, args)
 > -	     ffi_closure *closure;
 > -	     void **respp;
 > -  	     void *args;
 > -*/
 > -
 > -ARM_FUNC_START ffi_closure_SYSV
 > -	UNWIND .pad #16
 > -	add	ip, sp, #16
 > -	stmfd	sp!, {ip, lr}
 > -	UNWIND .save	{r0, lr}
 > -	add	r2, sp, #8
 > -	.pad #16
 > -	sub	sp, sp, #16
 > -	str	sp, [sp, #8]
 > -	add	r1, sp, #8
 > -	bl	ffi_closure_SYSV_inner
 > -	cmp	r0, #FFI_TYPE_INT
 > -	beq	.Lretint
 > -
 > -	cmp	r0, #FFI_TYPE_FLOAT
 > -#if defined(__SOFTFP__) || defined(__ARM_EABI__)
 > -	beq	.Lretint
 > -#else
 > -	beq	.Lretfloat
 > -#endif
 > -
 > -	cmp	r0, #FFI_TYPE_DOUBLE
 > -#if defined(__SOFTFP__) || defined(__ARM_EABI__)
 > -	beq	.Lretlonglong
 > -#else
 > -	beq	.Lretdouble
 > -#endif
 > -
 > -	cmp	r0, #FFI_TYPE_LONGDOUBLE
 > -#if defined(__SOFTFP__) || defined(__ARM_EABI__)
 > -	beq	.Lretlonglong
 > -#else
 > -	beq	.Lretlongdouble
 > -#endif
 > -
 > -	cmp	r0, #FFI_TYPE_SINT64
 > -	beq	.Lretlonglong
 > -.Lclosure_epilogue:
 > -	add	sp, sp, #16
 > -	ldmfd	sp, {sp, pc}
 > -.Lretint:
 > -	ldr	r0, [sp]
 > -	b	.Lclosure_epilogue
 > -.Lretlonglong:
 > -	ldr	r0, [sp]
 > -	ldr	r1, [sp, #4]
 > -	b	.Lclosure_epilogue
 > -
 > -#if !defined(__SOFTFP__) && !defined(__ARM_EABI__)
 > -.Lretfloat:
 > -	ldfs	f0, [sp]
 > -	b	.Lclosure_epilogue
 > -.Lretdouble:
 > -	ldfd	f0, [sp]
 > -	b	.Lclosure_epilogue
 > -.Lretlongdouble:
 > -	ldfd	f0, [sp]
 > -	b	.Lclosure_epilogue
 > -#endif
 > -
 > -.ffi_closure_SYSV_end:
 > -	UNWIND .fnend
 > -        .size    CNAME(ffi_closure_SYSV),.ffi_closure_SYSV_end-CNAME(ffi_closure_SYSV)
 >  
 >  
 >  ARM_FUNC_START ffi_closure_VFP
 > @@ -405,7 +409,7 @@ ARM_FUNC_START ffi_closure_VFP
 >  	UNWIND .save	{r0, lr}
 >  	add	r2, sp, #72
 >  	add	r3, sp, #8
 > -	.pad #72
 > +	UNWIND .pad #72
 >  	sub	sp, sp, #72
 >  	str	sp, [sp, #64]
 >  	add	r1, sp, #64

Mikael Pettersson <mikpe@it.uu.se> writes:

> This patch works for me.  Tested on armv5tel-linux-gnueabi w/ java
> and thus libffi: bootstrap succeeds and there were no test suite
> regressions compared to a previous run where I had reverted r166032.

Thanks for testing Mikael. 

Chung-Lin, please go ahead and commit this patch to GCC.

Thanks,

AG