[SPARC v8] Make ffi compatible with Solaris Studio aggregate return ABI

[SPARC v8] Make ffi compatible with Solaris Studio aggregate return ABI

[Ginn Chen]

SPARC V8 Manual has:
"When a procedure expecting an aggregate return value from a called function is
compiled, an UNIMP instruction is placed after the delay-slot instruction
following the CALL to the function in question. The immediate field in this
UNIMP instruction contains the low-order twelve bits of the size (in bytes) of
the area allocated by the caller for the aggregate value expected to be
returned."

The function compiled by gcc doesn't check the UNIMP instruction, it simply copy the returning struct and jump to %i7+12.
The same function compiled by Solaris Studio, checks the struct size in UNIMP instruction, if it matches the returning struct it will copy the value and jump to %i7+12,
if not it will do nothing and jump to %i7+8.

So simply adding a nop in v8.S doesn't work with libraries compiled by Solaris Studio.
e.g. if I compile testsuite/cls_8byte.c with Solaris Studio, it will not pass the test.

I have a patch to alloc some executable space and make the call with exact struct size there.
The patch is at:
https://bugzilla.mozilla.org/show_bug.cgi?id=583206

I have tested it on Solaris with both gcc and Solaris Studio.

Please review it.

Thanks,

Ginn  

Re: [SPARC v8] Make ffi compatible with Solaris Studio aggregate return ABI

[Anthony Green]

Ginn Chen <ginn.chen@oracle.com> writes:

> SPARC V8 Manual has:
> "When a procedure expecting an aggregate return value from a called function is
> compiled, an UNIMP instruction is placed after the delay-slot instruction
> following the CALL to the function in question. The immediate field in this
> UNIMP instruction contains the low-order twelve bits of the size (in bytes) of
> the area allocated by the caller for the aggregate value expected to be
> returned."
>
> The function compiled by gcc doesn't check the UNIMP instruction, it simply copy the returning struct and jump to %i7+12.
> The same function compiled by Solaris Studio, checks the struct size in UNIMP instruction, if it matches the returning struct it will copy the value and jump to %i7+12,
> if not it will do nothing and jump to %i7+8.
>
> So simply adding a nop in v8.S doesn't work with libraries compiled by Solaris Studio.
> e.g. if I compile testsuite/cls_8byte.c with Solaris Studio, it will not pass the test.
>
> I have a patch to alloc some executable space and make the call with exact struct size there.
> The patch is at:
> https://bugzilla.mozilla.org/show_bug.cgi?id=583206
>
> I have tested it on Solaris with both gcc and Solaris Studio.
>
> Please review it.

Thanks Ginn.  This looks good to me.  I've committed it to libffi.

AG


>
> Thanks,
>
> Ginn  

Re: [SPARC v8] Make ffi compatible with Solaris Studio aggregate return ABI

[Ginn Chen]

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

Hi Anthony,

I just found I missed to use "flush" to sync the instruction cache on v8.
It may cause problems in rare cases.
Attached is a follow-up patch for this issue.

Thanks,

Ginn


[-- Attachment #2: sparc_v8_flush.diff --]
[-- Type: application/octet-stream, Size: 1465 bytes --]

--- ffi.c	Wed Apr  6 01:53:00 2011
+++ /export/home/sceri/work/libffi-3.0.9/src/sparc/ffi.c	Wed Apr  6 09:50:48 2011
@@ -432,6 +432,10 @@
 		call_struct[5] = 0x01000000;	     	 /* nop			 */
 	      call_struct[6] = 0x81c7e008;		 /* ret			 */
 	      call_struct[7] = 0xbe100017;		 /* mov   %l7, %i7	 */
+	      asm volatile ("iflush %0; iflush %0+8; iflush %0+16; iflush %0+24" : : 
+			    "r" (call_struct) : "memory");
+	      /* SPARC v8 requires 5 instructions for flush to be visible */
+	      asm volatile ("nop; nop; nop; nop; nop");
 	      ffi_call_v8(ffi_prep_args_v8, &ecif, cif->bytes,
 			  cif->flags, rvalue, call_struct);
 	      ffi_closure_free(call_struct);
@@ -509,13 +513,13 @@
   closure->fun = fun;
   closure->user_data = user_data;
 
-  /* Flush the Icache.  FIXME: alignment isn't certain, assume 8 bytes */
+  /* Flush the Icache.  closure is 8 bytes aligned */
 #ifdef SPARC64
-  asm volatile ("flush	%0" : : "r" (closure) : "memory");
-  asm volatile ("flush	%0" : : "r" (((char *) closure) + 8) : "memory");
+  asm volatile ("flush	%0; flush %0+8" : : "r" (closure) : "memory");
 #else
-  asm volatile ("iflush	%0" : : "r" (closure) : "memory");
-  asm volatile ("iflush	%0" : : "r" (((char *) closure) + 8) : "memory");
+  asm volatile ("iflush %0; iflush %0+8" : : "r" (closure) : "memory");
+  /* SPARC v8 requires 5 instructions for flush to be visible */
+  asm volatile ("nop; nop; nop; nop; nop");
 #endif
 
   return FFI_OK;

[-- Attachment #3: Type: text/plain, Size: 1494 bytes --]




On Feb 8, 2011, at 11:45 PM, Anthony Green wrote:

> Ginn Chen <ginn.chen@oracle.com> writes:
> 
>> SPARC V8 Manual has:
>> "When a procedure expecting an aggregate return value from a called function is
>> compiled, an UNIMP instruction is placed after the delay-slot instruction
>> following the CALL to the function in question. The immediate field in this
>> UNIMP instruction contains the low-order twelve bits of the size (in bytes) of
>> the area allocated by the caller for the aggregate value expected to be
>> returned."
>> 
>> The function compiled by gcc doesn't check the UNIMP instruction, it simply copy the returning struct and jump to %i7+12.
>> The same function compiled by Solaris Studio, checks the struct size in UNIMP instruction, if it matches the returning struct it will copy the value and jump to %i7+12,
>> if not it will do nothing and jump to %i7+8.
>> 
>> So simply adding a nop in v8.S doesn't work with libraries compiled by Solaris Studio.
>> e.g. if I compile testsuite/cls_8byte.c with Solaris Studio, it will not pass the test.
>> 
>> I have a patch to alloc some executable space and make the call with exact struct size there.
>> The patch is at:
>> https://bugzilla.mozilla.org/show_bug.cgi?id=583206
>> 
>> I have tested it on Solaris with both gcc and Solaris Studio.
>> 
>> Please review it.
> 
> Thanks Ginn.  This looks good to me.  I've committed it to libffi.
> 
> AG
> 
> 
>> 
>> Thanks,
>> 
>> Ginn