diff mbox series

libffi PowerPC64 ELFv1 fp arg fixes

Message ID 20180504101020.GI28782@bubble.grove.modra.org
State New
Headers show
Series libffi PowerPC64 ELFv1 fp arg fixes | expand

Commit Message

Alan Modra May 4, 2018, 10:10 a.m. UTC
The attached patch has been accepted into upstream libffi.  It fixes
powerpc64-linux problems shown up by Bruno Haible's new libffi
testsuite tests.  Bootstrapped and regression tested powerpc64-linux
and powerpc64le-linux.  OK mainline and active branches?

Comments

Segher Boessenkool May 4, 2018, 11:02 a.m. UTC | #1
On Fri, May 04, 2018 at 07:40:20PM +0930, Alan Modra wrote:
> The attached patch has been accepted into upstream libffi.  It fixes
> powerpc64-linux problems shown up by Bruno Haible's new libffi
> testsuite tests.  Bootstrapped and regression tested powerpc64-linux
> and powerpc64le-linux.  OK mainline and active branches?

That looks fine, and since it is in upstream it counts as obvious I
think?

Is there a test for this, btw?


Segher


> >From a3b6c9db017d3f142031636a9dd6088c5478ca28 Mon Sep 17 00:00:00 2001
> From: Alan Modra <amodra@gmail.com>
> Date: Wed, 2 May 2018 19:10:53 +0930
> Subject: [PATCH] libffi PowerPC64 ELFv1 fp arg fixes
> 
> The ELFv1 ABI says: "Single precision floating point values are mapped
> to the second word in a single doubleword" and also "Floating point
> registers f1 through f13 are used consecutively to pass up to 13
> floating point values, one member aggregates passed by value
> containing a floating point value, and to pass complex floating point
> values".
> 
> libffi wasn't expecting float args in the second word, and wasn't
> passing one member aggregates in fp registers.  This patch fixes those
> problems, making use of the existing ELFv2 homogeneous aggregate
> support since a one element fp struct is a special case of an
> homogeneous aggregate.
> 
> I've also set a flag when returning pointers that might be used one
> day.  This is just a tidy since the ppc64 assembly support code
> currently doesn't test FLAG_RETURNS_64BITS for integer types..
> 
> 	* src/powerpc/ffi_linux64.c (discover_homogeneous_aggregate):
> 	Compile for ELFv1 too, handling single element aggregates.
> 	(ffi_prep_cif_linux64_core): Call discover_homogeneous_aggregate
> 	for ELFv1.  Set FLAG_RETURNS_64BITS for FFI_TYPE_POINTER return.
> 	(ffi_prep_args64): Call discover_homogeneous_aggregate for ELFv1,
> 	and handle single element structs containing float or double
> 	as if the element wasn't wrapped in a struct.  Store floats in
> 	second word of doubleword slot when big-endian.
> 	(ffi_closure_helper_LINUX64): Similarly.
Alan Modra May 4, 2018, 1:34 p.m. UTC | #2
On Fri, May 04, 2018 at 06:02:27AM -0500, Segher Boessenkool wrote:
> On Fri, May 04, 2018 at 07:40:20PM +0930, Alan Modra wrote:
> > The attached patch has been accepted into upstream libffi.  It fixes
> > powerpc64-linux problems shown up by Bruno Haible's new libffi
> > testsuite tests.  Bootstrapped and regression tested powerpc64-linux
> > and powerpc64le-linux.  OK mainline and active branches?
> 
> That looks fine, and since it is in upstream it counts as obvious I
> think?
> 
> Is there a test for this, btw?

https://github.com/libffi/libffi/tree/master/testsuite/libffi.bhaible
I'm guessing this could be imported but I'm not sure of the politics.

powerpc64 failed test-call.c DGTEST=12, DGTEST=13, DGTEST=55,
DGTEST=56, DGTEST=57 and DGTEST=68, and corresponding tests in
test-callback.c.
diff mbox series

Patch

From a3b6c9db017d3f142031636a9dd6088c5478ca28 Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Wed, 2 May 2018 19:10:53 +0930
Subject: [PATCH] libffi PowerPC64 ELFv1 fp arg fixes

The ELFv1 ABI says: "Single precision floating point values are mapped
to the second word in a single doubleword" and also "Floating point
registers f1 through f13 are used consecutively to pass up to 13
floating point values, one member aggregates passed by value
containing a floating point value, and to pass complex floating point
values".

libffi wasn't expecting float args in the second word, and wasn't
passing one member aggregates in fp registers.  This patch fixes those
problems, making use of the existing ELFv2 homogeneous aggregate
support since a one element fp struct is a special case of an
homogeneous aggregate.

I've also set a flag when returning pointers that might be used one
day.  This is just a tidy since the ppc64 assembly support code
currently doesn't test FLAG_RETURNS_64BITS for integer types..

	* src/powerpc/ffi_linux64.c (discover_homogeneous_aggregate):
	Compile for ELFv1 too, handling single element aggregates.
	(ffi_prep_cif_linux64_core): Call discover_homogeneous_aggregate
	for ELFv1.  Set FLAG_RETURNS_64BITS for FFI_TYPE_POINTER return.
	(ffi_prep_args64): Call discover_homogeneous_aggregate for ELFv1,
	and handle single element structs containing float or double
	as if the element wasn't wrapped in a struct.  Store floats in
	second word of doubleword slot when big-endian.
	(ffi_closure_helper_LINUX64): Similarly.

diff --git a/libffi/src/powerpc/ffi_linux64.c b/libffi/src/powerpc/ffi_linux64.c
index b84b91fb237..ef0361b24ee 100644
--- a/libffi/src/powerpc/ffi_linux64.c
+++ b/libffi/src/powerpc/ffi_linux64.c
@@ -62,7 +62,6 @@  ffi_prep_types_linux64 (ffi_abi abi)
 #endif
 
 
-#if _CALL_ELF == 2
 static unsigned int
 discover_homogeneous_aggregate (const ffi_type *t, unsigned int *elnum)
 {
@@ -86,8 +85,13 @@  discover_homogeneous_aggregate (const ffi_type *t, unsigned int *elnum)
 	      return 0;
 	    base_elt = el_elt;
 	    total_elnum += el_elnum;
+#if _CALL_ELF == 2
 	    if (total_elnum > 8)
 	      return 0;
+#else
+	    if (total_elnum > 1)
+	      return 0;
+#endif
 	    el++;
 	  }
 	*elnum = total_elnum;
@@ -98,7 +102,6 @@  discover_homogeneous_aggregate (const ffi_type *t, unsigned int *elnum)
       return 0;
     }
 }
-#endif
 
 
 /* Perform machine dependent cif processing */
@@ -109,9 +112,7 @@  ffi_prep_cif_linux64_core (ffi_cif *cif)
   unsigned bytes;
   unsigned i, fparg_count = 0, intarg_count = 0;
   unsigned flags = cif->flags;
-#if _CALL_ELF == 2
   unsigned int elt, elnum;
-#endif
 
 #if FFI_TYPE_LONGDOUBLE == FFI_TYPE_DOUBLE
   /* If compiled without long double support..  */
@@ -157,6 +158,7 @@  ffi_prep_cif_linux64_core (ffi_cif *cif)
       /* Fall through.  */
     case FFI_TYPE_UINT64:
     case FFI_TYPE_SINT64:
+    case FFI_TYPE_POINTER:
       flags |= FLAG_RETURNS_64BITS;
       break;
 
@@ -222,7 +224,6 @@  ffi_prep_cif_linux64_core (ffi_cif *cif)
 		intarg_count = ALIGN (intarg_count, align);
 	    }
 	  intarg_count += ((*ptr)->size + 7) / 8;
-#if _CALL_ELF == 2
 	  elt = discover_homogeneous_aggregate (*ptr, &elnum);
 	  if (elt)
 	    {
@@ -231,7 +232,6 @@  ffi_prep_cif_linux64_core (ffi_cif *cif)
 		flags |= FLAG_ARG_NEEDS_PSAVE;
 	    }
 	  else
-#endif
 	    {
 	      if (intarg_count > NUM_GPR_ARG_REGISTERS64)
 		flags |= FLAG_ARG_NEEDS_PSAVE;
@@ -449,9 +449,7 @@  ffi_prep_args64 (extended_cif *ecif, unsigned long *const stack)
        i < nargs;
        i++, ptr++, p_argv.v++)
     {
-#if _CALL_ELF == 2
       unsigned int elt, elnum;
-#endif
 
       switch ((*ptr)->type)
 	{
@@ -494,6 +492,7 @@  ffi_prep_args64 (extended_cif *ecif, unsigned long *const stack)
 	  /* Fall through.  */
 #endif
 	case FFI_TYPE_DOUBLE:
+	do_double:
 	  double_tmp = **p_argv.d;
 	  if (fparg_count < NUM_FPR_ARG_REGISTERS64 && i < nfixedargs)
 	    {
@@ -512,17 +511,30 @@  ffi_prep_args64 (extended_cif *ecif, unsigned long *const stack)
 	  break;
 
 	case FFI_TYPE_FLOAT:
+	do_float:
 	  double_tmp = **p_argv.f;
 	  if (fparg_count < NUM_FPR_ARG_REGISTERS64 && i < nfixedargs)
 	    {
 	      *fpr_base.d++ = double_tmp;
 #if _CALL_ELF != 2
 	      if ((flags & FLAG_COMPAT) != 0)
-		*next_arg.f = (float) double_tmp;
+		{
+# ifndef __LITTLE_ENDIAN__
+		  next_arg.f[1] = (float) double_tmp;
+# else
+		  next_arg.f[0] = (float) double_tmp;
+# endif
+		}
 #endif
 	    }
 	  else
-	    *next_arg.f = (float) double_tmp;
+	    {
+# ifndef __LITTLE_ENDIAN__
+	      next_arg.f[1] = (float) double_tmp;
+# else
+	      next_arg.f[0] = (float) double_tmp;
+# endif
+	    }
 	  if (++next_arg.ul == gpr_end.ul)
 	    next_arg.ul = rest.ul;
 	  fparg_count++;
@@ -538,10 +550,10 @@  ffi_prep_args64 (extended_cif *ecif, unsigned long *const stack)
 	      if (align > 1)
 		next_arg.p = ALIGN (next_arg.p, align);
 	    }
-#if _CALL_ELF == 2
 	  elt = discover_homogeneous_aggregate (*ptr, &elnum);
 	  if (elt)
 	    {
+#if _CALL_ELF == 2
 	      union {
 		void *v;
 		float *f;
@@ -583,9 +595,14 @@  ffi_prep_args64 (extended_cif *ecif, unsigned long *const stack)
 		    fparg_count++;
 		  }
 		while (--elnum != 0);
+#else
+	      if (elt == FFI_TYPE_FLOAT)
+		goto do_float;
+	      else
+		goto do_double;
+#endif
 	    }
 	  else
-#endif
 	    {
 	      words = ((*ptr)->size + 7) / 8;
 	      if (next_arg.ul >= gpr_base.ul && next_arg.ul + words > gpr_end.ul)
@@ -796,12 +813,10 @@  ffi_closure_helper_LINUX64 (ffi_cif *cif,
 	      if (align > 1)
 		pst = (unsigned long *) ALIGN ((size_t) pst, align);
 	    }
-	  elt = 0;
-#if _CALL_ELF == 2
 	  elt = discover_homogeneous_aggregate (arg_types[i], &elnum);
-#endif
 	  if (elt)
 	    {
+#if _CALL_ELF == 2
 	      union {
 		void *v;
 		unsigned long *ul;
@@ -853,6 +868,12 @@  ffi_closure_helper_LINUX64 (ffi_cif *cif,
 		    }
 		  while (--elnum != 0);
 		}
+#else
+	      if (elt == FFI_TYPE_FLOAT)
+		goto do_float;
+	      else
+		goto do_double;
+#endif
 	    }
 	  else
 	    {
@@ -894,6 +915,7 @@  ffi_closure_helper_LINUX64 (ffi_cif *cif,
 	  /* Fall through.  */
 #endif
 	case FFI_TYPE_DOUBLE:
+	do_double:
 	  /* On the outgoing stack all values are aligned to 8 */
 	  /* there are 13 64bit floating point registers */
 
@@ -908,6 +930,7 @@  ffi_closure_helper_LINUX64 (ffi_cif *cif,
 	  break;
 
 	case FFI_TYPE_FLOAT:
+	do_float:
 	  if (pfr < end_pfr && i < nfixedargs)
 	    {
 	      /* Float values are stored as doubles in the
@@ -917,7 +940,13 @@  ffi_closure_helper_LINUX64 (ffi_cif *cif,
 	      pfr++;
 	    }
 	  else
-	    avalue[i] = pst;
+	    {
+#ifndef __LITTLE_ENDIAN__
+	      avalue[i] = (char *) pst + 4;
+#else
+	      avalue[i] = pst;
+#endif
+	    }
 	  pst++;
 	  break;