Andreas' 2013-02-08 change reverted some breakage for struct return

values from 2011-11-12, but in so doing reintroduced string
instructions to sysv.S that are not supported on all powerpc variants.
This patch properly copies the bounce buffer to destination in C code
rather than in asm.

I have tested this on powerpc64-linux, powerpc-linux and
powerpc-freebsd.  Well, the last on powerpc-linux by lying to
configure with

CC="gcc -m32 -msvr4-struct-return -mlong-double-64" \
CXX="g++ -m32 -msvr4-struct-return -mlong-double-64" \
/src/libffi-current/configure --build=powerpc-freebsd

and then

make && make CC="gcc -m32" CXX="g++ -m32" \
RUNTESTFLAGS=--target_board=unix/-m32/-msvr4-struct-return/-mlong-double-64\
 check

diff --git a/ChangeLog b/ChangeLog
index 2a4c8dc8ad02d7585a788ebe491f7dd26592de1a..cf6da6fc4a717cb22d84fe375bf091e18f7de747 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2013-11-16  Alan Modra  <amodra@gmail.com>
+
+       * src/powerpc/ffi.c (ffi_prep_cif_machdep): Revert 2013-02-08
+       change.  Do not consume an int arg when returning a small struct
+       for FFI_SYSV ABI.
+       (ffi_call): Only use bounce buffer when FLAG_RETURNS_SMST.
+       Properly copy bounce buffer to destination.
+       * src/powerpc/sysv.S: Revert 2013-02-08 change.
+       * src/powerpc/ppc_closure.S: Remove stray '+'.
+
 2013-11-16  Alan Modra  <amodra@gmail.com>
 
        * src/powerpc/ffi.c (ffi_prep_args64): Align struct parameters

diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
index 12501b6de44bdbc99562aa9d225d8b774bb57019..cd63e268703d24604b8a514fa76f2b8cd5e002b5 100644 (file)
--- a/src/powerpc/ffi.c
+++ b/src/powerpc/ffi.c
@@ -48,11 +48,6 @@ enum {
 
   FLAG_RETURNS_128BITS  = 1 << (31-27), /* cr6  */
 
-  FLAG_SYSV_SMST_R4     = 1 << (31-26), /* use r4 for FFI_SYSV 8 byte
-                                          structs.  */
-  FLAG_SYSV_SMST_R3     = 1 << (31-25), /* use r3 for FFI_SYSV 4 byte
-                                          structs.  */
-
   FLAG_ARG_NEEDS_COPY   = 1 << (31- 7),
 #ifndef __NO_FPRS__
   FLAG_FP_ARGUMENTS     = 1 << (31- 6), /* cr1.eq; specified by ABI */
@@ -720,35 +715,21 @@ ffi_prep_cif_machdep_core (ffi_cif *cif)
       break;
 
     case FFI_TYPE_STRUCT:
-      if (cif->abi == FFI_SYSV)
+      /*
+       * The final SYSV ABI says that structures smaller or equal 8 bytes
+       * are returned in r3/r4.  The FFI_GCC_SYSV ABI instead returns them
+       * in memory.
+       *
+       * NOTE: The assembly code can safely assume that it just needs to
+       *       store both r3 and r4 into a 8-byte word-aligned buffer, as
+       *       we allocate a temporary buffer in ffi_call() if this flag is
+       *       set.
+       */
+      if (cif->abi == FFI_SYSV && size <= 8)
        {
-         /* The final SYSV ABI says that structures smaller or equal 8 bytes
-            are returned in r3/r4. The FFI_GCC_SYSV ABI instead returns them
-            in memory.  */
-
-         /* Treat structs with size <= 8 bytes.  */
-         if (size <= 8)
-           {
-             flags |= FLAG_RETURNS_SMST;
-             /* These structs are returned in r3. We pack the type and the
-                precalculated shift value (needed in the sysv.S) into flags.
-                The same applies for the structs returned in r3/r4.  */
-             if (size <= 4)
-               {
-                 flags |= FLAG_SYSV_SMST_R3;
-                 flags |= 8 * (4 - size) << 8;
-                 break;
-               }
-             /* These structs are returned in r3 and r4. See above.   */
-             if  (size <= 8)
-               {
-                 flags |= FLAG_SYSV_SMST_R3 | FLAG_SYSV_SMST_R4;
-                 flags |= 8 * (8 - size) << 8;
-                 break;
-               }
-           }
+         flags |= FLAG_RETURNS_SMST;
+         break;
        }
-
       intarg_count++;
       flags |= FLAG_RETVAL_REFERENCE;
       /* Fall through.  */
@@ -990,30 +971,25 @@ ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
 {
   /*
    * The final SYSV ABI says that structures smaller or equal 8 bytes
-   * are returned in r3/r4. The FFI_GCC_SYSV ABI instead returns them
+   * are returned in r3/r4.  The FFI_GCC_SYSV ABI instead returns them
    * in memory.
    *
-   * Just to keep things simple for the assembly code, we will always
-   * bounce-buffer struct return values less than or equal to 8 bytes.
-   * This allows the ASM to handle SYSV small structures by directly
-   * writing r3 and r4 to memory without worrying about struct size.
+   * We bounce-buffer SYSV small struct return values so that sysv.S
+   * can write r3 and r4 to memory without worrying about struct size.
    */
   unsigned int smst_buffer[2];
   extended_cif ecif;
-  unsigned int rsize = 0;
 
   ecif.cif = cif;
   ecif.avalue = avalue;
 
-  /* Ensure that we have a valid struct return value */
   ecif.rvalue = rvalue;
-  if (cif->rtype->type == FFI_TYPE_STRUCT) {
-    rsize = cif->rtype->size;
-    if (rsize <= 8)
-      ecif.rvalue = smst_buffer;
-    else if (!rvalue)
-      ecif.rvalue = alloca(rsize);
-  }
+  if ((cif->flags & FLAG_RETURNS_SMST) != 0)
+    ecif.rvalue = smst_buffer;
+  /* Ensure that we have a valid struct return value.
+     FIXME: Isn't this just papering over a user problem?  */
+  else if (!rvalue && cif->rtype->type == FFI_TYPE_STRUCT)
+    ecif.rvalue = alloca (cif->rtype->size);
 
   switch (cif->abi)
     {
@@ -1038,7 +1014,21 @@ ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
 
   /* Check for a bounce-buffered return value */
   if (rvalue && ecif.rvalue == smst_buffer)
-    memcpy(rvalue, smst_buffer, rsize);
+    {
+      unsigned int rsize = cif->rtype->size;
+#ifndef __LITTLE_ENDIAN__
+      /* The SYSV ABI returns a structure of up to 4 bytes in size
+        left-padded in r3.  */
+      if (rsize <= 4)
+       memcpy (rvalue, (char *) smst_buffer + 4 - rsize, rsize);
+      /* The SYSV ABI returns a structure of up to 8 bytes in size
+        left-padded in r3/r4.  */
+      else if (rsize <= 8)
+       memcpy (rvalue, (char *) smst_buffer + 8 - rsize, rsize);
+      else
+#endif
+       memcpy (rvalue, smst_buffer, rsize);
+    }
 }
 
 

diff --git a/src/powerpc/ppc_closure.S b/src/powerpc/ppc_closure.S
index c3349df23d631356da305b8a6cc4ac5c9f00abc1..3eefe7e73ea009ddad784a555ecf5df576f7cc7d 100644 (file)
--- a/src/powerpc/ppc_closure.S
+++ b/src/powerpc/ppc_closure.S
@@ -288,7 +288,7 @@ ENTRY(ffi_closure_SYSV)
 #ifdef __LITTLE_ENDIAN__
        mtlr %r0
        b .Lfinish
-+#else
+#else
        li %r5,16
        b .Lstruct567
 #endif

diff --git a/src/powerpc/sysv.S b/src/powerpc/sysv.S
index 5ee3a19fc189466d1fdd3f5f5e3e899069cceb41..675ed03eeb9667f8ff14aa659a00448f755d600e 100644 (file)
--- a/src/powerpc/sysv.S
+++ b/src/powerpc/sysv.S
@@ -142,19 +142,14 @@ L(float_return_value):
 #endif
 
 L(small_struct_return_value):
-       extrwi  %r6,%r31,2,19         /* number of bytes padding = shift/8 */
-       mtcrf   0x02,%r31             /* copy flags to cr[24:27] (cr6) */
-       extrwi  %r5,%r31,5,19         /* r5 <- number of bits of padding */
-       subfic  %r6,%r6,4             /* r6 <- number of useful bytes in r3 */
-       bf-     25,L(done_return_value) /* struct in r3 ? if not, done. */
-/* smst_one_register: */
-       slw     %r3,%r3,%r5           /* Left-justify value in r3 */
-       mtxer   %r6                   /* move byte count to XER ... */
-       stswx   %r3,0,%r30            /* ... and store that many bytes */
-       bf+     26,L(done_return_value)  /* struct in r3:r4 ? */
-       add     %r6,%r6,%r30          /* adjust pointer */
-       stswi   %r4,%r6,4             /* store last four bytes */
-       b       L(done_return_value)
+       /*
+        * The C code always allocates a properly-aligned 8-byte bounce
+        * buffer to make this assembly code very simple.  Just write out
+        * r3 and r4 to the buffer to allow the C code to handle the rest.
+        */
+       stw %r3, 0(%r30)
+       stw %r4, 4(%r30)
+       b L(done_return_value)
 
 .LFE1:
 END(ffi_call_SYSV)