Subject: Re: Performance of various memcpy()'s
To: Jason R Thorpe <thorpej@wasabisystems.com>
From: Bang Jun-Young <junyoung@mogua.com>
List: port-i386
Date: 10/28/2002 16:41:55
--9amGYk9869ThD9tj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Oct 22, 2002 at 10:43:58PM -0700, Jason R Thorpe wrote:
> On Wed, Oct 23, 2002 at 11:37:45AM +0900, Bang Jun-Young wrote:
> 
>  > BTW, I noticed that our i386 memcpy() in libc checks for overlapping,
>  > although the manpage says "to copy byte strings that overlap, use
>  > memmove(3)."
> 
> Yes, I would strongly support making memcpy() forward-only.

Here is a patch to separate memcpy() from bcopy.S for the purpose of
avoiding overlap check on memcpy(). There are already too many #if's 
in bcopy.S, adding one would be make not-easy-to-read source even
harder.

Along with this change, I'd like to get rid of _DIAGNOSTIC stuff as well.
I don't understand why there's such a pointer wraparound check, since
the memcpy(3) clearily says "use memmove() for overlap case." It might
be worth adding "if you're not sure, always use memmove()." comment
to memcpy(3).

Jun-Young

-- 
Bang Jun-Young <junyoung@mogua.com>

--9amGYk9869ThD9tj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="bcopy.diff"

? bcopy.diff
Index: bcopy.S
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/arch/i386/string/bcopy.S,v
retrieving revision 1.8
diff -u -r1.8 bcopy.S
--- bcopy.S	2002/07/10 06:01:51	1.8
+++ bcopy.S	2002/10/28 07:24:46
@@ -44,18 +44,14 @@
 	 *  ws@tools.de     (Wolfgang Solfrank, TooLs GmbH) +49-228-985800
 	 */
 
-#ifdef MEMCOPY
-ENTRY(memcpy)
-#else
 #ifdef MEMMOVE
 ENTRY(memmove)
 #else
 ENTRY(bcopy)
 #endif
-#endif
 	pushl	%esi
 	pushl	%edi
-#if defined(MEMCOPY) || defined(MEMMOVE)
+#if defined(MEMMOVE)
 	movl	12(%esp),%edi
 	movl	16(%esp),%esi
 	movl	%edi,%eax	/* return value */
@@ -86,7 +82,7 @@
 
 #ifdef _DIAGNOSTIC
 				/* check pointer wraparound */
-#if defined(MEMCOPY) || defined(MEMMOVE)
+#if defined(MEMMOVE)
 	cmpl	12(%esp),%edi
 #else
 	cmpl	16(%esp),%edi
@@ -97,7 +93,7 @@
 	pushl	$__LINE__-4
 	jmp	4f
 2:	
-#if defined(MEMCOPY) || defined(MEMMOVE)
+#if defined(MEMMOVE)
 	cmpl	16(%esp),%esi
 #else
 	cmpl	12(%esp),%esi
@@ -110,7 +106,7 @@
 	pushl	$file
 	call	_C_LABEL(__diagassert13)
 	addl	$16,%esp
-#if defined(MEMCOPY) || defined(MEMMOVE)
+#if defined(MEMMOVE)
 	movl	12(%esp),%eax
 #endif
 	popl	%edi
@@ -140,9 +136,7 @@
 file:
 	.asciz	__FILE__
 func:
-#if defined(MEMCOPY)
-	.asciz	"memcpy"
-#elseif defined(MEMMOVE)
+#if defined(MEMMOVE)
 	.asciz	"memmove"
 #else
 	.asciz	"bcopy"
Index: memcpy.S
===================================================================
RCS file: /cvsroot/basesrc/lib/libc/arch/i386/string/memcpy.S,v
retrieving revision 1.2
diff -u -r1.2 memcpy.S
--- memcpy.S	1998/01/09 03:45:07	1.2
+++ memcpy.S	2002/10/28 07:24:46
@@ -1,4 +1,64 @@
 /*	$NetBSD: memcpy.S,v 1.2 1998/01/09 03:45:07 perry Exp $	*/
 
-#define MEMCOPY
-#include "bcopy.S"
+/*-
+ * Copyright (c) 1990 The Regents of the University of California.
+ * All rights reserved.
+ *
+ * This code is derived from locore.s.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. All advertising materials mentioning features or use of this software
+ *    must display the following acknowledgement:
+ *	This product includes software developed by the University of
+ *	California, Berkeley and its contributors.
+ * 4. Neither the name of the University nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <machine/asm.h>
+
+#if defined(LIBC_SCCS)
+	RCSID("$NetBSD$")
+#endif
+
+ENTRY(memcpy)
+	pushl	%esi
+	pushl	%edi
+
+	movl	12(%esp),%edi
+	movl	16(%esp),%esi
+	movl	20(%esp),%ecx
+	movl	%edi,%eax	/* return value */
+
+	movl	%ecx,%edx
+	cld			/* nope, copy forwards. */
+	shrl	$2,%ecx		/* copy by words */
+	rep
+	movsl
+	movl	%edx,%ecx
+	andl	$3,%ecx		/* any bytes left? */
+	rep
+	movsb
+	popl	%edi
+	popl	%esi
+	ret

--9amGYk9869ThD9tj--