David, I added a performance patch, for the sh_isoption(shp,x),
sh_onoption(shp,x) and sh_offoption(shp,x) family of functions, to
this mail. Jennifer Pioch and I spend 2 hours today, on irc to work
the details out, and see how both Sun Studio and gcc 4.7 behave.

Result is, that the code is now 2 times as fast on Solaris with Sun
Studio, with some code savings, on Sun Studio 12.1 2480bytes are saved
with this patch.

- sh_onoption(shp,x) and sh_offoption(shp,x) no longer return a value.
We found, that calculating the value as a function call is expensive,
almost 1/2 of the total code walked. Even as macro, if the compiler
can not reduce the expression, because it is too complex, a lot of
code is produced, and walked, for no good reason. Therefore,
sh_onoption(shp,x) and sh_offoption(shp,x), no longer calculate a
return value, and the macros cast explicitly to (void) to prevent the
temptation to do so, and tell the compiler to stop there.
The code spend on sh_onoption(shp,x) and sh_offoption(shp,x) macros is
now reduced to nearly 1/2 of the original size, measured in lines of
assembler code (this has a linear correlation on RISC machines like
ARM (not ARM Thumb!) and SPARC64, but not necessarily a linear one on
CISC, like AMD64).
- sh_isoption(shp,x) now returns a C99 bool, which is true or false.
Previously, it returned an unsigned long, which was calculated with
great expense, but never used as unsigned long value - all consumers
just did a reduction to a Boolean value.
Reduction to just Boolean value makes the code much shorter and causes
less strain on the compiler's optimizer.
- Shopt_t now uses an array of C99 uint_fast64_t, if available. Jenny
and I reasoned that this gives a compiler a better hint what to do
with the data.
- Shopt_t data are now unsigned, and all shift constants are now
suffixed with UL and not L. The sign in L was confusing at least gcc,
which thought the sign bit may have extra use. Fixing this to unsigned
enabled the compiler to understand the use of the macro and do proper
bit shift optimizations.
- We tried the use of unsigned __int128, as supported by gcc. It
turned out to be much slower than using unsigned int, unsigned long,
unsigned long long and uint_fast64_t, for unknown reasons. Jenny says
the AMD64 assembler looks horrible, like some __int128 emulation code
which was then passed to the gcc optimizer, which was not able to
understand it, because the depth of optimizations is a few tree steps
too shallow. For now, this has no merit.

      ,   _                                    _   ,
     { \/`o;====-    Olga Kryzhanovska   -====;o`\/ }
.----'-/`-/     olga.kryzhanov...@gmail.com   \-`\-'----.
 `'-..-| /       http://twitter.com/fleyta     \ |-..-'`
      /\/\     Solaris/BSD//C/C++ programmer   /\/\
      `--`                                      `--`
diff -r -u src/cmd/ksh93/bltins/test.c src/cmd/ksh93/bltins/test.c
--- src/cmd/ksh93/bltins/test.c	2012-08-21 22:13:16.000000000 +0200
+++ src/cmd/ksh93/bltins/test.c	2012-09-22 08:07:28.305258568 +0200
@@ -418,7 +418,7 @@
 		op = sh_lookopt(arg,&f);
-		return(op && (f==(sh_isoption(shp,op)!=0)));
+		return(op && (f==(sh_isoption(shp,op)!=false)));
 	    case 't':
 		char *last;
diff -r -u src/cmd/ksh93/include/defs.h src/cmd/ksh93/include/defs.h
--- src/cmd/ksh93/include/defs.h	2012-09-05 22:52:02.000000000 +0200
+++ src/cmd/ksh93/include/defs.h	2012-09-22 08:30:08.856597086 +0200
@@ -463,18 +463,18 @@
 #define sh_translate(s)	_sh_translate(ERROR_dictionary(s))
-#define WBITS		(sizeof(long)*8)
+#define WBITS		(sizeof(Shopt_t_data_t)*8)
 #define WMASK		(0xff)
-#define is_option(s,x)	((s)->v[((x)&WMASK)/WBITS] & (1L << ((x) % WBITS)))
-#define on_option(s,x)	((s)->v[((x)&WMASK)/WBITS] |= (1L << ((x) % WBITS)))
-#define off_option(s,x)	((s)->v[((x)&WMASK)/WBITS] &= ~(1L << ((x) % WBITS)))
+#define is_option(s,x)	((bool)(((s)->v[((x)&WMASK)/WBITS] &  (1UL << ((x) % WBITS)))?true:false))
+#define on_option(s,x)	((void)((s)->v[((x)&WMASK)/WBITS] |=  (1UL << ((x) % WBITS))))
+#define off_option(s,x)	((void)((s)->v[((x)&WMASK)/WBITS] &= ~(1UL << ((x) % WBITS))))
 #undef sh_isoption
 #undef sh_onoption
 #undef sh_offoption
-#define sh_isoption(shp,x)	is_option(&(shp)->options,x)
-#define sh_onoption(shp,x)	on_option(&(shp)->options,x)
-#define sh_offoption(shp,x)	off_option(&(shp)->options,x)
+#define sh_isoption(shp,x)	is_option(&(shp)->options,(x))
+#define sh_onoption(shp,x)	on_option(&(shp)->options,(x))
+#define sh_offoption(shp,x)	off_option(&(shp)->options,(x))
 #define sh_state(x)	( 1<<(x))
diff -r -u src/cmd/ksh93/include/shell.h src/cmd/ksh93/include/shell.h
--- src/cmd/ksh93/include/shell.h	2012-08-24 17:04:20.000000000 +0200
+++ src/cmd/ksh93/include/shell.h	2012-09-22 09:31:33.001079518 +0200
@@ -37,14 +37,23 @@
 #   include	<nval.h>
 #endif /* _SH_PRIVATE */
+#if __STDC_VERSION__ >= 199901L
+#	include	<stdint.h>
 #undef NOT_USED
-#define NOT_USED(x)	(&x,1)
+#define NOT_USED(x)	(&(x),1)
 /* options */
+#if __STDC_VERSION__ >= 199901L
+typedef uint_fast64_t Shopt_t_data_t;
+typedef unsigned int Shopt_t_data_t;
 typedef struct
-	unsigned long v[4];
+	/* Room for 256 flags */
+	Shopt_t_data_t v[(256/8)/sizeof(Shopt_t_data_t)];
@@ -197,9 +206,9 @@
 extern Sfio_t		*sh_iogetiop(int,int);
 extern int		sh_main(int, char*[], Shinit_f);
 extern void		sh_menu(Sfio_t*, int, char*[]);
-extern unsigned long	sh_offoption(int);
-extern unsigned long	sh_isoption(int);
-extern unsigned long	sh_onoption(int);
+extern void		sh_offoption(int);
+extern bool		sh_isoption(int);
+extern void		sh_onoption(int);
 extern int		sh_open(const char*, int, ...);
 extern int		sh_openmax(void);
 extern void		*sh_parse(Shell_t*, Sfio_t*,int);
@@ -228,9 +237,9 @@
 extern int 		sh_fun_20120720(Shell_t*,Namval_t*,Namval_t*, char*[]);
 extern int 		sh_funscope_20120720(Shell_t*,int,char*[],int(*)(void*),void*,int);
 extern Shscope_t	*sh_getscope_20120720(Shell_t*,int,int);
-extern unsigned long	sh_offoption_20120720(Shell_t*,int);
-extern unsigned long	sh_isoption_20120720(Shell_t*,int);
-extern unsigned long	sh_onoption_20120720(Shell_t*,int);
+extern void		sh_offoption_20120720(Shell_t*,int);
+extern bool		sh_isoption_20120720(Shell_t*,int);
+extern void		sh_onoption_20120720(Shell_t*,int);
 extern void		sh_menu_20120720(Shell_t *,Sfio_t*, int, char*[]);
 extern Sfio_t		*sh_pathopen_20120720(Shell_t*,const char*);
 extern int		sh_reinit_20120720(Shell_t*,char*[]);
diff -r -u src/cmd/ksh93/sh/init.c src/cmd/ksh93/sh/init.c
--- src/cmd/ksh93/sh/init.c	2012-09-08 02:01:06.000000000 +0200
+++ src/cmd/ksh93/sh/init.c	2012-09-22 07:07:24.869566305 +0200
@@ -2280,34 +2280,34 @@
 #define DISABLE	/* proto workaround */
-unsigned long sh_isoption_20120720 DISABLE (Shell_t *shp,int opt)
+bool sh_isoption_20120720 DISABLE (Shell_t *shp,int opt)
 #undef sh_isoption
-unsigned long sh_isoption DISABLE (int opt)
+bool sh_isoption DISABLE (int opt)
-unsigned long sh_onoption_20120720 DISABLE (Shell_t *shp,int opt)
+void sh_onoption_20120720 DISABLE (Shell_t *shp,int opt)
-	return(sh_onoption(shp,opt));
+	sh_onoption(shp,opt);
 #undef sh_onoption
-unsigned long sh_onoption DISABLE (int opt)
+void sh_onoption DISABLE (int opt)
-	return(sh_onoption_20120720(sh_getinterp(),opt));
+	sh_onoption_20120720(sh_getinterp(),opt);
-unsigned long sh_offoption_20120720 DISABLE (Shell_t *shp,int opt)
+void sh_offoption_20120720 DISABLE (Shell_t *shp,int opt)
-	return(sh_offoption(shp,opt));
+	sh_offoption(shp,opt);
 #undef sh_offoption
-unsigned long sh_offoption DISABLE (int opt)
+void sh_offoption DISABLE (int opt)
-	return(sh_offoption_20120720(sh_getinterp(),opt));
+	sh_offoption_20120720(sh_getinterp(),opt);
 void	sh_sigcheck DISABLE (Shell_t *shp)
diff -r -u src/cmd/ksh93/sh/main.c src/cmd/ksh93/sh/main.c
--- src/cmd/ksh93/sh/main.c	2012-09-06 23:21:33.000000000 +0200
+++ src/cmd/ksh93/sh/main.c	2012-09-22 07:36:50.692659249 +0200
@@ -125,7 +125,8 @@
 	register Sfio_t  *iop;
 	register Shell_t *shp;
 	struct stat	statb;
-	int i, rshflag;		/* set for restricted shell */
+	int i;
+	bool rshflag;		/* set for restricted shell */
 	char *command;
 #ifdef _lib_sigvec
ast-developers mailing list

Reply via email to