Re: [PATCH RFC] spu return rc value from __send_to_ppe

[Kazunori Asayama <asayama at sm dot sony dot co dot jp>]

Patrick Mansfield <patmans@us.ibm.com> wrote:
> Per Ulrich Weigand's suggestion, return the rc value in __send_to_ppe,
> rather than doing so in each each caller of __send_to_ppe, so no code is
> generated to return the value (since the return value is already in
> register 3).
> 
> This moves 8 bytes (2 instructions) from the caller into __send_to_ppe. So
> to see any savings in size, you have to call two different assist calls.

Sounds good.

Here are my minor comments:

> Index: my-base-quilt/libgloss/spu/access.c
> ===================================================================
> --- my-base-quilt.orig/libgloss/spu/access.c
> +++ my-base-quilt/libgloss/spu/access.c
> @@ -40,8 +40,6 @@ access (const char *pathname, int mode)
>          sys.pathname = (unsigned int) pathname;
>          sys.mode = mode;
>  
> -        __send_to_ppe (JSRE_POSIX1_SIGNALCODE, JSRE_ACCESS, &sys);
> -
> -        return ( psys_out->rc);
> +        return __send_to_ppe (JSRE_POSIX1_SIGNALCODE, JSRE_ACCESS, &sys);
>  }

The variable 'psys_out' is no longer needed.

> Index: my-base-quilt/libgloss/spu/close.c
> Index: my-base-quilt/libgloss/spu/dup.c
> Index: my-base-quilt/libgloss/spu/fstat.c
> Index: my-base-quilt/libgloss/spu/ftruncate.c
> Index: my-base-quilt/libgloss/spu/gettimeofday.c
> Index: my-base-quilt/libgloss/spu/lseek.c
> Index: my-base-quilt/libgloss/spu/open.c
> Index: my-base-quilt/libgloss/spu/read.c
> Index: my-base-quilt/libgloss/spu/stat.c
> Index: my-base-quilt/libgloss/spu/unlink.c
> Index: my-base-quilt/libgloss/spu/write.c

Ditto.

> Index: my-base-quilt/newlib/libc/machine/spu/fgetpos.c
> ===================================================================
> --- my-base-quilt.orig/newlib/libc/machine/spu/fgetpos.c
> +++ my-base-quilt/newlib/libc/machine/spu/fgetpos.c
> @@ -58,9 +58,6 @@ _DEFUN (fgetpos, (fp, pos),
>    arg.fp = fp->_fp;
>    arg.pos = pos;
>  
> -  __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FGETPOS, &arg);
> -
> -
> -  return *result;
> +  return __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FGETPOS, &arg);
>  }
>  #endif /* ! _REENT_ONLY */

Likewise, 'result' is no longer needed.

> Index: my-base-quilt/newlib/libc/machine/spu/fgets.c
> ===================================================================
> --- my-base-quilt.orig/newlib/libc/machine/spu/fgets.c
> +++ my-base-quilt/newlib/libc/machine/spu/fgets.c
> @@ -61,8 +61,6 @@ _DEFUN (fgets, (buf, n, fp),
>    args.fp = fp->_fp;
>    ret = (char**) &args;
>  
> -  __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FGETS, &args);
> -
> -  return *ret;
> +  return (char*) __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FGETS, &args);
>  }
>  #endif /* ! _REENT_ONLY */

Likewise, 'ret' is no longer needed.

> Index: my-base-quilt/newlib/libc/machine/spu/fputc.c
> Index: my-base-quilt/newlib/libc/machine/spu/fputs.c
> Index: my-base-quilt/newlib/libc/machine/spu/fread.c
> Index: my-base-quilt/newlib/libc/machine/spu/fseek.c
> Index: my-base-quilt/newlib/libc/machine/spu/fsetpos.c
> Index: my-base-quilt/newlib/libc/machine/spu/fwrite.c
> Index: my-base-quilt/newlib/libc/machine/spu/putc.c
> Index: my-base-quilt/newlib/libc/machine/spu/rename.c
> Index: my-base-quilt/newlib/libc/machine/spu/setvbuf.c
> Index: my-base-quilt/newlib/libc/machine/spu/tmpnam.c
> Index: my-base-quilt/newlib/libc/machine/spu/ungetc.c
> Index: my-base-quilt/newlib/libc/machine/spu/vfprintf.c
> Index: my-base-quilt/newlib/libc/machine/spu/vfscanf.c
> Index: my-base-quilt/newlib/libc/machine/spu/vprintf.c
> Index: my-base-quilt/newlib/libc/machine/spu/vscanf.c
> Index: my-base-quilt/newlib/libc/machine/spu/vsnprintf.c
> Index: my-base-quilt/newlib/libc/machine/spu/vsprintf.c
> Index: my-base-quilt/newlib/libc/machine/spu/vsscanf.c

Ditto.

> Index: my-base-quilt/newlib/libc/machine/spu/fopen.c
> ===================================================================
> --- my-base-quilt.orig/newlib/libc/machine/spu/fopen.c
> +++ my-base-quilt/newlib/libc/machine/spu/fopen.c
> @@ -65,7 +65,7 @@ _DEFUN (fopen, (file, mode),
>    args.mode = mode;
>    ret = (int *) &args;
>  
> -  __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FOPEN, &args);
> +  *ret = __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FOPEN, &args);
>  
>    if (*ret) {
>      fp->_fp = *ret;

With your changes, it seems reasonable (and may be a little more
efficient) to use an integer variable rather than a pointer for 'ret',
for example:

Index: newlib-200705220000/newlib/libc/machine/spu/fopen.c
===================================================================
--- newlib-200705220000.orig/newlib/libc/machine/spu/fopen.c
+++ newlib-200705220000/newlib/libc/machine/spu/fopen.c
@@ -49,7 +49,7 @@ _DEFUN (fopen, (file, mode),
 	_CONST char *file _AND
 	_CONST char *mode)
 {
-  int *ret;
+  int ret;
   c99_fopen_t args;
   FILE *fp;
   struct _reent *ptr = _REENT;
@@ -63,12 +63,11 @@ _DEFUN (fopen, (file, mode),
 
   args.file = file;
   args.mode = mode;
-  ret = (int *) &args;
 
-  *ret = __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FOPEN, &args);
+  ret = __send_to_ppe(SPE_C99_SIGNALCODE, SPE_C99_FOPEN, &args);
 
-  if (*ret) {
-    fp->_fp = *ret;
+  if (ret) {
+    fp->_fp = ret;
     return fp;
   }
   else {

> Index: my-base-quilt/newlib/libc/machine/spu/fprintf.c
> Index: my-base-quilt/newlib/libc/machine/spu/freopen.c
> Index: my-base-quilt/newlib/libc/machine/spu/fscanf.c
> Index: my-base-quilt/newlib/libc/machine/spu/printf.c
> Index: my-base-quilt/newlib/libc/machine/spu/scanf.c
> Index: my-base-quilt/newlib/libc/machine/spu/snprintf.c
> Index: my-base-quilt/newlib/libc/machine/spu/sprintf.c
> Index: my-base-quilt/newlib/libc/machine/spu/sscanf.c

Ditto.

--
(ASAYAMA Kazunori
  (asayama@sm.sony.co.jp))
t