This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
Re: [PATCH RFC] spu return rc value from __send_to_ppe
- From: Kazunori Asayama <asayama at sm dot sony dot co dot jp>
- To: patmans at us dot ibm dot com
- Cc: newlib at sourceware dot org, Ulrich dot Weigand at de dot ibm dot com, vzbarsky at us dot ibm dot com
- Date: Tue, 22 May 2007 18:06:39 +0900 (LDT)
- Subject: Re: [PATCH RFC] spu return rc value from __send_to_ppe
- References: <20070521181533.GA19954@us.ibm.com>
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