Modernize i386 string.h
PATCH 1/1 Modernize i386 string.h
This patch modernizes i386 string.h. It removes all the fragile i386 inline str* functions and just switches to gcc's builtin variants. Modern gcc should generate equivalent or better code. Sometimes it calls out-of-line code, in that case the standard C functions in lib/string.c is used.
Essentially it's porting the x86-64 string.h to i386.
memcpy and memset use the x86 string functions. They are always out-of-line now. These seem to do quite well on many modern x86 (C stepping K8, P3, P4). On some CPUs (Athlon XP) it may be better to use special unrolled versions for clear_page/copy_page, that is subject to further work. I removed the call to MMX memcpy from the main memcpy because MMX only helps for very big copies and normal memcpy is rarely used for that. Longer term it would
be useful to implement optimized clear_page/copy_page functions and switch to the correct one for the current CPU using the alternative() framework.
I also removed struct_cpy and just let the compiler expand it.
[see the big comment in arch/i386/lib/memcpy.c for more details and some considerations]
-Andi
arch/i386/lib/memmove.c new file +31 -0
@@ -0,0 +1,31 @@ +#include <linux/module.h> +#include <linux/string.h> + +/* Normally compiler builtins are used, but sometimes the compiler calls out +of line code. Based on asm-i386/string.h. + +memmove is not that time critical, so this simple version should work. +*/ + +#undef memmove +void *memmove (void * dest,const void *src,size_t count) +{ +if (dest < src) { +__inline_memcpy (dest,src,count); + }else { +/* Could be more clever and move longs */ +unsigned long d0, d1, d2; + __asm__ __volatile__( +"std\n\t" +"rep\n\t" +"movsb\n\t" +"cld" + :"=&c" (d0),"=&S" (d1),"=&D" (d2) + :"0" (count), +"1" (count-1 +(const char *)src), +"2" (count-1 +(char *)dest) + :"memory" ); + } +return dest; +} +EXPORT_SYMBOL_NOVERS (memmove);
arch/i386/lib/memcpy.c renamed from arch/i386/lib/memcpy.c-STRING +47 -12
66 lines not on this page · raw message
arch/i386/lib/memset.c new file +27 -0
@@ -0,0 +1,27 @@ +#include <linux/module.h> +#include <linux/string.h> + +/* Please see the introductionary comment in memcpy.c. It applies to +memset too. */ + +#undef memset +void *memset (void * s,int c,size_t count) +{ +unsigned fill = (unsigned )c *0x01010101 ; +int d0, d1; + asmvolatile ( +"rep ; stosl\n\t" +"testl $2,%3\n\t" +"je 1f\n\t" +"stosw\n" +"1:\ttestl $1,%3\n\t" +"je 2f\n\t" +"stosb\n" +"2:" + :"=&c" (d0),"=&D" (d1) + :"a" (fill),"q" (count),"0" (count/4 ),"1" ((long ) s) + :"memory" ); +return (s); +} + +EXPORT_SYMBOL_NOVERS (memset);
arch/i386/lib/strstr.c renamed from arch/i386/lib/strstr.c-STRING +0 -31
@@ -1,31 +0,0 @@ -#include <linux/string.h> - -char *strstr (const char * cs,const char * ct) -{ -int d0, d1; -registerchar * __res; -__asm__ __volatile__( -"movl %6,%%edi\n\t" -"repne\n\t" -"scasb\n\t" -"notl %%ecx\n\t" -"decl %%ecx\n\t" /* NOTE! This also sets Z if searchstring='' */ -"movl %%ecx,%%edx\n" -"1:\tmovl %6,%%edi\n\t" -"movl %%esi,%%eax\n\t" -"movl %%edx,%%ecx\n\t" -"repe\n\t" -"cmpsb\n\t" -"je 2f\n\t" /* also works for empty string, see above */ -"xchgl %%eax,%%esi\n\t" -"incl %%esi\n\t" -"cmpb $0,-1(%%eax)\n\t" -"jne 1b\n\t" -"xorl %%eax,%%eax\n\t" -"2:" - :"=a" (__res),"=&c" (d0),"=&S" (d1) - :"0" (0 ),"1" (0xffffffff ),"2" (cs),"g" (ct) - :"dx" ,"di" ); -return __res; -} -
arch/i386/lib/Makefile renamed from arch/i386/lib/Makefile-STRING +1 -1
@@ -5,7 +5,7 @@ lib-y = checksum.o delay.o \ usercopy.o getuser.o \ - memcpy.o strstr.o + memcpy.o memmove.o memset.o lib-$(CONFIG_X86_USE_3DNOW) += mmx.o lib-$(CONFIG_HAVE_DEC_LOCK) += dec_and_lock.o
arch/i386/kernel/i386_ksyms.c renamed from arch/i386/kernel/i386_ksyms.c-STRING +32 -7
@@ -133,6 +133,38 @@EXPORT_SYMBOL (pcibios_get_irq_routing_table);#endif +/* Export string functions. We normally rely on gcc builtin for most of these, +but gcc sometimes decides not to inline them. */ +#undef memchr +#undef strlen +#undef strcpy +#undef strncmp +#undef strncpy +#undef strchr +#undef strcmp +#undef strcpy +#undef strcat + +extern size_t strlen (const char *); +extern char *strcpy (char * dest,const char *src); +extern int strcmp (const char * cs,const char * ct); +extern void *memchr (const void *s,int c,size_t n); +extern char *strcat (char *,const char *); + +EXPORT_SYMBOL_NOVERS (strlen); +EXPORT_SYMBOL_NOVERS (memmove); +EXPORT_SYMBOL_NOVERS (strcpy); +EXPORT_SYMBOL_NOVERS (strncmp); +EXPORT_SYMBOL_NOVERS (strncpy); +EXPORT_SYMBOL_NOVERS (strchr); +EXPORT_SYMBOL_NOVERS (strcmp); +EXPORT_SYMBOL_NOVERS (strcat); +EXPORT_SYMBOL_NOVERS (strncat); +EXPORT_SYMBOL_NOVERS (memchr); +EXPORT_SYMBOL_NOVERS (strrchr); +EXPORT_SYMBOL_NOVERS (strnlen); +EXPORT_SYMBOL_NOVERS (memscan); +#ifdef CONFIG_X86_USE_3DNOW EXPORT_SYMBOL (_mmx_memcpy);EXPORT_SYMBOL (mmx_clear_page);
@@ -179,13 +211,6 @@EXPORT_SYMBOL_GPL (set_nmi_callback);EXPORT_SYMBOL_GPL (unset_nmi_callback); -#undef memcpy -#undef memset -extern void *memset (void *,int ,__kernel_size_t ); -extern void *memcpy (void *,const void *,__kernel_size_t ); -EXPORT_SYMBOL_NOVERS (memcpy); -EXPORT_SYMBOL_NOVERS (memset); -#ifdef CONFIG_HAVE_DEC_LOCK EXPORT_SYMBOL (atomic_dec_and_lock);#endif
arch/i386/kernel/process.c renamed from arch/i386/kernel/process.c-STRING +1 -1
@@ -344,7 +344,7 @@int err; childregs = ((struct pt_regs *) (THREAD_SIZE + (unsigned long ) p->thread_info)) -1 ; -struct_cpy (childregs, regs); + *childregs = *regs; childregs->eax =0 ; childregs->esp = esp; p->set_child_tid = p->clear_child_tid =NULL ;
include/asm-i386/string.h renamed from include/asm-i386/string.h-STRING +30 -445
498 lines not on this page · raw message
Quoted 5 lines · Andi Kleen
This patch modernizes i386
string.h. It removes all the fragile i386 inline str* functions and just switches to gcc's builtin variants. Modern gcc should generate equivalent or better code. Sometimes it calls out-of-line code, in that case the standard C functions inlib/string.cis used.
[...]
Quoted 14 lines
diff -u linux-string/arch/i386/kernel/i386_ksyms.c-STRING
linux-string/arch/i386/kernel/i386_ksyms.carch/i386/kernel/i386_ksyms.c renamed from arch/i386/kernel/i386_ksyms.c-STRING +7
-0@@ -133,6 +133,38 @@EXPORT_SYMBOL (pcibios_get_irq_routing_table);#endif +/* Export string functions. We normally rely on gcc builtin for most of these, +but gcc sometimes decides not to inline them. */ +#undef memchr +#undef strlen +#undef strcpy +#undef strncmp +#undef strncpy
arch/i386/kernel/i386_ksyms.c line 152
Quoted 10 lines
arch/i386/kernel/i386_ksyms.c renamed from arch/i386/kernel/i386_ksyms.c-STRING +10
-0+#undef strchr +#undef strcmp +#undef strcpy +#undef strcat + +extern size_t strlen (const char *); +extern char *strcpy (char * dest,const char *src); +extern int strcmp (const char * cs,const char * ct); +extern void *memchr (const void *s,int c,size_t n); +extern char *strcat (char *,const char *);
ISTR this patch being shot down in the past...
It seems suboptimal for people with ancient compilers, or for people on embedded 486's, doesn't it?
Jeff
Quoted 34 lines · Jeff Garzik
Quoted 5 lines · Andi Kleen
This patch modernizes i386
string.h. It removes all the fragile i386 inline str* functions and just switches to gcc's builtin variants. Modern gcc should generate equivalent or better code. Sometimes it calls out-of-line code, in that case the standard C functions inlib/string.cis used.[...]
Quoted 14 lines
diff -u linux-string/arch/i386/kernel/i386_ksyms.c-STRING
linux-string/arch/i386/kernel/i386_ksyms.carch/i386/kernel/i386_ksyms.c renamed from arch/i386/kernel/i386_ksyms.c-STRING +7
-0@@ -133,6 +133,38 @@EXPORT_SYMBOL (pcibios_get_irq_routing_table);#endif +/* Export string functions. We normally rely on gcc builtin for most of these, +but gcc sometimes decides not to inline them. */ +#undef memchr +#undef strlen +#undef strcpy +#undef strncmp +#undef strncpyarch/i386/kernel/i386_ksyms.c line 152
Quoted 10 lines
arch/i386/kernel/i386_ksyms.c renamed from arch/i386/kernel/i386_ksyms.c-STRING +10
-0+#undef strchr +#undef strcmp +#undef strcpy +#undef strcat + +extern size_t strlen (const char *); +extern char *strcpy (char * dest,const char *src); +extern int strcmp (const char * cs,const char * ct); +extern void *memchr (const void *s,int c,size_t n); +extern char *strcat (char *,const char *);ISTR this patch being shot down in the past...
Quoted 5 lines
It seems suboptimal for people with ancient compilers, or for people on embedded 486's, doesn't it?
Jeff
I don't think so. It helps reduce the size of the code by having only one copy of each routine in memory rather than zillions.
If your code can't stand the overhead of the call/return, then you put some inline code at that particular place, not everywhere!
Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
Note 96.31% of all statistics are fiction.
Quoted 1 line
ISTR this patch being shot down in the past...
I don't remember that.
Quoted 3 lines
It seems suboptimal for people with ancient compilers, or for people on embedded 486's, doesn't it?
It is unlikely to make much difference for them. Kernel str* handling is not very time critical (except memset/memcpy, but they are handled efficiently on all compilers) The patch is mainly to make the code more maintainable.
-Andi
include/asm-i386/string.h line 37
Quoted 10 lines · Andi Kleen
include/asm-i386/string.h renamed from include/asm-i386/string.h-STRING +10
-0+#define __HAVE_ARCH_MEMCPY 1 +extern void *__memcpy (void *to,const void *from,size_t len); +#define memcpy (dst,src,len) \ + ({ size_t __len = (len); \ + void *__ret; \ + if (__builtin_constant_p(len) && __len >= 128) \ + __ret = __memcpy((dst),(src),__len); \ + else \ + __ret = __builtin_memcpy((dst),(src),__len); \ + __ret; })
Why not just __builtin_memcpy? Or indeed, why bother defining anything at all, since the compiler will infer __builtin_memcpy from the external symbol memcpy.
r~
Quoted 15 lines · Richard Henderson
include/asm-i386/string.h line 37
Quoted 10 lines · Andi Kleen
include/asm-i386/string.h renamed from include/asm-i386/string.h-STRING +10
-0+#define __HAVE_ARCH_MEMCPY 1 +extern void *__memcpy (void *to,const void *from,size_t len); +#define memcpy (dst,src,len) \ + ({ size_t __len = (len); \ + void *__ret; \ + if (__builtin_constant_p(len) && __len >= 128) \ + __ret = __memcpy((dst),(src),__len); \ + else \ + __ret = __builtin_memcpy((dst),(src),__len); \ + __ret; })Why not just
__builtin_memcpy? Or indeed, why bother defining anything at all, since the compiler will infer__builtin_memcpyfrom the external symbol memcpy.
I was considering that when I moved the code over from x86-64.
On x86-64 (in gcc 3.1 timeframe) I did it originally this way because memcpy didn't have this "use external for big copies" logic. I wasn't sure if gcc 3.3 handles it now correctly. If you can confirm that it does I will happily remove it.
-Andi
In message <20040118200919.GA26573@averell> you write:
Quoted 1 line
+EXPORT_SYMBOL_NOVERS_NOVERS(memmove);
FYI: this has no meaningful semantic in 2.6 (we don't mangle symbols, but include the version in a separate section anyway).
Please use EXPORT_SYMBOL() everywhere in new code.
Thanks!
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.