Re: [PATCH] SPU use a non-functional errno

[Jeff Johnston <jjohnstn at redhat dot com>]

Patrick Mansfield wrote:
> Hi Jeff -
>
> Not sure of the best way to handle this, please comment or apply, thanks!

If you want to override sys/errno.h, that's fine, but if you want 
impure.c changes, you'll have to override that file in libc/machine/spu 
as well.

> Modify SPU to directly use errno, since it does not need reentrant code.
> With this patch, any code using errno is 8 bytes smaller, plus there is no
> function call.
>
> More importantly, testing of the SPU optimized math function asin showed a
> decrease in time of 16%, a simple test took 16.6 seconds, where without
> the change it took 19.7 seconds. Similar gains are likely for other math
> domain checks in the SPU optimized math code (they are already set up for
> branchless compare and setting of errno, but the code has to always read
> and set errno).

You should only be touching errno on an error.  Normally, you have a 
local variable to store results and then check for failure.  If failure 
occurs, you set errno appropriately.  Thus, you only slow down on 
failure which is more than reasonable and if you get your local variable 
into a register(s), you can do very efficient checking/branching for the 
non-error case.

-- Jeff J.

> Alternatively, we could use:
>
> 	extern int errno;
>
> That means we would have another 4 bytes (maybe 8 for alignment) for the
> errno, and would need our own errno.c (keep __errno() for backwards
> compatibility), but would otherwise be ok.
>
> Or use:
>
> 	#define errno (_REENT->_errno)
>
> Means an extra level of indirection, so an extra instruction everywhere,
> still much better than a function call on SPU, but not ideal.
>
> 2008-03-27 Patrick Mansfield <patmans@us.ibm.com>
>
> 	* libc/machine/spu/sys/errno.h: Define errno to  _reent_data._errno.
> 	* libc/reent/impure.c: Add SPU extern _reent_data.
> 	* spu/sbrk.c: Remove "extern int errno", use whatever is supplied
> 	  by sys/errno.h.
>
> Index: quilt/libgloss/spu/sbrk.c
> ===================================================================
> --- quilt.orig/libgloss/spu/sbrk.c
> +++ quilt/libgloss/spu/sbrk.c
> @@ -34,8 +34,6 @@ Author: Andreas Neukoetter (ti95neuk@de.
>  #include <errno.h>
>  #include <spu_intrinsics.h>
>  
> -extern int errno;
> -
>  extern caddr_t  _end;
>  #define STACKSIZE 4096
>  
> Index: quilt/newlib/libc/machine/spu/sys/errno.h
> ===================================================================
> --- quilt.orig/newlib/libc/machine/spu/sys/errno.h
> +++ quilt/newlib/libc/machine/spu/sys/errno.h
> @@ -19,10 +19,6 @@
>     on which it is based, except values used or returned by syscalls must
>     be those of the Linux ppc.  */
>  
> -/* errno is not a global variable, because that would make using it
> -   non-reentrant.  Instead, its address is returned by the function
> -   __errno.  */
> -
>  #ifndef _SYS_ERRNO_H_
>  #ifdef __cplusplus
>  extern "C" {
> @@ -31,10 +27,8 @@ extern "C" {
>  
>  #include <sys/reent.h>
>  
> -#ifndef _REENT_ONLY
> -#define errno (*__errno())
> -extern int *__errno _PARAMS ((void));
> -#endif
> +extern struct _reent _reent_data;
> +#define errno (_reent_data._errno)
>  
>  /* Please don't use these variables directly.
>     Use strerror instead. */
> Index: quilt/newlib/libc/reent/impure.c
> ===================================================================
> --- quilt.orig/newlib/libc/reent/impure.c
> +++ quilt/newlib/libc/reent/impure.c
> @@ -10,7 +10,9 @@
>  #endif
>  
>  static struct _reent __ATTRIBUTE_IMPURE_DATA__ impure_data = _REENT_INIT (impure_data);
> -#ifdef __CYGWIN__
> +#if defined(__SPU__)
> +extern struct _reent _reent_data __attribute__ ((alias("impure_data")));
> +#elif defined(__CYGWIN__)
>  extern struct _reent reent_data __attribute__ ((alias("impure_data")));
>  #endif
>  struct _reent *__ATTRIBUTE_IMPURE_PTR__ _impure_ptr = &impure_data;