[PATCH] libffi support for CRIS

[PATCH] libffi support for CRIS

[Simon Posnjak]

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

Hi all,

This patch adds support for CRIS platform to libffi.

The added initialize_aggregate_packed_struct function in 
libffi/src/prep_cif.c is needed because CRIS uses *packed* structs.


	Regards Simon



[-- Attachment #2: gcc_cris_libffi_submit.diff --]
[-- Type: text/x-patch, Size: 16988 bytes --]

diff -uNr gcc/libffi/Makefile.am gcc_cris/libffi/Makefile.am
--- gcc/libffi/Makefile.am	2004-10-13 19:20:21.000000000 +0200
+++ gcc_cris/libffi/Makefile.am	2004-10-25 12:58:24.101724568 +0200
@@ -7,6 +7,7 @@
 EXTRA_DIST = LICENSE ChangeLog.v1 \
 	src/alpha/ffi.c src/alpha/osf.S src/alpha/ffitarget.h \
 	src/arm/ffi.c src/arm/sysv.S src/arm/ffitarget.h \
+	src/cris/ffi.c src/cris/sysv.S src/cris/ffitarget.h \
 	src/mips/ffi.c src/mips/n32.S src/mips/o32.S \
 	src/mips/ffitarget.h \
 	src/m32r/ffi.c src/m32r/sysv.S src/m32r/ffitarget.h \
@@ -120,6 +121,9 @@
 if ARM
 nodist_libffi_la_SOURCES += src/arm/sysv.S src/arm/ffi.c
 endif
+if CRIS32
+nodist_libffi_la_SOURCES += src/cris/sysv.S src/cris/ffi.c
+endif
 if FRV
 nodist_libffi_la_SOURCES += src/frv/eabi.S src/frv/ffi.c
 endif
diff -uNr gcc/libffi/configure.ac gcc_cris/libffi/configure.ac
--- gcc/libffi/configure.ac	2004-10-13 19:20:23.000000000 +0200
+++ gcc_cris/libffi/configure.ac	2004-10-25 12:58:24.102724416 +0200
@@ -68,6 +68,7 @@
 rs6000-*-aix*) TARGET=POWERPC_AIX; TARGETDIR=powerpc;;
 arm*-*-linux-*) TARGET=ARM; TARGETDIR=arm;;
 arm*-*-netbsdelf* | arm*-*-knetbsd*-gnu) TARGET=ARM; TARGETDIR=arm;;
+cris*-*-linux-*) TARGET=CRIS32; TARGETDIR=cris;;
 s390-*-linux-*) TARGET=S390; TARGETDIR=s390;;
 s390x-*-linux-*) TARGET=S390; TARGETDIR=s390;;
 x86_64-*-linux* | x86_64-*-freebsd* | x86_64-*-kfreebsd*-gnu) TARGET=X86_64; TARGETDIR=x86;;
@@ -95,6 +96,7 @@
 AM_CONDITIONAL(POWERPC_AIX, test x$TARGET = xPOWERPC_AIX)
 AM_CONDITIONAL(POWERPC_DARWIN, test x$TARGET = xPOWERPC_DARWIN)
 AM_CONDITIONAL(ARM, test x$TARGET = xARM)
+AM_CONDITIONAL(CRIS32, test x$TARGET = xCRIS32)
 AM_CONDITIONAL(FRV, test x$TARGET = xFRV)
 AM_CONDITIONAL(S390, test x$TARGET = xS390)
 AM_CONDITIONAL(X86_64, test x$TARGET = xX86_64)
diff -uNr gcc/libffi/src/cris/ffi.c gcc_cris/libffi/src/cris/ffi.c
--- gcc/libffi/src/cris/ffi.c	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/ffi.c	2004-10-25 12:58:24.103724264 +0200
@@ -0,0 +1,199 @@
+/* -----------------------------------------------------------------------
+   ffi.c - Copyright (c) 1998 Cygnus Solutions
+           Copyright (c) 2004 Simon Posnjak
+
+   CRIS Foreign Function Interface
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL CYGNUS SOLUTIONS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+   ----------------------------------------------------------------------- */
+
+#include <ffi.h>
+#include <ffi_common.h>
+
+/* ffi_prep_args is called by the assembly routine once stack space
+   has been allocated for the function's arguments */
+
+/*@-exportheader@*/
+int ffi_prep_args (char *stack, extended_cif * ecif)
+/*@=exportheader@*/
+{
+  register unsigned int i;
+  register unsigned int struct_count = 0;
+  register void **p_argv;
+  register char *argp;
+  register ffi_type **p_arg;
+
+  argp = stack;
+
+//  if (ecif->cif->rtype->type == FFI_TYPE_STRUCT ) {
+//    *(void **) argp = ecif->rvalue;
+//    argp += 4;
+//  }
+
+  p_argv = ecif->avalue;
+
+  for (i = ecif->cif->nargs, p_arg = ecif->cif->arg_types;
+       (i != 0); i--, p_arg++)
+    {
+      size_t z;
+
+      /* Align if necessary */
+      if (((*p_arg)->alignment - 1) & (unsigned) argp)
+	{
+	  argp = (char *) ALIGN (argp, (*p_arg)->alignment);
+	}
+
+      switch ((*p_arg)->type)
+	{
+	case FFI_TYPE_STRUCT:
+	  {
+	    z = (*p_arg)->size;
+	    if (z <= 8)
+	      {
+		memcpy (argp, *p_argv, z);
+		z += (4 - (z % 4));
+	      }
+	    else
+	      {
+		unsigned int uiLocOnStack;
+		
+		uiLocOnStack  = 4 * ecif->cif->nargs + struct_count;
+		struct_count = struct_count + (*p_arg)->size;
+		*(unsigned int *) argp =
+		  (unsigned int) (UINT32 *) (stack + uiLocOnStack);
+		memcpy ((stack + uiLocOnStack), *p_argv, (*p_arg)->size);
+	      }
+	    break;
+	  }
+	default:
+	  z = (*p_arg)->size;
+	  if (z < sizeof (int))
+	    {
+	      z = sizeof (int);
+	      switch ((*p_arg)->type)
+		{
+		case FFI_TYPE_SINT8:
+		  *(signed int *) argp = (signed int) *(SINT8 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_UINT8:
+		  *(unsigned int *) argp =
+		    (unsigned int) *(UINT8 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_SINT16:
+		  *(signed int *) argp = (signed int) *(SINT16 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_UINT16:
+		  *(unsigned int *) argp =
+		    (unsigned int) *(UINT16 *) (*p_argv);
+		  break;
+
+		default:
+		  FFI_ASSERT (0);
+		}
+	    }
+	  else if (z == sizeof (int))
+	    {
+	      *(unsigned int *) argp = (unsigned int) *(UINT32 *) (*p_argv);
+	    }
+	  else
+	    {
+	      memcpy (argp, *p_argv, z);
+	    }
+	  break;
+	}
+      p_argv++;
+      argp += z;
+    }
+
+  return (struct_count);
+}
+
+/* Perform machine dependent cif processing */
+ffi_status ffi_prep_cif_machdep (ffi_cif * cif)
+{
+  switch (cif->rtype->type)
+    {
+    case FFI_TYPE_VOID:
+    case FFI_TYPE_STRUCT:
+    case FFI_TYPE_FLOAT:
+    case FFI_TYPE_DOUBLE:
+    case FFI_TYPE_SINT64:
+    case FFI_TYPE_UINT64:
+      cif->flags = (unsigned) cif->rtype->type;
+      break;
+
+    default:
+      cif->flags = FFI_TYPE_INT;
+      break;
+    }
+
+  return FFI_OK;
+}
+
+
+/*@-declundef@*/
+/*@-exportheader@*/
+extern void ffi_call_SYSV (void (*)(char *, extended_cif *),
+			   /*@out@ */ extended_cif *,
+			   unsigned, unsigned,
+			   /*@out@ */ unsigned *,
+			   void (*fn) ());
+/*@=declundef@*/
+/*@=exportheader@*/
+
+void ffi_call ( /*@dependent@ */ ffi_cif * cif,
+	  void (*fn) (),
+	  /*@out@ */ void *rvalue,
+	  /*@dependent@ */ void **avalue)
+{
+  extended_cif ecif;
+
+  ecif.cif = cif;
+  ecif.avalue = avalue;
+
+  /* If the return value is a struct and we don't have a return */
+  /* value address then we need to make one                     */
+
+  if ((rvalue == NULL) && (cif->rtype->type == FFI_TYPE_STRUCT))
+    {
+      /*@-sysunrecog@ */
+      ecif.rvalue = alloca (cif->rtype->size);
+      /*@=sysunrecog@ */
+    }
+  else
+    ecif.rvalue = rvalue;
+
+
+  switch (cif->abi)
+    {
+    case FFI_SYSV:
+      /*@-usedef@ */
+      ffi_call_SYSV (ffi_prep_args, &ecif, cif->bytes,
+		     cif->flags, ecif.rvalue, fn);
+      /*@=usedef@ */
+      break;
+    default:
+      FFI_ASSERT (0);
+      break;
+    }
+}
diff -uNr gcc/libffi/src/cris/ffitarget.h gcc_cris/libffi/src/cris/ffitarget.h
--- gcc/libffi/src/cris/ffitarget.h	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/ffitarget.h	2004-10-25 12:58:24.104724112 +0200
@@ -0,0 +1,47 @@
+/* -----------------------------------------------------------------*-C-*-
+   ffitarget.h - Copyright (c) 1996-2003  Red Hat, Inc.
+   Target configuration macros for CRIS.
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL CYGNUS SOLUTIONS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+
+   ----------------------------------------------------------------------- */
+
+#ifndef LIBFFI_TARGET_H
+#define LIBFFI_TARGET_H
+
+#ifndef LIBFFI_ASM
+typedef unsigned long          ffi_arg;
+typedef signed long            ffi_sarg;
+
+typedef enum ffi_abi {
+  FFI_FIRST_ABI = 0,
+  FFI_SYSV,
+  FFI_DEFAULT_ABI = FFI_SYSV,
+  FFI_LAST_ABI = FFI_DEFAULT_ABI + 1
+} ffi_abi;
+#endif
+
+/* ---- Definitions for closures ----------------------------------------- */
+
+#define FFI_CLOSURES 0
+#define FFI_NATIVE_RAW_API 0
+
+#endif
+
diff -uNr gcc/libffi/src/cris/sysv.S gcc_cris/libffi/src/cris/sysv.S
--- gcc/libffi/src/cris/sysv.S	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/sysv.S	2004-10-25 12:58:24.105723960 +0200
@@ -0,0 +1,163 @@
+/* -----------------------------------------------------------------------
+   sysv.S - Copyright (c) 2004 Simon Posnjak
+
+   CRIS Foreign Function Interface
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL CYGNUS SOLUTIONS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+   ----------------------------------------------------------------------- */
+
+#define LIBFFI_ASM
+#include <ffi.h>
+
+	.text
+
+	;; OK, when we get called we should have this (according to 
+	;; AXIS ETRAX 100LX Programmer's Manual chapter 6.3)
+	;; 
+	;; R10:	 ffi_prep_args (func. pointer)
+	;; R11:  &ecif
+	;; R12:  cif->bytes
+	;; R13:  fig->flags
+	;; sp+0: ecif.rvalue
+	;; sp+4: fn (function pointer to the function that we need to call)
+
+	.globl  ffi_call_SYSV
+	.type   ffi_call_SYSV,@function
+
+ffi_call_SYSV:
+	;; save the regs to the stack
+	Push $srp
+	push $r6
+	push $r7
+	push $r8
+	push $r13
+	move.d $sp,$r8
+	move.d $sp,$r6
+	
+	;; move address of ffi_prep_args to r13
+	move.d $r10, $r13
+
+	;; make room on the stack for the args of fn
+	sub.d  $r12, $sp
+	
+	;; void ffi_prep_args(char *stack, extended_cif *ecif)
+	;; so:
+	;; r10 <-- stack pointer
+	;; r11 <-- &ecif (already there)
+	move.d $sp,$r10
+	
+	;; load the ffi_prep_args function pointer into r13
+	;; call the function
+	jsr $r13
+	
+	;; save the size of the structures which are passed by stack
+	move.d $r10, $r7
+	
+	;; move first four args in to r10..r13
+	move.d [$sp+0], $r10
+	move.d [$sp+4], $r11
+	move.d [$sp+8], $r12
+	move.d [$sp+12], $r13
+	
+	;; adjust the stack
+	addq 16, $sp
+	sub.d $r7, $r6
+	cmp.d $sp, $r6
+	
+	bmi go_on_no_params_on_stack
+	nop
+	
+        beq go_on_no_params_on_stack
+        nop
+	
+	ba go_on
+	nop
+	
+go_on_no_params_on_stack:
+	move.d $r6, $sp
+	
+go_on:
+	;; discover if we need to put rval address in to $r9
+        move.d [$r8 + 0], $r7
+	cmpq FFI_TYPE_STRUCT, $r7
+	bne call_now
+	nop
+	
+	move.d [$r8 + 20], $r9
+
+call_now:
+	;; move address of the function in to r7
+	move.d [$r8 + 24], $r7
+	
+	;; call the function
+	jsr $r7
+	
+	;; reset stack
+	move.d $r8, $sp
+	
+	;; Load rval type in to r13
+	pop $r13
+	
+	;; detect rval type
+	cmpq FFI_TYPE_VOID, $r13
+	beq epilogue
+	nop
+
+	cmpq FFI_TYPE_STRUCT, $r13
+	beq epilogue
+	nop
+	
+	cmpq FFI_TYPE_DOUBLE, $r13
+	beq return_double_or_longlong
+	nop
+
+	cmpq FFI_TYPE_UINT64, $r13
+	beq return_double_or_longlong
+	nop
+
+	cmpq FFI_TYPE_SINT64, $r13
+	beq return_double_or_longlong
+	nop
+
+	;; just return the 32 bit value
+	ba return
+	nop
+		
+return_double_or_longlong:
+	move.d [$sp+16], $r13
+	move.d $r10, [$r13]
+	addq 4, $r13
+	move.d $r11, [$r13]
+	ba epilogue
+	nop	
+	
+return:
+	move.d [$sp+16], $r13
+	move.d $r10, [$r13]
+	ba epilogue
+	nop
+		
+epilogue:
+	pop $r8
+	pop $r7
+	pop $r6
+	Jump [$sp+]
+	
+        .size   ffi_call_SYSV,.-ffi_call_SYSV
diff -uNr gcc/libffi/src/prep_cif.c gcc_cris/libffi/src/prep_cif.c
--- gcc/libffi/src/prep_cif.c	2004-03-19 23:34:17.000000000 +0100
+++ gcc_cris/libffi/src/prep_cif.c	2004-10-25 12:58:24.106723808 +0200
@@ -33,6 +33,44 @@
 /* Perform machine independent initialization of aggregate type
    specifications. */
 
+ffi_status initialize_aggregate_packed_struct ( /*@out@ */ ffi_type * arg)
+{
+  ffi_type **ptr;
+
+  FFI_ASSERT (arg != NULL);
+
+  /*@-usedef@ */
+
+  FFI_ASSERT (arg->elements != NULL);
+  FFI_ASSERT (arg->size == 0);
+  FFI_ASSERT (arg->alignment == 0);
+
+  ptr = &(arg->elements[0]);
+
+  while ((*ptr) != NULL)    {
+      if (((*ptr)->size == 0)
+          && (initialize_aggregate_packed_struct ((*ptr)) != FFI_OK))
+        return FFI_BAD_TYPEDEF;
+
+      /* Perform a sanity check on the argument type */
+      FFI_ASSERT (ffi_type_test ((*ptr)));
+
+      arg->size += (*ptr)->size;
+
+      arg->alignment = (arg->alignment > (*ptr)->alignment) ?
+        arg->alignment : (*ptr)->alignment;
+
+      ptr++;
+    }
+
+  if (arg->size == 0)
+    return FFI_BAD_TYPEDEF;
+  else
+    return FFI_OK;
+
+  /*@=usedef@ */
+}
+
 static ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
 {
   ffi_type **ptr; 
@@ -113,7 +151,7 @@
   FFI_ASSERT_VALID_TYPE(cif->rtype);
 
   /* x86-64 and s390 stack space allocation is handled in prep_machdep.  */
-#if !defined M68K && !defined __x86_64__ && !defined S390 && !defined PA
+#if !defined M68K && !defined __x86_64__ && !defined S390 && !defined PA && !defined CRIS32
   /* Make space for the return structure pointer */
   if (cif->rtype->type == FFI_TYPE_STRUCT
 #ifdef SPARC
@@ -127,8 +165,20 @@
     {
 
       /* Initialize any uninitialized aggregate type definitions */
+#ifdef CRIS32
+      if ((*ptr)->type == FFI_TYPE_STRUCT)
+      {
+        if (((*ptr)->size == 0) && (initialize_aggregate_packed_struct((*ptr)) != FFI_OK))
+           return FFI_BAD_TYPEDEF;
+      }
+      else
+      {
+#endif
       if (((*ptr)->size == 0) && (initialize_aggregate((*ptr)) != FFI_OK))
 	return FFI_BAD_TYPEDEF;
+#ifdef CRIS32
+      }
+#endif
 
       /* Perform a sanity check on the argument type, do this 
 	 check after the initialization.  */
@@ -147,8 +197,29 @@
 	  /* Add any padding if necessary */
 	  if (((*ptr)->alignment - 1) & bytes)
 	    bytes = ALIGN(bytes, (*ptr)->alignment);
-	  
+#ifdef CRIS32
+          if ((*ptr)->type == FFI_TYPE_STRUCT)
+          {
+           if ((*ptr)->size > 8)
+           {
+               bytes += (*ptr)->size;
+               bytes += sizeof (void *);
+           }
+           else
+           {
+               if ((*ptr)->size > 4)
+                   bytes += 8;
+               else
+                   bytes += 4;
+           }
+         }
+         else
+         {
+           bytes += STACK_ARG_SIZE((*ptr)->size);
+         }
+#else
 	  bytes += STACK_ARG_SIZE((*ptr)->size);
+#endif
 	}
 #endif
     }
diff -uNr gcc/libffi/src/types.c gcc_cris/libffi/src/types.c
--- gcc/libffi/src/types.c	2004-10-13 19:20:24.000000000 +0200
+++ gcc_cris/libffi/src/types.c	2004-10-25 12:58:24.106723808 +0200
@@ -53,7 +53,7 @@
 
 #endif
 
-#if defined X86 || defined ARM || defined M68K
+#if defined X86 || defined ARM || defined M68K || defined CRIS32
 
 FFI_INTEGRAL_TYPEDEF(uint64, 8, 4, FFI_TYPE_UINT64);
 FFI_INTEGRAL_TYPEDEF(sint64, 8, 4, FFI_TYPE_SINT64);
@@ -80,7 +80,7 @@
 #endif
 FFI_INTEGRAL_TYPEDEF(longdouble, 12, 4, FFI_TYPE_LONGDOUBLE);
 
-#elif defined ARM || defined SH || defined POWERPC_AIX || defined M32R
+#elif defined ARM || defined SH || defined POWERPC_AIX || defined M32R || defined CRIS32
 
 FFI_INTEGRAL_TYPEDEF(double, 8, 4, FFI_TYPE_DOUBLE);
 FFI_INTEGRAL_TYPEDEF(longdouble, 8, 4, FFI_TYPE_LONGDOUBLE);

Re: [PATCH] libffi support for CRIS

[Hans-Peter Nilsson]

On Mon, 25 Oct 2004, Simon Posnjak wrote:

> This patch adds support for CRIS platform to libffi.

Many thanks for your work!

I hoped to hear something from Anthony Green in the past week
about status on copyright assignment of libffi to the FSF
generally, but he hasn't replied, so let's continue as-is, and
hope that we can persuade your employer to do the
copyright-assignment if that ever becomes an issue.  (For the
record, Simon has gone through the process for gdb in the past,
so I have faith it can happen. ;-)

AFAIK, port copyright assignment is not an issue for libffi
currently, and people seem to commit ports left and right with
personal copyright headers, so it can't be an issue that Simon
doesn't have any (GCC) copyright assignment papers on file,
right?

Over to the actual patch: Please use the "-p" diff option ("diff
-up", not just "diff -u").  Sending the patch as an attachment
(rather than in-body) made it harder to comment on each piece
(to the crowd: comments on my choice of mailer to /dev/null).

Same comment applies as to boehm-gc, CRIS32 is a bit confusing.
I'd rather just use the predefined __CRIS__ and __linux__ (eh,
make that __gnu_linux__ ;-) and not define anything in
configury.  Or maybe define CRIS_LINUX.

I don't claim to understand libffi details (after all, I hadn't
written a CRIS port for it ;-) so maybe I shouldn't comment on
the implementation.  But then, nobody else would.  My comments
are mostly about coding standard nits: they nits stick out like
sore thumbs when you review code, so I can't help commenting on
them.  (I know not all code in libffi follows the GCC coding
standard, but that is to be improved, not used as an excuse.)

> The added initialize_aggregate_packed_struct function in
> libffi/src/prep_cif.c is needed because CRIS uses *packed* structs.

I can't approve this as-is, as it touches target-independent
libffi bits (which I anyway think should be handled with less
#ifdefs).

Still, I could pick up the patch, help it past libffi
maintainers, fixing *all* the following nits and issues (but I
hope you fix most), myself except one: you yourself have to fix
the copyright notice for ffitarget.h as commented below.

(I know that file is within ten lines of effective code, but I
will *not* change the notice related to copyright holder in
someone elses code whatsoever.)


> +++ gcc_cris/libffi/configure.ac	2004-10-25 12:58:24.102724416 +0200
> @@ -68,6 +68,7 @@
>  rs6000-*-aix*) TARGET=POWERPC_AIX; TARGETDIR=powerpc;;
>  arm*-*-linux-*) TARGET=ARM; TARGETDIR=arm;;
>  arm*-*-netbsdelf* | arm*-*-knetbsd*-gnu) TARGET=ARM; TARGETDIR=arm;;
> +cris*-*-linux-*) TARGET=CRIS32; TARGETDIR=cris;;

Better use "cris-..." and even "cris-*-linux*".  The ARM port is
not to be followed; it has other suffixes, the CRIS port in FSF
just uses "cris".  (At present that is, "crisv32" coming, FWIW.)

+++ gcc_cris/libffi/src/cris/ffi.c	2004-10-25 12:58:24.103724264 +0200
> +/* -----------------------------------------------------------------------
> +   ffi.c - Copyright (c) 1998 Cygnus Solutions
> +           Copyright (c) 2004 Simon Posnjak

I doubt Cygnus Solutions should be credited here.

> +//  if (ecif->cif->rtype->type == FFI_TYPE_STRUCT ) {
> +//    *(void **) argp = ecif->rvalue;
> +//    argp += 4;
> +//  }

No commented-out code please.  (Why is it there at all for this new code?
Was arm/ffi.c copied here?)

> +      /* Align if necessary */
> +      if (((*p_arg)->alignment - 1) & (unsigned) argp)
> +	{
> +	  argp = (char *) ALIGN (argp, (*p_arg)->alignment);
> +	}

Please explain why alignment can be necessary here.  Is it part
of the libffi API?  Nothing needs more than byte-alignment in
the CRIS ABI, but perhaps libffi supports structs with members
with __attribute__ ((__align__ (N))) directives with N larger
than normal alignment?

(Also, coding standard nits: no braces for single statements.
Comments are full sentences, with "." and two spaces after the
".".  Those two nits applies to all this code; I won't repeat
those two.)

> +  /* If the return value is a struct and we don't have a return */
> +  /* value address then we need to make one                     */

Misformatted comment.  It's:
/* If the return value is a struct and we don't have a return
   value address then we need to make one.  */

> +  else
> +    ecif.rvalue = rvalue;
> +
> +

Only one empty line, please.

> +++ gcc_cris/libffi/src/cris/ffitarget.h	2004-10-25 12:58:24.104724112 +0200
> @@ -0,0 +1,47 @@
> +/* -----------------------------------------------------------------*-C-*-
> +   ffitarget.h - Copyright (c) 1996-2003  Red Hat, Inc.

Why is Red Hat mentioned here?  Copy-paste?
Please resend with *at least* this fixed.

> +typedef enum ffi_abi {

Formatting nit.

> +++ gcc_cris/libffi/src/cris/sysv.S	2004-10-25 12:58:24.105723960 +0200
> @@ -0,0 +1,163 @@

There's a mix of TAB and spaces in this file.  Also, inconsistent
formatting: I suggest you consistently drop the space after comma in "move
X,Y".  Best to format it like the gcc output.  (Consistent with that,
"move X,[Y+Z]", not "move X, [Y + Z]".  This is arguably a matter or taste
but please be consistent within the file.)

> +ffi_call_SYSV:
> +	;; save the regs to the stack
> +	Push $srp
> +	push $r6
> +	push $r7
> +	push $r8
> +	push $r13
> +	move.d $sp,$r8
> +	move.d $sp,$r6

I don't understand the choice of call-saved registers here.  Better use
$R0 and up, so they can be saved and restored with a single "MOVEM".
Comments on register use would also be helpful.

> +
> +	;; move address of ffi_prep_args to r13

Comments should consist of full sentences, ending in "." in assembly too.

> +	cmp.d $sp, $r6
> +
> +	bmi go_on_no_params_on_stack
> +	nop
> +
> +        beq go_on_no_params_on_stack
> +        nop
> +
> +	ba go_on
> +	nop
> +
> +go_on_no_params_on_stack:
> +	move.d $r6, $sp
> +
> +go_on:

Those branches look a bit strange.  The "beq" is redundant (copying over
same contents), so just "bpl go_on" would have done it.  Also, because I
prefer to use insns that relate to the compare, I'd use "bhs go_on"
(branch-higher-same); "bmi" (branch-minus) is less understandable IMHO.
(The stack-adjusting is a bit hard to follow as it is.)

> +	;; discover if we need to put rval address in to $r9
> +        move.d [$r8 + 0], $r7
> +	cmpq FFI_TYPE_STRUCT, $r7
> +	bne call_now
> +	nop
> +
> +	move.d [$r8 + 20], $r9
> +
> +call_now:
> +	;; move address of the function in to r7
> +	move.d [$r8 + 24], $r7
> +
> +	;; call the function
> +	jsr $r7
> +
> +	;; reset stack
> +	move.d $r8, $sp
> +
> +	;; Load rval type in to r13
> +	pop $r13

Where was this pushed?  Ah, this is fig->flags?  The reference
to "rval type" confused me; flags is a *set* of values/bits to
me; not a single enum-like value as is seen below.  At first I
thought it was a bug and you meant to refer to ecif.rvalue.

> +
> +	;; detect rval type
> +	cmpq FFI_TYPE_VOID, $r13
> +	beq epilogue
> +	nop
> +
> +	cmpq FFI_TYPE_STRUCT, $r13
> +	beq epilogue
> +	nop
> +
> +	cmpq FFI_TYPE_DOUBLE, $r13
> +	beq return_double_or_longlong
> +	nop
> +
> +	cmpq FFI_TYPE_UINT64, $r13
> +	beq return_double_or_longlong
> +	nop
> +
> +	cmpq FFI_TYPE_SINT64, $r13
> +	beq return_double_or_longlong
> +	nop

All those delay-slots could have been filled with the following
"CMPQ" with no loss in readbility (code-wise, just deleting the
NOP).  Ok, maybe that's just me...

Hm, if the area ecif.rvalue points to is *always* at least 64
bits long, there's no need for different epilogues.  You could
remove all the above rval type compares and branches and just do
this part:

> +return_double_or_longlong:
> +	move.d [$sp+16], $r13
> +	move.d $r10, [$r13]
> +	addq 4, $r13
> +	move.d $r11, [$r13]

> diff -uNr gcc/libffi/src/prep_cif.c gcc_cris/libffi/src/prep_cif.c

> +  while ((*ptr) != NULL)    {

Bad formatting: brace comes on next line.

> +      arg->alignment = (arg->alignment > (*ptr)->alignment) ?
> +        arg->alignment : (*ptr)->alignment;

Bad formatting.  The "?" should have been first on the continued line.
(I'm sure people will disagree on the coding standard interpretation
here. ;-)

>  static ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)

> @@ -127,8 +165,20 @@
>      {
>
> 	 /* Initialize any uninitialized aggregate type definitions */
> +#ifdef CRIS32
> +      if ((*ptr)->type == FFI_TYPE_STRUCT)
> +      {
> +        if (((*ptr)->size == 0) && (initialize_aggregate_packed_struct((*ptr)) != FFI_OK))
> +           return FFI_BAD_TYPEDEF;
> +      }
> +      else
> +      {
> +#endif
> 	 if (((*ptr)->size == 0) && (initialize_aggregate((*ptr)) != FFI_OK))
> 	  return FFI_BAD_TYPEDEF;
> +#ifdef CRIS32
> +      }
> +#endif

A bit too many ifdefs for my taste here.  I recognize there are
several in the code as-is, but continuing that would have things
worse. For this particular effect, wouldn't it just work with
#ifdef __CRIS__
#define initialize_aggregate initialize_aggregate_packed_struct
#endif
(perhaps in the initialize_aggregate body; after the header)?
Hmm, I don't quite understand the code here, which may be my
lack of libffi knowledge.  What does initialize_aggregate do
that your initialize_aggregate_packed_struct wouldn't do?

But wait, how about wrapping the whole initialize_aggregate body
after "cif->" lines up to including "cif->bytes = bytes;" in
"#ifndef __CRIS__ / #endif" and let ffi_prep_cif_machdep do the
structure handling?

brgds, H-P

Re: [PATCH] libffi support for CRIS

[Simon Posnjak]

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

V tor, 26.10.2004 ob 04:27 je Hans-Peter Nilsson napisal(a):
> Over to the actual patch: Please use the "-p" diff option ("diff
> -up", not just "diff -u").  Sending the patch as an attachment
> (rather than in-body) made it harder to comment on each piece
> (to the crowd: comments on my choice of mailer to /dev/null).
OK. I will do it like that.

> Same comment applies as to boehm-gc, CRIS32 is a bit confusing.
> I'd rather just use the predefined __CRIS__ and __linux__ (eh,
> make that __gnu_linux__ ;-) and not define anything in
> configury.  Or maybe define CRIS_LINUX.
I choose CRIS_LINUX.

> I can't approve this as-is, as it touches target-independent
> libffi bits (which I anyway think should be handled with less
> #ifdefs).
I moved the stuff to target-depended part of libffi.

> > +++ gcc_cris/libffi/configure.ac	2004-10-25 12:58:24.102724416 +0200
> > @@ -68,6 +68,7 @@
> >  rs6000-*-aix*) TARGET=POWERPC_AIX; TARGETDIR=powerpc;;
> >  arm*-*-linux-*) TARGET=ARM; TARGETDIR=arm;;
> >  arm*-*-netbsdelf* | arm*-*-knetbsd*-gnu) TARGET=ARM; TARGETDIR=arm;;
> > +cris*-*-linux-*) TARGET=CRIS32; TARGETDIR=cris;;
> 
> Better use "cris-..." and even "cris-*-linux*".  The ARM port is
> not to be followed; it has other suffixes, the CRIS port in FSF
> just uses "cris".  (At present that is, "crisv32" coming, FWIW.)
Fixed.

> +++ gcc_cris/libffi/src/cris/ffi.c	2004-10-25 12:58:24.103724264 +0200
> > +/* -----------------------------------------------------------------------
> > +   ffi.c - Copyright (c) 1998 Cygnus Solutions
> > +           Copyright (c) 2004 Simon Posnjak
> 
> I doubt Cygnus Solutions should be credited here.
Yes, they should be be, because the file came from arm/ffi.c and was
adopted for cris. (The copyright headers are correct on all files)

> > +//  if (ecif->cif->rtype->type == FFI_TYPE_STRUCT ) {
> > +//    *(void **) argp = ecif->rvalue;
> > +//    argp += 4;
> > +//  }
> 
> No commented-out code please.  (Why is it there at all for this new code?
> Was arm/ffi.c copied here?)
Oops, missed that one.

> > +      /* Align if necessary */
> > +      if (((*p_arg)->alignment - 1) & (unsigned) argp)
> > +	{
> > +	  argp = (char *) ALIGN (argp, (*p_arg)->alignment);
> > +	}
> 
> Please explain why alignment can be necessary here.  Is it part
> of the libffi API?  Nothing needs more than byte-alignment in
> the CRIS ABI, but perhaps libffi supports structs with members
> with __attribute__ ((__align__ (N))) directives with N larger
> than normal alignment?
I can not, because it is not needed, so i removed it.

> (Also, coding standard nits: no braces for single statements.
> Comments are full sentences, with "." and two spaces after the
> ".".  Those two nits applies to all this code; I won't repeat
> those two.)
I did some reformatting so it should be better now.

> > +++ gcc_cris/libffi/src/cris/ffitarget.h	2004-10-25 12:58:24.104724112 +0200
> > @@ -0,0 +1,47 @@
> > +/* -----------------------------------------------------------------*-C-*-
> > +   ffitarget.h - Copyright (c) 1996-2003  Red Hat, Inc.
> 
> Why is Red Hat mentioned here?  Copy-paste?
> Please resend with *at least* this fixed.
The file is exatcly the same as in arm/ffitarget.h. (s/ARM/CRIS/g)

> > +++ gcc_cris/libffi/src/cris/sysv.S	2004-10-25 12:58:24.105723960 +0200
> > @@ -0,0 +1,163 @@
> 
> There's a mix of TAB and spaces in this file.  Also, inconsistent
> formatting: I suggest you consistently drop the space after comma in "move
> X,Y".  Best to format it like the gcc output.  (Consistent with that,
> "move X,[Y+Z]", not "move X, [Y + Z]".  This is arguably a matter or taste
> but please be consistent within the file.)
OK. Done.

> > +ffi_call_SYSV:
> > +	;; save the regs to the stack
> > +	Push $srp
> > +	push $r6
> > +	push $r7
> > +	push $r8
> > +	push $r13
> > +	move.d $sp,$r8
> > +	move.d $sp,$r6
> 
> I don't understand the choice of call-saved registers here.  Better use
> $R0 and up, so they can be saved and restored with a single "MOVEM".
> Comments on register use would also be helpful.
Added some comments.

> > +	cmp.d $sp, $r6
> > +
> > +	bmi go_on_no_params_on_stack
> > +	nop
> > +
> > +        beq go_on_no_params_on_stack
> > +        nop
> > +
> > +	ba go_on
> > +	nop
> > +
> > +go_on_no_params_on_stack:
> > +	move.d $r6, $sp
> > +
> > +go_on:
> 
> Those branches look a bit strange.  The "beq" is redundant (copying over
> same contents), so just "bpl go_on" would have done it.  Also, because I
> prefer to use insns that relate to the compare, I'd use "bhs go_on"
> (branch-higher-same); "bmi" (branch-minus) is less understandable IMHO.
> (The stack-adjusting is a bit hard to follow as it is.)
You are correct. I cleaned it up.

> > +	cmpq FFI_TYPE_UINT64, $r13
> > +	beq return_double_or_longlong
> > +	nop
> > +
> > +	cmpq FFI_TYPE_SINT64, $r13
> > +	beq return_double_or_longlong
> > +	nop
> 
> All those delay-slots could have been filled with the following
> "CMPQ" with no loss in readbility (code-wise, just deleting the
> NOP).  Ok, maybe that's just me...
OK. Done.

> Hm, if the area ecif.rvalue points to is *always* at least 64
> bits long, there's no need for different epilogues.  You could
> remove all the above rval type compares and branches and just do
> this part:
It is not always 64 bits long. Only if return value is long long or double, all 
others are 32 bit. Or am I missing something?

> A bit too many ifdefs for my taste here.  I recognize there are
> several in the code as-is, but continuing that would have things
> worse. For this particular effect, wouldn't it just work with
> #ifdef __CRIS__
> #define initialize_aggregate initialize_aggregate_packed_struct
> #endif
> (perhaps in the initialize_aggregate body; after the header)?
> Hmm, I don't quite understand the code here, which may be my
> lack of libffi knowledge.  What does initialize_aggregate do
> that your initialize_aggregate_packed_struct wouldn't do?
> 
> But wait, how about wrapping the whole initialize_aggregate body
> after "cif->" lines up to including "cif->bytes = bytes;" in
> "#ifndef __CRIS__ / #endif" and let ffi_prep_cif_machdep do the
> structure handling?
I tried to clean it up the best I can.

		Regards Simon


[-- Attachment #2: gcc_cris_libffi_cleanup_submit.diff --]
[-- Type: text/x-patch, Size: 17869 bytes --]

diff -uNrp gcc/libffi/configure.ac gcc_cris/libffi/configure.ac
--- gcc/libffi/configure.ac     2004-10-13 19:20:23.000000000 +0200
+++ gcc_cris/libffi/configure.ac        2004-10-26 12:04:38.430195600 +0200
@@ -68,6 +68,7 @@ powerpc-*-aix*) TARGET=POWERPC_AIX; TARG
 rs6000-*-aix*) TARGET=POWERPC_AIX; TARGETDIR=powerpc;;
 arm*-*-linux-*) TARGET=ARM; TARGETDIR=arm;;
 arm*-*-netbsdelf* | arm*-*-knetbsd*-gnu) TARGET=ARM; TARGETDIR=arm;;
+cris-*-linux-*) TARGET=CRIS_LINUX; TARGETDIR=cris;;
 s390-*-linux-*) TARGET=S390; TARGETDIR=s390;;
 s390x-*-linux-*) TARGET=S390; TARGETDIR=s390;;
 x86_64-*-linux* | x86_64-*-freebsd* | x86_64-*-kfreebsd*-gnu) TARGET=X86_64; TARGETDIR=x86;;
@@ -95,6 +96,7 @@ AM_CONDITIONAL(POWERPC, test x$TARGET =
 AM_CONDITIONAL(POWERPC_AIX, test x$TARGET = xPOWERPC_AIX)
 AM_CONDITIONAL(POWERPC_DARWIN, test x$TARGET = xPOWERPC_DARWIN)
 AM_CONDITIONAL(ARM, test x$TARGET = xARM)
+AM_CONDITIONAL(CRIS_LINUX, test x$TARGET = xCRIS_LINUX)
 AM_CONDITIONAL(FRV, test x$TARGET = xFRV)
 AM_CONDITIONAL(S390, test x$TARGET = xS390)
 AM_CONDITIONAL(X86_64, test x$TARGET = xX86_64)
diff -uNrp gcc/libffi/Makefile.am gcc_cris/libffi/Makefile.am
--- gcc/libffi/Makefile.am	2004-10-13 19:20:21.000000000 +0200
+++ gcc_cris/libffi/Makefile.am	2004-10-26 12:04:38.000000000 +0200
@@ -7,6 +7,7 @@ SUBDIRS = include testsuite
 EXTRA_DIST = LICENSE ChangeLog.v1 \
 	src/alpha/ffi.c src/alpha/osf.S src/alpha/ffitarget.h \
 	src/arm/ffi.c src/arm/sysv.S src/arm/ffitarget.h \
+	src/cris/ffi.c src/cris/sysv.S src/cris/ffitarget.h \
 	src/mips/ffi.c src/mips/n32.S src/mips/o32.S \
 	src/mips/ffitarget.h \
 	src/m32r/ffi.c src/m32r/sysv.S src/m32r/ffitarget.h \
@@ -120,6 +121,9 @@ endif
 if ARM
 nodist_libffi_la_SOURCES += src/arm/sysv.S src/arm/ffi.c
 endif
+if CRIS_LINUX
+nodist_libffi_la_SOURCES += src/cris/sysv.S src/cris/ffi.c
+endif
 if FRV
 nodist_libffi_la_SOURCES += src/frv/eabi.S src/frv/ffi.c
 endif
diff -uNrp gcc/libffi/include/ffi_common.h gcc_cris/libffi/include/ffi_common.h
--- gcc/libffi/include/ffi_common.h	2004-08-30 17:43:02.000000000 +0200
+++ gcc_cris/libffi/include/ffi_common.h	2004-10-26 12:04:38.000000000 +0200
@@ -63,7 +63,14 @@ void ffi_type_test(/*@temp@*/ /*@out@*/ 
 #define ALIGN_DOWN(v, a) (((size_t) (v)) & -a)
 
 /* Perform machine dependent cif processing */
+#ifdef __cris__
+ffi_status ffi_prep_cif_machdep (ffi_cif * cif,
+                      ffi_abi abi,
+                      unsigned int nargs,
+                      ffi_type * rtype, ffi_type ** atypes);
+#else
 ffi_status ffi_prep_cif_machdep(ffi_cif *cif);
+#endif
 
 /* Extended cif, used in callback from assembly routine */
 typedef struct
diff -uNrp gcc/libffi/src/cris/ffi.c gcc_cris/libffi/src/cris/ffi.c
--- gcc/libffi/src/cris/ffi.c	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/ffi.c	2004-10-26 12:30:15.674498944 +0200
@@ -0,0 +1,270 @@
+/* -----------------------------------------------------------------------
+   ffi.c - Copyright (c) 1998 Cygnus Solutions
+           Copyright (c) 2004 Simon Posnjak
+
+   CRIS Foreign Function Interface
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL SIMON POSNJAK BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+   ----------------------------------------------------------------------- */
+
+#include <ffi.h>
+#include <ffi_common.h>
+
+#define STACK_ARG_SIZE(x) ALIGN(x, FFI_SIZEOF_ARG)
+
+ffi_status
+initialize_aggregate_packed_struct (ffi_type * arg)
+{
+  ffi_type **ptr;
+
+  FFI_ASSERT (arg != NULL);
+
+  FFI_ASSERT (arg->elements != NULL);
+  FFI_ASSERT (arg->size == 0);
+  FFI_ASSERT (arg->alignment == 0);
+
+  ptr = &(arg->elements[0]);
+
+  while ((*ptr) != NULL)
+    {
+      if (((*ptr)->size == 0)
+	  && (initialize_aggregate_packed_struct ((*ptr)) != FFI_OK))
+	return FFI_BAD_TYPEDEF;
+
+      FFI_ASSERT (ffi_type_test ((*ptr)));
+
+      arg->size += (*ptr)->size;
+
+      arg->alignment = (arg->alignment > (*ptr)->alignment) ?
+	arg->alignment : (*ptr)->alignment;
+
+      ptr++;
+    }
+
+  if (arg->size == 0)
+    return FFI_BAD_TYPEDEF;
+  else
+    return FFI_OK;
+}
+
+int
+ffi_prep_args (char *stack, extended_cif * ecif)
+{
+  register unsigned int i;
+  register unsigned int struct_count = 0;
+  register void **p_argv;
+  register char *argp;
+  register ffi_type **p_arg;
+
+  argp = stack;
+
+  p_argv = ecif->avalue;
+
+  for (i = ecif->cif->nargs, p_arg = ecif->cif->arg_types;
+       (i != 0); i--, p_arg++)
+    {
+      size_t z;
+
+      switch ((*p_arg)->type)
+	{
+	case FFI_TYPE_STRUCT:
+	  {
+	    z = (*p_arg)->size;
+	    if (z <= 8)
+	      {
+		memcpy (argp, *p_argv, z);
+		z += (4 - (z % 4));
+	      }
+	    else
+	      {
+		unsigned int uiLocOnStack;
+
+		uiLocOnStack = 4 * ecif->cif->nargs + struct_count;
+		struct_count = struct_count + (*p_arg)->size;
+		*(unsigned int *) argp =
+		  (unsigned int) (UINT32 *) (stack + uiLocOnStack);
+		memcpy ((stack + uiLocOnStack), *p_argv, (*p_arg)->size);
+	      }
+	    break;
+	  }
+	default:
+	  z = (*p_arg)->size;
+	  if (z < sizeof (int))
+	    {
+	      z = sizeof (int);
+	      switch ((*p_arg)->type)
+		{
+		case FFI_TYPE_SINT8:
+		  *(signed int *) argp = (signed int) *(SINT8 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_UINT8:
+		  *(unsigned int *) argp =
+		    (unsigned int) *(UINT8 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_SINT16:
+		  *(signed int *) argp = (signed int) *(SINT16 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_UINT16:
+		  *(unsigned int *) argp =
+		    (unsigned int) *(UINT16 *) (*p_argv);
+		  break;
+
+		default:
+		  FFI_ASSERT (0);
+		}
+	    }
+	  else if (z == sizeof (int))
+	    {
+	      *(unsigned int *) argp = (unsigned int) *(UINT32 *) (*p_argv);
+	    }
+	  else
+	    {
+	      memcpy (argp, *p_argv, z);
+	    }
+	  break;
+	}
+      p_argv++;
+      argp += z;
+    }
+
+  return (struct_count);
+}
+
+ffi_status
+ffi_prep_cif_machdep (ffi_cif * cif,
+		      ffi_abi abi,
+		      unsigned int nargs,
+		      ffi_type * rtype, ffi_type ** atypes)
+{
+  unsigned bytes = 0;
+  unsigned int i;
+  ffi_type **ptr;
+
+  FFI_ASSERT (cif != NULL);
+  FFI_ASSERT ((abi > FFI_FIRST_ABI) && (abi <= FFI_DEFAULT_ABI));
+
+  cif->abi = abi;
+  cif->arg_types = atypes;
+  cif->nargs = nargs;
+  cif->rtype = rtype;
+
+  cif->flags = 0;
+
+  if ((cif->rtype->size == 0)
+      && (initialize_aggregate (cif->rtype) != FFI_OK))
+    return FFI_BAD_TYPEDEF;
+
+  FFI_ASSERT_VALID_TYPE (cif->rtype);
+
+  for (ptr = cif->arg_types, i = cif->nargs; i > 0; i--, ptr++)
+    {
+      if ((*ptr)->type == FFI_TYPE_STRUCT)
+	{
+	  if (((*ptr)->size == 0)
+	      && (initialize_aggregate_packed_struct ((*ptr)) != FFI_OK))
+	    return FFI_BAD_TYPEDEF;
+	}
+      else
+	{
+	  if (((*ptr)->size == 0)
+	      && (initialize_aggregate ((*ptr)) != FFI_OK))
+	    return FFI_BAD_TYPEDEF;
+	}
+
+      FFI_ASSERT_VALID_TYPE (*ptr);
+
+      if (((*ptr)->alignment - 1) & bytes)
+	bytes = ALIGN (bytes, (*ptr)->alignment);
+      if ((*ptr)->type == FFI_TYPE_STRUCT)
+	{
+	  if ((*ptr)->size > 8)
+	    {
+	      bytes += (*ptr)->size;
+	      bytes += sizeof (void *);
+	    }
+	  else
+	    {
+	      if ((*ptr)->size > 4)
+		bytes += 8;
+	      else
+		bytes += 4;
+	    }
+	}
+      else
+	{
+	  bytes += STACK_ARG_SIZE ((*ptr)->size);
+	}
+    }
+
+  cif->bytes = bytes;
+
+  switch (cif->rtype->type)
+    {
+    case FFI_TYPE_VOID:
+    case FFI_TYPE_STRUCT:
+    case FFI_TYPE_FLOAT:
+    case FFI_TYPE_DOUBLE:
+    case FFI_TYPE_SINT64:
+    case FFI_TYPE_UINT64:
+      cif->flags = (unsigned) cif->rtype->type;
+      break;
+
+    default:
+      cif->flags = FFI_TYPE_INT;
+      break;
+    }
+
+  return FFI_OK;
+}
+
+
+extern void ffi_call_SYSV (void (*)(char *, extended_cif *),
+			   extended_cif *,
+			   unsigned, unsigned, unsigned *, void (*fn) ());
+
+void
+ffi_call (ffi_cif * cif, void (*fn) (), void *rvalue, void **avalue)
+{
+  extended_cif ecif;
+
+  ecif.cif = cif;
+  ecif.avalue = avalue;
+
+  if ((rvalue == NULL) && (cif->rtype->type == FFI_TYPE_STRUCT))
+    {
+      ecif.rvalue = alloca (cif->rtype->size);
+    }
+  else
+    ecif.rvalue = rvalue;
+
+  switch (cif->abi)
+    {
+    case FFI_SYSV:
+      ffi_call_SYSV (ffi_prep_args, &ecif, cif->bytes,
+		     cif->flags, ecif.rvalue, fn);
+      break;
+    default:
+      FFI_ASSERT (0);
+      break;
+    }
+}
diff -uNrp gcc/libffi/src/cris/ffitarget.h gcc_cris/libffi/src/cris/ffitarget.h
--- gcc/libffi/src/cris/ffitarget.h	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/ffitarget.h	2004-10-26 12:04:38.433195144 +0200
@@ -0,0 +1,47 @@
+/* -----------------------------------------------------------------*-C-*-
+   ffitarget.h - Copyright (c) 1996-2003  Red Hat, Inc.
+   Target configuration macros for CRIS.
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL CYGNUS SOLUTIONS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+
+   ----------------------------------------------------------------------- */
+
+#ifndef LIBFFI_TARGET_H
+#define LIBFFI_TARGET_H
+
+#ifndef LIBFFI_ASM
+typedef unsigned long          ffi_arg;
+typedef signed long            ffi_sarg;
+
+typedef enum ffi_abi {
+  FFI_FIRST_ABI = 0,
+  FFI_SYSV,
+  FFI_DEFAULT_ABI = FFI_SYSV,
+  FFI_LAST_ABI = FFI_DEFAULT_ABI + 1
+} ffi_abi;
+#endif
+
+/* ---- Definitions for closures ----------------------------------------- */
+
+#define FFI_CLOSURES 0
+#define FFI_NATIVE_RAW_API 0
+
+#endif
+
diff -uNrp gcc/libffi/src/cris/sysv.S gcc_cris/libffi/src/cris/sysv.S
--- gcc/libffi/src/cris/sysv.S	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/sysv.S	2004-10-26 12:22:22.577420632 +0200
@@ -0,0 +1,161 @@
+/* -----------------------------------------------------------------------
+   sysv.S - Copyright (c) 2004 Simon Posnjak
+
+   CRIS Foreign Function Interface
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL SIMON POSNJAK BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+   ----------------------------------------------------------------------- */
+
+#define LIBFFI_ASM
+#include <ffi.h>
+
+	.text
+
+	;; OK, when we get called we should have this (according to 
+	;; AXIS ETRAX 100LX Programmer's Manual chapter 6.3).  
+	;; 
+	;; R10:	 ffi_prep_args (func. pointer)
+	;; R11:  &ecif
+	;; R12:  cif->bytes
+	;; R13:  fig->flags
+	;; sp+0: ecif.rvalue
+	;; sp+4: fn (function pointer to the function that we need to call)
+
+	.globl  ffi_call_SYSV
+	.type   ffi_call_SYSV,@function
+
+ffi_call_SYSV:
+	;; Save the regs to the stack.  
+	push $srp
+	;; Used for stack pointer saving.  
+	push $r6
+	;; Used for function address pointer.  
+	push $r7
+	;; Used for stack pointer saving.  
+	push $r8
+	;; We save fig->flags to stack we will need them after we 
+	;; call The Function. 
+	push $r13
+	
+	;; Saving current stack pointer.  
+	move.d $sp,$r8
+	move.d $sp,$r6
+
+	;; Move address of ffi_prep_args to r13.  
+	move.d $r10,$r13
+
+	;; Make room on the stack for the args of fn.  
+	sub.d  $r12,$sp
+
+	;; Function void ffi_prep_args(char *stack, extended_cif *ecif) parameters are:
+	;; 	r10 <-- stack pointer
+	;; 	r11 <-- &ecif (already there)
+	move.d $sp,$r10
+
+	;; Call the function.  
+	jsr $r13
+
+	;; Save the size of the structures which are passed on stack.  
+	move.d $r10,$r7
+
+	;; Move first four args in to r10..r13.  
+	move.d [$sp+0],$r10
+	move.d [$sp+4],$r11
+	move.d [$sp+8],$r12
+	move.d [$sp+12],$r13
+
+	;; Adjust the stack and check if any parameters are given on stack.  
+	addq 16,$sp
+	sub.d $r7,$r6
+	cmp.d $sp,$r6
+
+	bpl go_on
+	nop
+
+go_on_no_params_on_stack:
+	move.d $r6,$sp
+
+go_on:
+	;; Discover if we need to put rval address in to r9.  
+	move.d [$r8+0],$r7
+	cmpq FFI_TYPE_STRUCT,$r7
+	bne call_now
+	nop
+
+	;; Move rval address to $r9.  
+	move.d [$r8+20],$r9
+
+call_now:
+	;; Move address of The Function in to r7.  
+	move.d [$r8+24],$r7
+
+	;; Call The Function.  
+	jsr $r7
+
+	;; Reset stack.  
+	move.d $r8,$sp
+
+	;; Load rval type (fig->flags) in to r13.  
+	pop $r13
+
+	;; Detect rval type.  
+	cmpq FFI_TYPE_VOID,$r13
+	beq epilogue
+
+	cmpq FFI_TYPE_STRUCT,$r13
+	beq epilogue
+
+	cmpq FFI_TYPE_DOUBLE,$r13
+	beq return_double_or_longlong
+
+	cmpq FFI_TYPE_UINT64,$r13
+	beq return_double_or_longlong
+
+	cmpq FFI_TYPE_SINT64,$r13
+	beq return_double_or_longlong
+	nop
+	
+	;; Just return the 32 bit value.  
+	ba return
+	nop
+	
+return_double_or_longlong:
+	;; Load half of the rval to r10 and the other half to r11.
+	move.d [$sp+16],$r13
+	move.d $r10,[$r13]
+	addq 4,$r13
+	move.d $r11,[$r13]
+	ba epilogue
+	nop	
+
+return:
+	;; Load the rval to r10.  
+	move.d [$sp+16],$r13
+	move.d $r10,[$r13]
+	ba epilogue
+	nop
+	
+epilogue:
+	pop $r8
+	pop $r7
+	pop $r6
+	Jump [$sp+]
+
+	.size   ffi_call_SYSV,.-ffi_call_SYSV
diff -uNrp gcc/libffi/src/prep_cif.c gcc_cris/libffi/src/prep_cif.c
--- gcc/libffi/src/prep_cif.c	2004-03-19 23:34:17.000000000 +0100
+++ gcc_cris/libffi/src/prep_cif.c	2004-10-26 12:04:38.000000000 +0200
@@ -25,7 +25,6 @@
 #include <ffi_common.h>
 #include <stdlib.h>
 
-
 /* Round up to FFI_SIZEOF_ARG. */
 
 #define STACK_ARG_SIZE(x) ALIGN(x, FFI_SIZEOF_ARG)
@@ -33,7 +32,7 @@
 /* Perform machine independent initialization of aggregate type
    specifications. */
 
-static ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
+ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
 {
   ffi_type **ptr; 
 
@@ -89,6 +88,9 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@par
 			/*@dependent@*/ /*@out@*/ /*@partial@*/ ffi_type *rtype, 
 			/*@dependent@*/ ffi_type **atypes)
 {
+#ifdef __cris__
+  return ffi_prep_cif_machdep (cif, abi, nargs, rtype, atypes);
+#else
   unsigned bytes = 0;
   unsigned int i;
   ffi_type **ptr;
@@ -147,7 +149,7 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@par
 	  /* Add any padding if necessary */
 	  if (((*ptr)->alignment - 1) & bytes)
 	    bytes = ALIGN(bytes, (*ptr)->alignment);
-	  
+
 	  bytes += STACK_ARG_SIZE((*ptr)->size);
 	}
 #endif
@@ -157,4 +159,5 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@par
 
   /* Perform machine dependent cif processing */
   return ffi_prep_cif_machdep(cif);
+#endif
 }
diff -uNrp gcc/libffi/src/types.c gcc_cris/libffi/src/types.c
--- gcc/libffi/src/types.c	2004-10-13 19:20:24.000000000 +0200
+++ gcc_cris/libffi/src/types.c	2004-10-26 12:04:38.000000000 +0200
@@ -53,7 +53,7 @@ FFI_INTEGRAL_TYPEDEF(pointer, 4, 4, FFI_
 
 #endif
 
-#if defined X86 || defined ARM || defined M68K
+#if defined X86 || defined ARM || defined M68K || defined __cris__
 
 FFI_INTEGRAL_TYPEDEF(uint64, 8, 4, FFI_TYPE_UINT64);
 FFI_INTEGRAL_TYPEDEF(sint64, 8, 4, FFI_TYPE_SINT64);
@@ -80,7 +80,7 @@ FFI_INTEGRAL_TYPEDEF(double, 8, 4, FFI_T
 #endif
 FFI_INTEGRAL_TYPEDEF(longdouble, 12, 4, FFI_TYPE_LONGDOUBLE);
 
-#elif defined ARM || defined SH || defined POWERPC_AIX || defined M32R
+#elif defined ARM || defined SH || defined POWERPC_AIX || defined M32R || defined __cris__
 
 FFI_INTEGRAL_TYPEDEF(double, 8, 4, FFI_TYPE_DOUBLE);
 FFI_INTEGRAL_TYPEDEF(longdouble, 8, 4, FFI_TYPE_LONGDOUBLE);

Re: [PATCH] libffi support for CRIS

[Hans-Peter Nilsson]

(CC as per
<URL:http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02173.html>.
This is a reply to
<URL:http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02170.html>
which didn't CC java-patches.)

Again, thanks for your work!
If you're able to do all the requested fixups I'll be thankful.
I'm ok with your interpretation of what should be in copyright
headers.  (If libffi maintainers think otherwise, they'll
override.)  I'll handle shepherding of the port past libffi
maintainers after your reply (and hopefully last patch
iteration?)

On Tue, 26 Oct 2004, Simon Posnjak wrote:

> V tor, 26.10.2004 ob 04:27 je Hans-Peter Nilsson napisal(a):
> > Over to the actual patch: Please use the "-p" diff option ("diff
> > -up", not just "diff -u").  Sending the patch as an attachment
> > (rather than in-body) made it harder to comment on each piece
> > (to the crowd: comments on my choice of mailer to /dev/null).
> OK. I will do it like that.
(Except this too was in an attachment.  Maybe next time? ;-)

>
> > Same comment applies as to boehm-gc, CRIS32 is a bit confusing.
> > I'd rather just use the predefined __CRIS__ and __linux__ (eh,
> > make that __gnu_linux__ ;-) and not define anything in
> > configury.  Or maybe define CRIS_LINUX.
> I choose CRIS_LINUX.
>
> > I can't approve this as-is, as it touches target-independent
> > libffi bits (which I anyway think should be handled with less
> > #ifdefs).
> I moved the stuff to target-depended part of libffi.

Progress, but a bit more needed, IMO.

> > +++ gcc_cris/libffi/src/cris/ffi.c	2004-10-25 12:58:24.103724264 +0200
> > > +/* -----------------------------------------------------------------------
> > > +   ffi.c - Copyright (c) 1998 Cygnus Solutions
> > > +           Copyright (c) 2004 Simon Posnjak
> >
> > I doubt Cygnus Solutions should be credited here.
> Yes, they should be be, because the file came from arm/ffi.c and was
> adopted for cris. (The copyright headers are correct on all files)

In the GCC repo, the arm/ffi.c header is updated to say
"Red Hat Inc".  But I see what you mean.
(Hm, I hope your patch applies to the GCC repo.)

> > > +++ gcc_cris/libffi/src/cris/ffitarget.h	2004-10-25 12:58:24.104724112 +0200
> > > @@ -0,0 +1,47 @@
> > > +/* -----------------------------------------------------------------*-C-*-
> > > +   ffitarget.h - Copyright (c) 1996-2003  Red Hat, Inc.
> >
> > Why is Red Hat mentioned here?  Copy-paste?
> > Please resend with *at least* this fixed.
> The file is exatcly the same as in arm/ffitarget.h. (s/ARM/CRIS/g)

Hm, I see your point.

There are many targets for which this file is the same.  This
*should* be a generic header used as a default, but that's
supposedly a fallacy of the libffi framework, which shouldn't be
altered in this patch.

> > Hm, if the area ecif.rvalue points to is *always* at least 64
> > bits long, there's no need for different epilogues.  You could
> > remove all the above rval type compares and branches and just do
> > this part:
> It is not always 64 bits long. Only if return value is long long or double, all
> others are 32 bit. Or am I missing something?

A thinko on my part.

Over to your updated patch:

> --- gcc/libffi/include/ffi_common.h	2004-08-30 17:43:02.000000000 +0200

> +#ifdef __cris__

The preferred macro is __CRIS__, but...

> +ffi_status ffi_prep_cif_machdep (ffi_cif * cif,
> +                      ffi_abi abi,
> +                      unsigned int nargs,
> +                      ffi_type * rtype, ffi_type ** atypes);
> +#else
>  ffi_status ffi_prep_cif_machdep(ffi_cif *cif);
> +#endif

...this should be general, not target-dependent -- *if* it had
been a good change.  But... both arguments are moot, see further
below.


> +++ gcc_cris/libffi/src/cris/ffi.c	2004-10-26 12:30:15.674498944 +0200

Not including the copyright header, I didn't see a *single
comment* in this file.  I had to look at the other targets.

> +ffi_status
> +ffi_prep_cif_machdep (ffi_cif * cif,

> +  if ((cif->rtype->size == 0)
> +      && (initialize_aggregate (cif->rtype) != FFI_OK))

Don't you mean initialize_aggregate_packed_struct here?


> diff -uNrp gcc/libffi/src/cris/sysv.S gcc_cris/libffi/src/cris/sysv.S

> +ffi_call_SYSV:
> +	;; Save the regs to the stack.
> +	push $srp
> +	;; Used for stack pointer saving.
> +	push $r6
> +	;; Used for function address pointer.
> +	push $r7
> +	;; Used for stack pointer saving.
> +	push $r8

Still suboptimal register allocation, wrt. my earlier movem comment.

> +	;; Detect rval type.
> +	cmpq FFI_TYPE_VOID,$r13
> +	beq epilogue
> +
> +	cmpq FFI_TYPE_STRUCT,$r13
> +	beq epilogue
> +
> +	cmpq FFI_TYPE_DOUBLE,$r13
> +	beq return_double_or_longlong
> +
> +	cmpq FFI_TYPE_UINT64,$r13
> +	beq return_double_or_longlong
> +
> +	cmpq FFI_TYPE_SINT64,$r13
> +	beq return_double_or_longlong
> +	nop

Maybe I should have added "...but please add (a) comment that the
delay-slot-filling by the next cmpq is deliberate".  (A few other
delay-slots are also fillable, FWIW.)


> +++ gcc_cris/libffi/src/prep_cif.c	2004-10-26 12:04:38.000000000 +0200
> @@ -25,7 +25,6 @@
>  #include <ffi_common.h>
>  #include <stdlib.h>
>
> -
>  /* Round up to FFI_SIZEOF_ARG. */

Gratuitous whitespace fix.  I didn't see many, but try to avoid them.
(I'd hope they are welcome as a general *separate* patch though.)

> @@ -33,7 +32,7 @@
>  /* Perform machine independent initialization of aggregate type
>     specifications. */
>
> -static ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
> +ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)

*If* you'd needed it, I think you should add a initialize_aggregate
declaration in a header somewhere...  right, in ffi_common.h.

On the other hand, I *don't* think you don't need it.  See comment at
callsite above.

> @@ -89,6 +88,9 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@par
> 			  /*@dependent@*/ /*@out@*/ /*@partial@*/ ffi_type *rtype,
> 			  /*@dependent@*/ ffi_type **atypes)
>  {
> +#ifdef __cris__
> +  return ffi_prep_cif_machdep (cif, abi, nargs, rtype, atypes);
> +#else
>    unsigned bytes = 0;
>    unsigned int i;
>    ffi_type **ptr;
> @@ -147,7 +149,7 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@par
> 	    /* Add any padding if necessary */
> 	    if (((*ptr)->alignment - 1) & bytes)
> 	      bytes = ALIGN(bytes, (*ptr)->alignment);
> -
> +
(The only other spurious whitespace fixup I noticed.)
> 	    bytes += STACK_ARG_SIZE((*ptr)->size);
> 	  }
>  #endif
> @@ -157,4 +159,5 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@par
>
>    /* Perform machine dependent cif processing */
>    return ffi_prep_cif_machdep(cif);
> +#endif

Nah, me not really happy with this solution.  (For example, with
the prototype changed to include all the new parameters, you
could just #ifdef out all except the return statement in the
function; presumably better than replacing it.)

On the other hand, you should wrap the *whole* function in #ifdef __CRIS__
(you *could* also do that for initialize_aggregate) and rename the CRIS
ffi_prep_cif_machdep to ffi_prep_cif, completely replacing it.
(That's what I refer to at the top.)

IMHO this is another fallacy of the libffi framework.  Here, it
seems oblivious of the possibility of an ABI that doesn't pad
structures to some alignment.  But fixing that should be a
separate patch, IMHO.

brgds, H-P
PS. It boggles the mind that the main reason for the CRIS ABI having
structures "packed" was once upon a time compatibility with
other compilers!

Re: [PATCH] libffi support for CRIS

[Hans Boehm]

On Wed, 27 Oct 2004, Hans-Peter Nilsson wrote:
> >
> > > Same comment applies as to boehm-gc, CRIS32 is a bit confusing.
> > > I'd rather just use the predefined __CRIS__ and __linux__ (eh,
> > > make that __gnu_linux__ ;-) and not define anything in
> > > configury.  Or maybe define CRIS_LINUX.
> > I choose CRIS_LINUX.
> >
If I understand ths correctly, please don't do this in the context of
the collector.  It currently consistently defines its own macros for
each architecture in order to hide inconsistencies in macros predefined
by various compilers etc.  These macros don't have underscores in them,
in that they are private to the collector implementation.

I would prefer CRIS to CRIS32, especially since I suspect there is no
CRIS64.  (And if there is a CRIS16, it will never be supported by the
collector.)  But in my view, this is minor.

LINUX should of course also remain defined.

Hans

Re: [PATCH] libffi support for CRIS

[Hans-Peter Nilsson]

On Wed, 27 Oct 2004, Hans Boehm wrote:
> On Wed, 27 Oct 2004, Hans-Peter Nilsson wrote:
> > >
> > > > Same comment applies as to boehm-gc, CRIS32 is a bit confusing.
> > > > I'd rather just use the predefined __CRIS__ and __linux__ (eh,
> > > > make that __gnu_linux__ ;-) and not define anything in
> > > > configury.  Or maybe define CRIS_LINUX.
> > > I choose CRIS_LINUX.
> > >
> If I understand ths correctly, please don't do this in the context of
> the collector.  It currently consistently defines its own macros for
> each architecture in order to hide inconsistencies in macros predefined
> by various compilers etc.  These macros don't have underscores in them,
> in that they are private to the collector implementation.
>
> I would prefer CRIS to CRIS32, especially since I suspect there is no
> CRIS64.

CRIS is already defined (unless -iso -anso or -younameit-standard)
in addition to __CRIS__.

>  (And if there is a CRIS16, it will never be supported by the
> collector.)  But in my view, this is minor.

There's no CRIS16 and no plans for it. :-)

> LINUX should of course also remain defined.

It's not touched.  Maybe there's a misunderstanding here?

So, judging from your comments, defining CRIS_LINUX in boehm-gc
configury to identify the port is fine by you?
And you too prefer it to CRIS32?  Then we're all set!

brgds, H-P

Re: [PATCH] libffi support for CRIS

[Hans Boehm]

If we're refering to the patch in
http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02057.html
as I think we are, then I'd prefer that gcconfig.h just leaves the
existing definition of CRIS in place, with a comment to that effect,
and tests CRIS later.  The current convention doesn't use
architecture_os macros; it uses separate macros for each, i.e.
CRIS and LINUX in this case.

Thanks.

Hans

On Thu, 28 Oct 2004, Hans-Peter Nilsson wrote:

> On Wed, 27 Oct 2004, Hans Boehm wrote:
> > On Wed, 27 Oct 2004, Hans-Peter Nilsson wrote:
> > > >
> > > > > Same comment applies as to boehm-gc, CRIS32 is a bit confusing.
> > > > > I'd rather just use the predefined __CRIS__ and __linux__ (eh,
> > > > > make that __gnu_linux__ ;-) and not define anything in
> > > > > configury.  Or maybe define CRIS_LINUX.
> > > > I choose CRIS_LINUX.
> > > >
> > If I understand ths correctly, please don't do this in the context of
> > the collector.  It currently consistently defines its own macros for
> > each architecture in order to hide inconsistencies in macros predefined
> > by various compilers etc.  These macros don't have underscores in them,
> > in that they are private to the collector implementation.
> >
> > I would prefer CRIS to CRIS32, especially since I suspect there is no
> > CRIS64.
>
> CRIS is already defined (unless -iso -anso or -younameit-standard)
> in addition to __CRIS__.
>
> >  (And if there is a CRIS16, it will never be supported by the
> > collector.)  But in my view, this is minor.
>
> There's no CRIS16 and no plans for it. :-)
>
> > LINUX should of course also remain defined.
>
> It's not touched.  Maybe there's a misunderstanding here?
>
> So, judging from your comments, defining CRIS_LINUX in boehm-gc
> configury to identify the port is fine by you?
> And you too prefer it to CRIS32?  Then we're all set!
>
> brgds, H-P
>

Re: [PATCH] libffi support for CRIS

[Hans-Peter Nilsson]

On Wed, 27 Oct 2004, Hans Boehm wrote:
> If we're refering to the patch in
> http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02057.html
> as I think we are, then I'd prefer that gcconfig.h just leaves the
> existing definition of CRIS in place, with a comment to that effect,
> and tests CRIS later.

Ok.  That *could* run afoul of compiler options forcing standard
compliance being added (very popular these days), but we can
deal with that when it happens.

brgds, H-P

Re: [PATCH] libffi support for CRIS

[Simon Posnjak]

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

V čet, 28.10.2004 ob 05:19 je Hans-Peter Nilsson napisal(a):
> In the GCC repo, the arm/ffi.c header is updated to say
> "Red Hat Inc".  But I see what you mean.
> (Hm, I hope your patch applies to the GCC repo.)
Yes, to todays CVS.

> Hm, I see your point.
> 
> There are many targets for which this file is the same.  This
> *should* be a generic header used as a default, but that's
> supposedly a fallacy of the libffi framework, which shouldn't be
> altered in this patch.
OK.

> Over to your updated patch:
> 
> > --- gcc/libffi/include/ffi_common.h	2004-08-30 17:43:02.000000000 +0200
> 
> > +#ifdef __cris__
> 
> The preferred macro is __CRIS__, but...
Changed.

> > +++ gcc_cris/libffi/src/cris/ffi.c	2004-10-26 12:30:15.674498944 +0200
> 
> Not including the copyright header, I didn't see a *single
> comment* in this file.  I had to look at the other targets.
> 
> > +ffi_status
> > +ffi_prep_cif_machdep (ffi_cif * cif,
> 
> > +  if ((cif->rtype->size == 0)
> > +      && (initialize_aggregate (cif->rtype) != FFI_OK))
> 
> Don't you mean initialize_aggregate_packed_struct here?
No, thats correct - this one works on aligned parameters, not packed.

> > +	;; Used for stack pointer saving.
> > +	push $r8
> 
> Still suboptimal register allocation, wrt. my earlier movem comment.
This is still open. I do not have the time (and my knowledge of CRIS asm
is not that good) to fix it right now. (I tried a quick fix but I am
getting just segfaults).

> > @@ -33,7 +32,7 @@
> >  /* Perform machine independent initialization of aggregate type
> >     specifications. */
> >
> > -static ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
> > +ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
> 
> *If* you'd needed it, I think you should add a initialize_aggregate
> declaration in a header somewhere...  right, in ffi_common.h.
Added.

> Nah, me not really happy with this solution.  (For example, with
> the prototype changed to include all the new parameters, you
> could just #ifdef out all except the return statement in the
> function; presumably better than replacing it.)
> 
> On the other hand, you should wrap the *whole* function in #ifdef __CRIS__
> (you *could* also do that for initialize_aggregate) and rename the CRIS
> ffi_prep_cif_machdep to ffi_prep_cif, completely replacing it.
> (That's what I refer to at the top.)
Done.

		Regards Simon


[-- Attachment #2: gcc_cris_libffi_cleanup_of_cleanup_submit.diff --]
[-- Type: text/x-patch, Size: 17376 bytes --]

diff -uNrp gcc/libffi/Makefile.am gcc_cris/libffi/Makefile.am
--- gcc/libffi/Makefile.am	2004-10-13 19:20:21.000000000 +0200
+++ gcc_cris/libffi/Makefile.am	2004-10-28 11:36:39.849423752 +0200
@@ -7,6 +7,7 @@ SUBDIRS = include testsuite
 EXTRA_DIST = LICENSE ChangeLog.v1 \
 	src/alpha/ffi.c src/alpha/osf.S src/alpha/ffitarget.h \
 	src/arm/ffi.c src/arm/sysv.S src/arm/ffitarget.h \
+	src/cris/ffi.c src/cris/sysv.S src/cris/ffitarget.h \
 	src/mips/ffi.c src/mips/n32.S src/mips/o32.S \
 	src/mips/ffitarget.h \
 	src/m32r/ffi.c src/m32r/sysv.S src/m32r/ffitarget.h \
@@ -120,6 +121,9 @@ endif
 if ARM
 nodist_libffi_la_SOURCES += src/arm/sysv.S src/arm/ffi.c
 endif
+if CRIS_LINUX
+nodist_libffi_la_SOURCES += src/cris/sysv.S src/cris/ffi.c
+endif
 if FRV
 nodist_libffi_la_SOURCES += src/frv/eabi.S src/frv/ffi.c
 endif
diff -uNrp gcc/libffi/configure.ac gcc_cris/libffi/configure.ac
--- gcc/libffi/configure.ac	2004-10-13 19:20:23.000000000 +0200
+++ gcc_cris/libffi/configure.ac	2004-10-28 11:36:39.850423600 +0200
@@ -68,6 +68,7 @@ powerpc-*-aix*) TARGET=POWERPC_AIX; TARG
 rs6000-*-aix*) TARGET=POWERPC_AIX; TARGETDIR=powerpc;;
 arm*-*-linux-*) TARGET=ARM; TARGETDIR=arm;;
 arm*-*-netbsdelf* | arm*-*-knetbsd*-gnu) TARGET=ARM; TARGETDIR=arm;;
+cris-*-linux-*) TARGET=CRIS_LINUX; TARGETDIR=cris;;
 s390-*-linux-*) TARGET=S390; TARGETDIR=s390;;
 s390x-*-linux-*) TARGET=S390; TARGETDIR=s390;;
 x86_64-*-linux* | x86_64-*-freebsd* | x86_64-*-kfreebsd*-gnu) TARGET=X86_64; TARGETDIR=x86;;
@@ -95,6 +96,7 @@ AM_CONDITIONAL(POWERPC, test x$TARGET = 
 AM_CONDITIONAL(POWERPC_AIX, test x$TARGET = xPOWERPC_AIX)
 AM_CONDITIONAL(POWERPC_DARWIN, test x$TARGET = xPOWERPC_DARWIN)
 AM_CONDITIONAL(ARM, test x$TARGET = xARM)
+AM_CONDITIONAL(CRIS_LINUX, test x$TARGET = xCRIS_LINUX)
 AM_CONDITIONAL(FRV, test x$TARGET = xFRV)
 AM_CONDITIONAL(S390, test x$TARGET = xS390)
 AM_CONDITIONAL(X86_64, test x$TARGET = xX86_64)
diff -uNrp gcc/libffi/include/ffi_common.h gcc_cris/libffi/include/ffi_common.h
--- gcc/libffi/include/ffi_common.h	2004-08-30 17:43:02.000000000 +0200
+++ gcc_cris/libffi/include/ffi_common.h	2004-10-28 11:36:39.851423448 +0200
@@ -62,6 +62,8 @@ void ffi_type_test(/*@temp@*/ /*@out@*/ 
 #define ALIGN(v, a)  (((((size_t) (v))-1) | ((a)-1))+1)
 #define ALIGN_DOWN(v, a) (((size_t) (v)) & -a)
 
+ffi_status initialize_aggregate(ffi_type *arg);
+
 /* Perform machine dependent cif processing */
 ffi_status ffi_prep_cif_machdep(ffi_cif *cif);
 
diff -uNrp gcc/libffi/src/cris/ffi.c gcc_cris/libffi/src/cris/ffi.c
--- gcc/libffi/src/cris/ffi.c	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/ffi.c	2004-10-28 11:36:39.852423296 +0200
@@ -0,0 +1,274 @@
+/* -----------------------------------------------------------------------
+   ffi.c - Copyright (c) 1998 Cygnus Solutions
+           Copyright (c) 2004 Simon Posnjak
+
+   CRIS Foreign Function Interface
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL SIMON POSNJAK BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+   ----------------------------------------------------------------------- */
+
+#include <ffi.h>
+#include <ffi_common.h>
+
+#define STACK_ARG_SIZE(x) ALIGN(x, FFI_SIZEOF_ARG)
+
+static ffi_status
+initialize_aggregate_packed_struct (ffi_type * arg)
+{
+  ffi_type **ptr;
+
+  FFI_ASSERT (arg != NULL);
+
+  FFI_ASSERT (arg->elements != NULL);
+  FFI_ASSERT (arg->size == 0);
+  FFI_ASSERT (arg->alignment == 0);
+
+  ptr = &(arg->elements[0]);
+
+  while ((*ptr) != NULL)
+    {
+      if (((*ptr)->size == 0)
+	  && (initialize_aggregate_packed_struct ((*ptr)) != FFI_OK))
+	return FFI_BAD_TYPEDEF;
+
+      FFI_ASSERT (ffi_type_test ((*ptr)));
+
+      arg->size += (*ptr)->size;
+
+      arg->alignment = (arg->alignment > (*ptr)->alignment) ?
+	arg->alignment : (*ptr)->alignment;
+
+      ptr++;
+    }
+
+  if (arg->size == 0)
+    return FFI_BAD_TYPEDEF;
+  else
+    return FFI_OK;
+}
+
+int
+ffi_prep_args (char *stack, extended_cif * ecif)
+{
+  register unsigned int i;
+  register unsigned int struct_count = 0;
+  register void **p_argv;
+  register char *argp;
+  register ffi_type **p_arg;
+
+  argp = stack;
+
+  p_argv = ecif->avalue;
+
+  for (i = ecif->cif->nargs, p_arg = ecif->cif->arg_types;
+       (i != 0); i--, p_arg++)
+    {
+      size_t z;
+
+      switch ((*p_arg)->type)
+	{
+	case FFI_TYPE_STRUCT:
+	  {
+	    z = (*p_arg)->size;
+	    if (z <= 8)
+	      {
+		memcpy (argp, *p_argv, z);
+		z += (4 - (z % 4));
+	      }
+	    else
+	      {
+		unsigned int uiLocOnStack;
+
+		uiLocOnStack = 4 * ecif->cif->nargs + struct_count;
+		struct_count = struct_count + (*p_arg)->size;
+		*(unsigned int *) argp =
+		  (unsigned int) (UINT32 *) (stack + uiLocOnStack);
+		memcpy ((stack + uiLocOnStack), *p_argv, (*p_arg)->size);
+	      }
+	    break;
+	  }
+	default:
+	  z = (*p_arg)->size;
+	  if (z < sizeof (int))
+	    {
+	      z = sizeof (int);
+	      switch ((*p_arg)->type)
+		{
+		case FFI_TYPE_SINT8:
+		  *(signed int *) argp = (signed int) *(SINT8 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_UINT8:
+		  *(unsigned int *) argp =
+		    (unsigned int) *(UINT8 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_SINT16:
+		  *(signed int *) argp = (signed int) *(SINT16 *) (*p_argv);
+		  break;
+
+		case FFI_TYPE_UINT16:
+		  *(unsigned int *) argp =
+		    (unsigned int) *(UINT16 *) (*p_argv);
+		  break;
+
+		default:
+		  FFI_ASSERT (0);
+		}
+	    }
+	  else if (z == sizeof (int))
+	    {
+	      *(unsigned int *) argp = (unsigned int) *(UINT32 *) (*p_argv);
+	    }
+	  else
+	    {
+	      memcpy (argp, *p_argv, z);
+	    }
+	  break;
+	}
+      p_argv++;
+      argp += z;
+    }
+
+  return (struct_count);
+}
+
+ffi_status
+ffi_prep_cif (ffi_cif * cif,
+	      ffi_abi abi, unsigned int nargs,
+	      ffi_type * rtype, ffi_type ** atypes)
+{
+  unsigned bytes = 0;
+  unsigned int i;
+  ffi_type **ptr;
+
+  FFI_ASSERT (cif != NULL);
+  FFI_ASSERT ((abi > FFI_FIRST_ABI) && (abi <= FFI_DEFAULT_ABI));
+
+  cif->abi = abi;
+  cif->arg_types = atypes;
+  cif->nargs = nargs;
+  cif->rtype = rtype;
+
+  cif->flags = 0;
+
+  if ((cif->rtype->size == 0)
+      && (initialize_aggregate (cif->rtype) != FFI_OK))
+    return FFI_BAD_TYPEDEF;
+
+  FFI_ASSERT_VALID_TYPE (cif->rtype);
+
+  for (ptr = cif->arg_types, i = cif->nargs; i > 0; i--, ptr++)
+    {
+      if ((*ptr)->type == FFI_TYPE_STRUCT)
+	{
+	  if (((*ptr)->size == 0)
+	      && (initialize_aggregate_packed_struct ((*ptr)) != FFI_OK))
+	    return FFI_BAD_TYPEDEF;
+	}
+      else
+	{
+	  if (((*ptr)->size == 0)
+	      && (initialize_aggregate ((*ptr)) != FFI_OK))
+	    return FFI_BAD_TYPEDEF;
+	}
+
+      FFI_ASSERT_VALID_TYPE (*ptr);
+
+      if (((*ptr)->alignment - 1) & bytes)
+	bytes = ALIGN (bytes, (*ptr)->alignment);
+      if ((*ptr)->type == FFI_TYPE_STRUCT)
+	{
+	  if ((*ptr)->size > 8)
+	    {
+	      bytes += (*ptr)->size;
+	      bytes += sizeof (void *);
+	    }
+	  else
+	    {
+	      if ((*ptr)->size > 4)
+		bytes += 8;
+	      else
+		bytes += 4;
+	    }
+	}
+      else
+	{
+	  bytes += STACK_ARG_SIZE ((*ptr)->size);
+	}
+    }
+    
+  cif->bytes = bytes;
+  
+  return ffi_prep_cif_machdep (cif);
+}
+
+ffi_status
+ffi_prep_cif_machdep (ffi_cif * cif)
+{
+  switch (cif->rtype->type)
+    {
+    case FFI_TYPE_VOID:
+    case FFI_TYPE_STRUCT:
+    case FFI_TYPE_FLOAT:
+    case FFI_TYPE_DOUBLE:
+    case FFI_TYPE_SINT64:
+    case FFI_TYPE_UINT64:
+      cif->flags = (unsigned) cif->rtype->type;
+      break;
+
+    default:
+      cif->flags = FFI_TYPE_INT;
+      break;
+    }
+
+  return FFI_OK;
+}
+
+extern void ffi_call_SYSV (void (*)(char *, extended_cif *),
+			   extended_cif *,
+			   unsigned, unsigned, unsigned *, void (*fn) ());
+
+void
+ffi_call (ffi_cif * cif, void (*fn) (), void *rvalue, void **avalue)
+{
+  extended_cif ecif;
+
+  ecif.cif = cif;
+  ecif.avalue = avalue;
+
+  if ((rvalue == NULL) && (cif->rtype->type == FFI_TYPE_STRUCT))
+    {
+      ecif.rvalue = alloca (cif->rtype->size);
+    }
+  else
+    ecif.rvalue = rvalue;
+
+  switch (cif->abi)
+    {
+    case FFI_SYSV:
+      ffi_call_SYSV (ffi_prep_args, &ecif, cif->bytes,
+		     cif->flags, ecif.rvalue, fn);
+      break;
+    default:
+      FFI_ASSERT (0);
+      break;
+    }
+}
diff -uNrp gcc/libffi/src/cris/ffitarget.h gcc_cris/libffi/src/cris/ffitarget.h
--- gcc/libffi/src/cris/ffitarget.h	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/ffitarget.h	2004-10-28 11:36:39.853423144 +0200
@@ -0,0 +1,47 @@
+/* -----------------------------------------------------------------*-C-*-
+   ffitarget.h - Copyright (c) 1996-2003  Red Hat, Inc.
+   Target configuration macros for CRIS.
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL CYGNUS SOLUTIONS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+
+   ----------------------------------------------------------------------- */
+
+#ifndef LIBFFI_TARGET_H
+#define LIBFFI_TARGET_H
+
+#ifndef LIBFFI_ASM
+typedef unsigned long          ffi_arg;
+typedef signed long            ffi_sarg;
+
+typedef enum ffi_abi {
+  FFI_FIRST_ABI = 0,
+  FFI_SYSV,
+  FFI_DEFAULT_ABI = FFI_SYSV,
+  FFI_LAST_ABI = FFI_DEFAULT_ABI + 1
+} ffi_abi;
+#endif
+
+/* ---- Definitions for closures ----------------------------------------- */
+
+#define FFI_CLOSURES 0
+#define FFI_NATIVE_RAW_API 0
+
+#endif
+
diff -uNrp gcc/libffi/src/cris/sysv.S gcc_cris/libffi/src/cris/sysv.S
--- gcc/libffi/src/cris/sysv.S	1970-01-01 01:00:00.000000000 +0100
+++ gcc_cris/libffi/src/cris/sysv.S	2004-10-28 11:36:39.854422992 +0200
@@ -0,0 +1,161 @@
+/* -----------------------------------------------------------------------
+   sysv.S - Copyright (c) 2004 Simon Posnjak
+
+   CRIS Foreign Function Interface
+
+   Permission is hereby granted, free of charge, to any person obtaining
+   a copy of this software and associated documentation files (the
+   ``Software''), to deal in the Software without restriction, including
+   without limitation the rights to use, copy, modify, merge, publish,
+   distribute, sublicense, and/or sell copies of the Software, and to
+   permit persons to whom the Software is furnished to do so, subject to
+   the following conditions:
+
+   The above copyright notice and this permission notice shall be included
+   in all copies or substantial portions of the Software.
+
+   THE SOFTWARE IS PROVIDED ``AS IS'', WITHOUT WARRANTY OF ANY KIND, EXPRESS
+   OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+   IN NO EVENT SHALL SIMON POSNJAK BE LIABLE FOR ANY CLAIM, DAMAGES OR
+   OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+   ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+   OTHER DEALINGS IN THE SOFTWARE.
+   ----------------------------------------------------------------------- */
+
+#define LIBFFI_ASM
+#include <ffi.h>
+
+	.text
+
+	;; OK, when we get called we should have this (according to 
+	;; AXIS ETRAX 100LX Programmer's Manual chapter 6.3).  
+	;; 
+	;; R10:	 ffi_prep_args (func. pointer)
+	;; R11:  &ecif
+	;; R12:  cif->bytes
+	;; R13:  fig->flags
+	;; sp+0: ecif.rvalue
+	;; sp+4: fn (function pointer to the function that we need to call)
+
+	.globl  ffi_call_SYSV
+	.type   ffi_call_SYSV,@function
+
+ffi_call_SYSV:
+	;; Save the regs to the stack.  
+	push $srp
+	;; Used for stack pointer saving.  
+	push $r6
+	;; Used for function address pointer.  
+	push $r7
+	;; Used for stack pointer saving.  
+	push $r8
+	;; We save fig->flags to stack we will need them after we 
+	;; call The Function. 
+	push $r13
+	
+	;; Saving current stack pointer.  
+	move.d $sp,$r8
+	move.d $sp,$r6
+
+	;; Move address of ffi_prep_args to r13.  
+	move.d $r10,$r13
+
+	;; Make room on the stack for the args of fn.  
+	sub.d  $r12,$sp
+
+	;; Function void ffi_prep_args(char *stack, extended_cif *ecif) parameters are:
+	;; 	r10 <-- stack pointer
+	;; 	r11 <-- &ecif (already there)
+	move.d $sp,$r10
+
+	;; Call the function.  
+	jsr $r13
+
+	;; Save the size of the structures which are passed on stack.  
+	move.d $r10,$r7
+
+	;; Move first four args in to r10..r13.  
+	move.d [$sp+0],$r10
+	move.d [$sp+4],$r11
+	move.d [$sp+8],$r12
+	move.d [$sp+12],$r13
+
+	;; Adjust the stack and check if any parameters are given on stack.  
+	addq 16,$sp
+	sub.d $r7,$r6
+	cmp.d $sp,$r6
+
+	bpl go_on
+	nop
+
+go_on_no_params_on_stack:
+	move.d $r6,$sp
+
+go_on:
+	;; Discover if we need to put rval address in to r9.  
+	move.d [$r8+0],$r7
+	cmpq FFI_TYPE_STRUCT,$r7
+	bne call_now
+	nop
+
+	;; Move rval address to $r9.  
+	move.d [$r8+20],$r9
+
+call_now:
+	;; Move address of The Function in to r7.  
+	move.d [$r8+24],$r7
+
+	;; Call The Function.  
+	jsr $r7
+
+	;; Reset stack.  
+	move.d $r8,$sp
+
+	;; Load rval type (fig->flags) in to r13.  
+	pop $r13
+
+	;; Detect rval type.  
+	cmpq FFI_TYPE_VOID,$r13
+	beq epilogue
+
+	cmpq FFI_TYPE_STRUCT,$r13
+	beq epilogue
+
+	cmpq FFI_TYPE_DOUBLE,$r13
+	beq return_double_or_longlong
+
+	cmpq FFI_TYPE_UINT64,$r13
+	beq return_double_or_longlong
+
+	cmpq FFI_TYPE_SINT64,$r13
+	beq return_double_or_longlong
+	nop
+	
+	;; Just return the 32 bit value.  
+	ba return
+	nop
+	
+return_double_or_longlong:
+	;; Load half of the rval to r10 and the other half to r11.
+	move.d [$sp+16],$r13
+	move.d $r10,[$r13]
+	addq 4,$r13
+	move.d $r11,[$r13]
+	ba epilogue
+	nop	
+
+return:
+	;; Load the rval to r10.  
+	move.d [$sp+16],$r13
+	move.d $r10,[$r13]
+	ba epilogue
+	nop
+	
+epilogue:
+	pop $r8
+	pop $r7
+	pop $r6
+	Jump [$sp+]
+
+	.size   ffi_call_SYSV,.-ffi_call_SYSV
diff -uNrp gcc/libffi/src/prep_cif.c gcc_cris/libffi/src/prep_cif.c
--- gcc/libffi/src/prep_cif.c	2004-03-19 23:34:17.000000000 +0100
+++ gcc_cris/libffi/src/prep_cif.c	2004-10-28 11:36:56.770851304 +0200
@@ -33,7 +33,7 @@
 /* Perform machine independent initialization of aggregate type
    specifications. */
 
-static ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
+ffi_status initialize_aggregate(/*@out@*/ ffi_type *arg)
 {
   ffi_type **ptr; 
 
@@ -83,7 +83,7 @@ static ffi_status initialize_aggregate(/
 
 /* Perform machine independent ffi_cif preparation, then call
    machine dependent routine. */
-
+#ifndef __CRIS__
 ffi_status ffi_prep_cif(/*@out@*/ /*@partial@*/ ffi_cif *cif, 
 			ffi_abi abi, unsigned int nargs, 
 			/*@dependent@*/ /*@out@*/ /*@partial@*/ ffi_type *rtype, 
@@ -158,3 +158,4 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@par
   /* Perform machine dependent cif processing */
   return ffi_prep_cif_machdep(cif);
 }
+#endif
diff -uNrp gcc/libffi/src/types.c gcc_cris/libffi/src/types.c
--- gcc/libffi/src/types.c	2004-10-13 19:20:24.000000000 +0200
+++ gcc_cris/libffi/src/types.c	2004-10-28 11:36:39.856422688 +0200
@@ -53,7 +53,7 @@ FFI_INTEGRAL_TYPEDEF(pointer, 4, 4, FFI_
 
 #endif
 
-#if defined X86 || defined ARM || defined M68K
+#if defined X86 || defined ARM || defined M68K || defined __CRIS__
 
 FFI_INTEGRAL_TYPEDEF(uint64, 8, 4, FFI_TYPE_UINT64);
 FFI_INTEGRAL_TYPEDEF(sint64, 8, 4, FFI_TYPE_SINT64);
@@ -80,7 +80,7 @@ FFI_INTEGRAL_TYPEDEF(double, 8, 4, FFI_T
 #endif
 FFI_INTEGRAL_TYPEDEF(longdouble, 12, 4, FFI_TYPE_LONGDOUBLE);
 
-#elif defined ARM || defined SH || defined POWERPC_AIX || defined M32R
+#elif defined ARM || defined SH || defined POWERPC_AIX || defined M32R || defined __CRIS__
 
 FFI_INTEGRAL_TYPEDEF(double, 8, 4, FFI_TYPE_DOUBLE);
 FFI_INTEGRAL_TYPEDEF(longdouble, 8, 4, FFI_TYPE_LONGDOUBLE);

Re: [PATCH] libffi support for CRIS

[Hans-Peter Nilsson]

I'll fix up all issues below and would like to start negotiation
with libffi approvers.  I'd like to hear from them about this
patch (for reference,
<URL:http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02477.html>).

I ask for approval on the target-independent changes there,
minus the globalization of initialize_aggregate plus comments
at the #ifdef additions to the effect of
/* The CRIS ABI specifies structure elements to the effect of
   byte alignment only, so it completely overrides this
   functions, which assumes "natural" alignment and padding.  */

On Thu, 28 Oct 2004, Simon Posnjak wrote:

Huh, again sent as an attachment, not in-text.  It takes a
little longer to review it this way.  Though that's not the
main reason for this delay, sorry.

> V čet, 28.10.2004 ob 05:19 je Hans-Peter Nilsson napisal(a):
> > > +++ gcc_cris/libffi/src/cris/ffi.c	2004-10-26 12:30:15.674498944 +0200
> >
> > Not including the copyright header, I didn't see a *single
> > comment* in this file.  I had to look at the other targets.

I still don't see *any* comments in cris/ffi.c and a few "{
statement; }" where no braces should be as per the coding
standards.

> > > +ffi_status
> > > +ffi_prep_cif_machdep (ffi_cif * cif,
> >
> > > +  if ((cif->rtype->size == 0)
> > > +      && (initialize_aggregate (cif->rtype) != FFI_OK))
> >
> > Don't you mean initialize_aggregate_packed_struct here?
> No, thats correct - this one works on aligned parameters, not packed.

What do you mean by that?  The return value doesn't have any
different layout than parameters; it's "packed" too.  I see no
reason to not call initialize_aggregate_packed_struct there.

> > > +	;; Used for stack pointer saving.
> > > +	push $r8
> >
> > Still suboptimal register allocation, wrt. my earlier movem comment.
> This is still open. I do not have the time (and my knowledge of CRIS asm
> is not that good) to fix it right now. (I tried a quick fix but I am
> getting just segfaults).

If you care to revisit some time, look at output from an -O2
compilation.

> > On the other hand, you should wrap the *whole* function in #ifdef __CRIS__
> > (you *could* also do that for initialize_aggregate) and rename the CRIS
> > ffi_prep_cif_machdep to ffi_prep_cif, completely replacing it.
> > (That's what I refer to at the top.)
> Done.

Much better.  A comment at the #ifdef would help followers,
though.

brgds, H-P

Re: [PATCH] libffi support for CRIS

[Simon Posnjak]

V sob, 06.11.2004 ob 18:48 je Hans-Peter Nilsson napisal(a):
> Huh, again sent as an attachment, not in-text.  It takes a
> little longer to review it this way.  Though that's not the
> main reason for this delay, sorry.
It's my dam mailer. I told it, it should send the attachments in-line
but is still does not do it (copy&past makes a mess of the patch).
Sorry.

> > > > +ffi_status
> > > > +ffi_prep_cif_machdep (ffi_cif * cif,
> > >
> > > > +  if ((cif->rtype->size == 0)
> > > > +      && (initialize_aggregate (cif->rtype) != FFI_OK))
> > >
> > > Don't you mean initialize_aggregate_packed_struct here?
> > No, thats correct - this one works on aligned parameters, not packed.
> 
> What do you mean by that?  The return value doesn't have any
> different layout than parameters; it's "packed" too.  I see no
> reason to not call initialize_aggregate_packed_struct there.
There was a reason why I left that in, but i can not recall what it was.
Does the ffitest (or the test-suite) work if you comment it out? [I
would try it myself but i really do not have the time to do it right
now]

> > > > +	;; Used for stack pointer saving.
> > > > +	push $r8
> > >
> > > Still suboptimal register allocation, wrt. my earlier movem comment.
> > This is still open. I do not have the time (and my knowledge of CRIS asm
> > is not that good) to fix it right now. (I tried a quick fix but I am
> > getting just segfaults).
> 
> If you care to revisit some time, look at output from an -O2
> compilation.
OK, I will look in to it when I find the time.

			Regards Simon

Re: [PATCH] libffi support for CRIS

[Hans-Peter Nilsson]

On Mon, 8 Nov 2004, Simon Posnjak wrote:
> V sob, 06.11.2004 ob 18:48 je Hans-Peter Nilsson napisal(a):
> > > > Don't you mean initialize_aggregate_packed_struct here?
> > > No, thats correct - this one works on aligned parameters, not packed.
> > What do you mean by that?  The return value doesn't have any
> > different layout than parameters; it's "packed" too.  I see no
> > reason to not call initialize_aggregate_packed_struct there.
> There was a reason why I left that in, but i can not recall what it was.

That's a good hint that a comment is needed there, whatever the
call should be.  (SCNR.)

> Does the ffitest (or the test-suite) work if you comment it out? [I
> would try it myself but i really do not have the time to do it right
> now]

I haven't run it, but I will not commit anything without running
whatever testsuite there is.

Now we wait for a say from the libffi maintainers regarding the
modifications to the target-independent files.

brgds, H-P

Re: [PATCH] libffi support for CRIS

[Tom Tromey]

>>>>> "H-P" == Hans-Peter Nilsson <hp@bitrange.com> writes:

H-P> I'll fix up all issues below and would like to start negotiation
H-P> with libffi approvers.  I'd like to hear from them about this
H-P> patch (for reference,
H-P> <URL:http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02477.html>).

Sorry for the long delay on this.

H-P> I ask for approval on the target-independent changes there,
H-P> minus the globalization of initialize_aggregate plus comments
H-P> at the #ifdef additions to the effect of
H-P> /* The CRIS ABI specifies structure elements to the effect of
H-P>    byte alignment only, so it completely overrides this
H-P>    functions, which assumes "natural" alignment and padding.  */

Makefile.am: Ok
configure.ac: Ok
prep_cif.c:  with the omission you mention, ok
types.c: Ok

For the initialize_aggregate change, if that function becomes
non-static then it will have to have an "ffi_" prefix added to its
name.

Let me know if there's something I missed.

Perhaps someday it would be nice to have things like the #ifndef
around ffi_prep_cif be controlled by a feature macro set in the port's
header file, rather than ad hoc target defines all over.

Tom