public inbox for libffi-discuss@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: Anthony Green <green@moxielogic.com>
Cc: Matthias Klose <doko@ubuntu.com>,
	libffi-discuss@sourceware.org,	Bill Schmidt <wschmidt@us.ibm.com>
Subject: Re: libffi 3.3 release candidate 0
Date: Wed, 02 May 2018 09:18:00 -0000	[thread overview]
Message-ID: <20180502091834.GC28782@bubble.grove.modra.org> (raw)
In-Reply-To: <CACxje59JcWsHpodMgUAnmL+QE_vO=KN0jzw=U0QR1Ro9g-qZCw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 770 bytes --]

On Sat, Apr 07, 2018 at 07:24:55PM -0400, Anthony Green wrote:
> Thanks, Matthias.  The Debian auto-builder is really useful.
> 
> Most of those problems have already been cleaned up.  The two major problem
> areas left are powerpc64-linux-gnu, and 32-bit x86, which appears to be
> really broken (we test many more ABI variants on that platform).

The attached patch fixes the powerpc64-linux fails, with the exception
of the new libffi.bhaible tests returning an int (which do not comply
with man/ffi_call.3 requirement that rvalue must point to storage that
is sizeof(ffi_arg) or larger for non-floating point types).  I know
Matthias has a testsuite fix..

Regression tested both powerpc64-linux and powerpc64le-linux.

-- 
Alan Modra
Australia Development Lab, IBM

[-- Attachment #2: 0001-PowerPC64-ELFv1-fp-arg-fixes.patch --]
[-- Type: text/x-diff, Size: 6749 bytes --]

From cf62e6a15942978a0c0ec70fc460603227c6ff52 Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Wed, 2 May 2018 13:55:29 +0930
Subject: [PATCH] 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/src/powerpc/ffi_linux64.c b/src/powerpc/ffi_linux64.c
index b481c60..93a31f9 100644
--- a/src/powerpc/ffi_linux64.c
+++ b/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 = FFI_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 = FFI_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 *) FFI_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;
 

  reply	other threads:[~2018-05-02  9:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-02 12:54 Anthony Green
2018-04-04 21:05 ` Matthias Klose
2018-04-07 23:25   ` Anthony Green
2018-05-02  9:18     ` Alan Modra [this message]
2018-10-17 13:50   ` Matthias Klose

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180502091834.GC28782@bubble.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=doko@ubuntu.com \
    --cc=green@moxielogic.com \
    --cc=libffi-discuss@sourceware.org \
    --cc=wschmidt@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).