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
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193
diff --git a/ChangeLog b/ChangeLog
index 2a4c8dc..cf6da6f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
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
according to __STRUCT_PARM_ALIGN__.
(ffi_prep_cif_machdep_core): Likewise.
diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
index 12501b6..cd63e26 100644
--- 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 c3349df..3eefe7e 100644
--- 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 5ee3a19..675ed03 100644
--- 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)