Re: [BK+PATCH] remove __constant_memcpy

[Jeff Garzik <jgarzik@pobox.com>]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]

Linus Torvalds wrote:
> On Thu, 17 Apr 2003, Jeff Garzik wrote:
> 
>>__constant_memcpy was used for small, constant-sized cases AFTER
>>the kernel made the decision not to hand the copy duties over to the
>>kernel's MMX/SSE code.  Take a look at the bottom of the patch below,
>>and also this snip from a non-hacked string.h, for illustration...
> 
> 
> This is the part I don't like
> 
>  #define memcpy(t, f, n) \
>  (__builtin_constant_p(n) ? \
> - __constant_memcpy((t),(f),(n)) : \
> + __builtin_memcpy((t),(f),(n)) : \
>   __memcpy((t),(f),(n)))
> 
> Notice? Our old __constant_memcpy() would do the rigth thing for large 
> copies. In conrast, I don't know that gcc will do so.


If DTRT means just using the existing code for large copies in general, 
that's easy enough...  patch attached.  I made the definition of "large 
copy" more pessimistic, where <= 128 bytes goes to __builtin_memcpy, 
otherwise to __constant_memcpy.  I carried this over to the MMX side too 
by proxy, as the existing MMX code already called __constant_memcpy. 
Thus, the modification is now only in one place.

If DTRT means not using MMX/SSE[2], that's a given considering the above 
code is from the "we don't have MMX/SSE" part of the ifdef.  If gcc 
starts using MMX with -march=i586 it's time for us all to go home and 
write a new compiler ;-)

Three tangents:
1) where did the 486 string ops go?
2) why no sse2-optimized memcpy?  just that noone has done one yet?
3) for copies >512 bytes, should we simply call the un-inlined memcpy? 
I would think that the call would get lost in cache misses of the block 
copy itself, but then again, __constant_memcpy (as it appears in output 
asm) is pretty darn small already.

	Jeff



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2015 bytes --]

===== include/asm-i386/string.h 1.8 vs edited =====
--- 1.8/include/asm-i386/string.h	Mon Mar 31 17:29:08 2003
+++ edited/include/asm-i386/string.h	Thu Apr 17 19:45:20 2003
@@ -214,49 +214,9 @@
  */
 static inline void * __constant_memcpy(void * to, const void * from, size_t n)
 {
-	switch (n) {
-		case 0:
-			return to;
-		case 1:
-			*(unsigned char *)to = *(const unsigned char *)from;
-			return to;
-		case 2:
-			*(unsigned short *)to = *(const unsigned short *)from;
-			return to;
-		case 3:
-			*(unsigned short *)to = *(const unsigned short *)from;
-			*(2+(unsigned char *)to) = *(2+(const unsigned char *)from);
-			return to;
-		case 4:
-			*(unsigned long *)to = *(const unsigned long *)from;
-			return to;
-		case 6:	/* for Ethernet addresses */
-			*(unsigned long *)to = *(const unsigned long *)from;
-			*(2+(unsigned short *)to) = *(2+(const unsigned short *)from);
-			return to;
-		case 8:
-			*(unsigned long *)to = *(const unsigned long *)from;
-			*(1+(unsigned long *)to) = *(1+(const unsigned long *)from);
-			return to;
-		case 12:
-			*(unsigned long *)to = *(const unsigned long *)from;
-			*(1+(unsigned long *)to) = *(1+(const unsigned long *)from);
-			*(2+(unsigned long *)to) = *(2+(const unsigned long *)from);
-			return to;
-		case 16:
-			*(unsigned long *)to = *(const unsigned long *)from;
-			*(1+(unsigned long *)to) = *(1+(const unsigned long *)from);
-			*(2+(unsigned long *)to) = *(2+(const unsigned long *)from);
-			*(3+(unsigned long *)to) = *(3+(const unsigned long *)from);
-			return to;
-		case 20:
-			*(unsigned long *)to = *(const unsigned long *)from;
-			*(1+(unsigned long *)to) = *(1+(const unsigned long *)from);
-			*(2+(unsigned long *)to) = *(2+(const unsigned long *)from);
-			*(3+(unsigned long *)to) = *(3+(const unsigned long *)from);
-			*(4+(unsigned long *)to) = *(4+(const unsigned long *)from);
-			return to;
-	}
+	if (n <= 128)
+		return __builtin_memcpy(to, from, n);
+
 #define COMMON(x) \
 __asm__ __volatile__( \
 	"rep ; movsl" \