diff mbox

[ARM] Reorganize memcpy selection.

Message ID 563B279F.7060400@arm.com
State Superseded
Headers show

Commit Message

Marcus Shawcroft Nov. 5, 2015, 9:55 a.m. UTC 
Following up on Richard's comments in 
https://sourceware.org/ml/newlib/2015/msg00739.html this is the first of 
a series of patches to clean up the auto configury mechanism used to 
select different implementations of common functions for various 
architecture versions.

There are three functions with special handling in Makefile.am, this 
patch addresses only memcpy, following patches will address memchr and 
strlen in a similar fashion.

The approach here is to remove the selection of memcpy within automake 
and instead use complimentary logic in memcpy-stub.c and memcpy.S to 
choose between the generic memcpy.c implemenation or one of the 
architecture specific memcpy*.S implemenations.

If this patch is taken I'll followup with equivalent rework for memchr 
and strlen.

Regressed for armv7-a armv5 armv8-a, correct selection of memcpy 
implementation by manual inspection of a test program built for these 
three architectures.

OK ?


/Marcus


2015-11-05  Marcus Shawcroft  <marcus.shawcroft@arm.com>

        * libc/machine/arm/Makefile.am: Drop MEMCPY_SRC and MEMCPY_OBJ.
        * libc/machine/arm/Makefile.in: Regenerate.
        * libc/machine/arm/memcpy-stub.c: New.
        * libc/machine/arm/memcpy.c: Adjust copyright year.  Adjust
          comments.
diff mbox

Patch

diff --git a/newlib/ChangeLog b/newlib/ChangeLog
index bbb4683..2842389 100644
--- a/newlib/ChangeLog
+++ b/newlib/ChangeLog
@@ -1,3 +1,10 @@ 
+2015-11-03  Marcus Shawcroft  <marcus.shawcroft@arm.com>
+
+	* libc/machine/arm/Makefile.am: Drop MEMCPY_SRC and MEMCPY_OBJ.
+	* libc/machine/arm/Makefile.in: Regenerate.
+	* libc/machine/arm/memcpy-stub.c: New.
+	* libc/machine/arm/memcpy.c: Adjust copyright year.  Adjust comments.
+
 2015-11-04  Marcus Shawcroft  <marcus.shawcroft@arm.com>
 
 	* libc/machine/arm/configure.in (HAVE_ARMV7): Correct logic.
diff --git a/newlib/libc/machine/arm/Makefile.am b/newlib/libc/machine/arm/Makefile.am
index 0eb1cf1..70afdf1 100644
--- a/newlib/libc/machine/arm/Makefile.am
+++ b/newlib/libc/machine/arm/Makefile.am
@@ -29,39 +29,19 @@  MEMCHR_SRC=
 MEMCHR_OBJ=
 endif
 
-if OPT_SIZE
-MEMCPY_SRC=
-MEMCPY_OBJ=
-else
-if HAVE_ARMV8A
-MEMCPY_SRC=memcpy.S
-MEMCPY_OBJ=$(lpfx)memcpy.o
-else
-if HAVE_ARMV7A
-MEMCPY_SRC=memcpy.S
-MEMCPY_OBJ=$(lpfx)memcpy.o
-else
-if HAVE_ARMV7M
-MEMCPY_SRC=memcpy.S
-MEMCPY_OBJ=$(lpfx)memcpy.o
-else
-MEMCPY_SRC=
-MEMCPY_OBJ=
-endif !HAVE_ARMV7M
-endif !HAVE_ARMV7A
-endif !HAVE_ARMV8A
-endif !OPT_SIZE
-
 lib_a_SOURCES = setjmp.S access.c strcmp.S strcpy.c \
-	        $(MEMCPY_SRC) $(MEMCHR_SRC) $(STRLEN_SRC) \
+		$(MEMCHR_SRC) $(STRLEN_SRC) \
 		strlen-armv7.S aeabi_memcpy.c aeabi_memcpy-armv7a.S \
 		aeabi_memmove.c aeabi_memmove-soft.S \
 		aeabi_memset.c aeabi_memset-soft.S aeabi_memclr.c
+lib_a_SOURCES += memcpy-stub.c
+lib_a_SOURCES += memcpy.S
 
 lib_a_CCASFLAGS=$(AM_CCASFLAGS)
 lib_a_CFLAGS = $(AM_CFLAGS)
-lib_a_LIBADD = $(STRLEN_OBJ) $(MEMCHR_OBJ) $(MEMCPY_OBJ)
-lib_a_DEPENDENCIES = $(STRLEN_OBJ) $(MEMCHR_OBJ) $(MEMCPY_OBJ)
+lib_a_LIBADD = $(STRLEN_OBJ) $(MEMCHR_OBJ)
+
+lib_a_DEPENDENCIES = $(STRLEN_OBJ) $(MEMCHR_OBJ)
 
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
diff --git a/newlib/libc/machine/arm/Makefile.in b/newlib/libc/machine/arm/Makefile.in
index 81dea30..8b69c12 100644
--- a/newlib/libc/machine/arm/Makefile.in
+++ b/newlib/libc/machine/arm/Makefile.in
@@ -71,27 +71,20 @@  lib_a_AR = $(AR) $(ARFLAGS)
 @HAVE_THUMB1_FALSE@am__DEPENDENCIES_1 = $(lpfx)strlen.o
 @HAVE_THUMB1_TRUE@@OPT_SIZE_TRUE@am__DEPENDENCIES_1 = $(lpfx)strlen.o
 @HAVE_ARMV7_TRUE@am__DEPENDENCIES_2 = $(lpfx)memchr.o
-@HAVE_ARMV7A_FALSE@@HAVE_ARMV7M_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@am__DEPENDENCIES_3 = $(lpfx)memcpy.o
-@HAVE_ARMV7A_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@am__DEPENDENCIES_3 = $(lpfx)memcpy.o
-@HAVE_ARMV8A_TRUE@@OPT_SIZE_FALSE@am__DEPENDENCIES_3 =  \
-@HAVE_ARMV8A_TRUE@@OPT_SIZE_FALSE@	$(lpfx)memcpy.o
-@HAVE_ARMV7A_FALSE@@HAVE_ARMV7M_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@am__objects_1 = lib_a-memcpy.$(OBJEXT)
-@HAVE_ARMV7A_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@am__objects_1 = lib_a-memcpy.$(OBJEXT)
-@HAVE_ARMV8A_TRUE@@OPT_SIZE_FALSE@am__objects_1 =  \
-@HAVE_ARMV8A_TRUE@@OPT_SIZE_FALSE@	lib_a-memcpy.$(OBJEXT)
-@HAVE_ARMV7_TRUE@am__objects_2 = lib_a-memchr.$(OBJEXT)
-@HAVE_THUMB1_FALSE@am__objects_3 = lib_a-strlen.$(OBJEXT)
-@HAVE_THUMB1_TRUE@@OPT_SIZE_TRUE@am__objects_3 =  \
+@HAVE_ARMV7_TRUE@am__objects_1 = lib_a-memchr.$(OBJEXT)
+@HAVE_THUMB1_FALSE@am__objects_2 = lib_a-strlen.$(OBJEXT)
+@HAVE_THUMB1_TRUE@@OPT_SIZE_TRUE@am__objects_2 =  \
 @HAVE_THUMB1_TRUE@@OPT_SIZE_TRUE@	lib_a-strlen.$(OBJEXT)
 am_lib_a_OBJECTS = lib_a-setjmp.$(OBJEXT) lib_a-access.$(OBJEXT) \
 	lib_a-strcmp.$(OBJEXT) lib_a-strcpy.$(OBJEXT) $(am__objects_1) \
-	$(am__objects_2) $(am__objects_3) lib_a-strlen-armv7.$(OBJEXT) \
+	$(am__objects_2) lib_a-strlen-armv7.$(OBJEXT) \
 	lib_a-aeabi_memcpy.$(OBJEXT) \
 	lib_a-aeabi_memcpy-armv7a.$(OBJEXT) \
 	lib_a-aeabi_memmove.$(OBJEXT) \
 	lib_a-aeabi_memmove-soft.$(OBJEXT) \
 	lib_a-aeabi_memset.$(OBJEXT) lib_a-aeabi_memset-soft.$(OBJEXT) \
-	lib_a-aeabi_memclr.$(OBJEXT)
+	lib_a-aeabi_memclr.$(OBJEXT) lib_a-memcpy-stub.$(OBJEXT) \
+	lib_a-memcpy.$(OBJEXT)
 lib_a_OBJECTS = $(am_lib_a_OBJECTS)
 DEFAULT_INCLUDES = -I.@am__isrc@
 depcomp =
@@ -228,26 +221,15 @@  noinst_LIBRARIES = lib.a
 @HAVE_ARMV7_TRUE@MEMCHR_SRC = memchr.S
 @HAVE_ARMV7_FALSE@MEMCHR_OBJ = 
 @HAVE_ARMV7_TRUE@MEMCHR_OBJ = $(lpfx)memchr.o
-@HAVE_ARMV7A_FALSE@@HAVE_ARMV7M_FALSE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@MEMCPY_SRC = 
-@HAVE_ARMV7A_FALSE@@HAVE_ARMV7M_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@MEMCPY_SRC = memcpy.S
-@HAVE_ARMV7A_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@MEMCPY_SRC = memcpy.S
-@HAVE_ARMV8A_TRUE@@OPT_SIZE_FALSE@MEMCPY_SRC = memcpy.S
-@OPT_SIZE_TRUE@MEMCPY_SRC = 
-@HAVE_ARMV7A_FALSE@@HAVE_ARMV7M_FALSE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@MEMCPY_OBJ = 
-@HAVE_ARMV7A_FALSE@@HAVE_ARMV7M_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@MEMCPY_OBJ = $(lpfx)memcpy.o
-@HAVE_ARMV7A_TRUE@@HAVE_ARMV8A_FALSE@@OPT_SIZE_FALSE@MEMCPY_OBJ = $(lpfx)memcpy.o
-@HAVE_ARMV8A_TRUE@@OPT_SIZE_FALSE@MEMCPY_OBJ = $(lpfx)memcpy.o
-@OPT_SIZE_TRUE@MEMCPY_OBJ = 
-lib_a_SOURCES = setjmp.S access.c strcmp.S strcpy.c \
-	        $(MEMCPY_SRC) $(MEMCHR_SRC) $(STRLEN_SRC) \
-		strlen-armv7.S aeabi_memcpy.c aeabi_memcpy-armv7a.S \
-		aeabi_memmove.c aeabi_memmove-soft.S \
-		aeabi_memset.c aeabi_memset-soft.S aeabi_memclr.c
-
+lib_a_SOURCES = setjmp.S access.c strcmp.S strcpy.c $(MEMCHR_SRC) \
+	$(STRLEN_SRC) strlen-armv7.S aeabi_memcpy.c \
+	aeabi_memcpy-armv7a.S aeabi_memmove.c aeabi_memmove-soft.S \
+	aeabi_memset.c aeabi_memset-soft.S aeabi_memclr.c \
+	memcpy-stub.c memcpy.S
 lib_a_CCASFLAGS = $(AM_CCASFLAGS)
 lib_a_CFLAGS = $(AM_CFLAGS)
-lib_a_LIBADD = $(STRLEN_OBJ) $(MEMCHR_OBJ) $(MEMCPY_OBJ)
-lib_a_DEPENDENCIES = $(STRLEN_OBJ) $(MEMCHR_OBJ) $(MEMCPY_OBJ)
+lib_a_LIBADD = $(STRLEN_OBJ) $(MEMCHR_OBJ)
+lib_a_DEPENDENCIES = $(STRLEN_OBJ) $(MEMCHR_OBJ)
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
 MEMCPY_DEP = memcpy-armv7a.S memcpy-armv7m.S
@@ -330,12 +312,6 @@  lib_a-strcmp.o: strcmp.S
 lib_a-strcmp.obj: strcmp.S
 	$(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CCASFLAGS) $(CCASFLAGS) -c -o lib_a-strcmp.obj `if test -f 'strcmp.S'; then $(CYGPATH_W) 'strcmp.S'; else $(CYGPATH_W) '$(srcdir)/strcmp.S'; fi`
 
-lib_a-memcpy.o: memcpy.S
-	$(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CCASFLAGS) $(CCASFLAGS) -c -o lib_a-memcpy.o `test -f 'memcpy.S' || echo '$(srcdir)/'`memcpy.S
-
-lib_a-memcpy.obj: memcpy.S
-	$(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CCASFLAGS) $(CCASFLAGS) -c -o lib_a-memcpy.obj `if test -f 'memcpy.S'; then $(CYGPATH_W) 'memcpy.S'; else $(CYGPATH_W) '$(srcdir)/memcpy.S'; fi`
-
 lib_a-memchr.o: memchr.S
 	$(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CCASFLAGS) $(CCASFLAGS) -c -o lib_a-memchr.o `test -f 'memchr.S' || echo '$(srcdir)/'`memchr.S
 
@@ -366,6 +342,12 @@  lib_a-aeabi_memset-soft.o: aeabi_memset-soft.S
 lib_a-aeabi_memset-soft.obj: aeabi_memset-soft.S
 	$(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CCASFLAGS) $(CCASFLAGS) -c -o lib_a-aeabi_memset-soft.obj `if test -f 'aeabi_memset-soft.S'; then $(CYGPATH_W) 'aeabi_memset-soft.S'; else $(CYGPATH_W) '$(srcdir)/aeabi_memset-soft.S'; fi`
 
+lib_a-memcpy.o: memcpy.S
+	$(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CCASFLAGS) $(CCASFLAGS) -c -o lib_a-memcpy.o `test -f 'memcpy.S' || echo '$(srcdir)/'`memcpy.S
+
+lib_a-memcpy.obj: memcpy.S
+	$(CCAS) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CCASFLAGS) $(CCASFLAGS) -c -o lib_a-memcpy.obj `if test -f 'memcpy.S'; then $(CYGPATH_W) 'memcpy.S'; else $(CYGPATH_W) '$(srcdir)/memcpy.S'; fi`
+
 .c.o:
 	$(COMPILE) -c $<
 
@@ -414,6 +396,12 @@  lib_a-aeabi_memclr.o: aeabi_memclr.c
 lib_a-aeabi_memclr.obj: aeabi_memclr.c
 	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-aeabi_memclr.obj `if test -f 'aeabi_memclr.c'; then $(CYGPATH_W) 'aeabi_memclr.c'; else $(CYGPATH_W) '$(srcdir)/aeabi_memclr.c'; fi`
 
+lib_a-memcpy-stub.o: memcpy-stub.c
+	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-memcpy-stub.o `test -f 'memcpy-stub.c' || echo '$(srcdir)/'`memcpy-stub.c
+
+lib_a-memcpy-stub.obj: memcpy-stub.c
+	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-memcpy-stub.obj `if test -f 'memcpy-stub.c'; then $(CYGPATH_W) 'memcpy-stub.c'; else $(CYGPATH_W) '$(srcdir)/memcpy-stub.c'; fi`
+
 ID: $(HEADERS) $(SOURCES) $(LISP) $(TAGS_FILES)
 	list='$(SOURCES) $(HEADERS) $(LISP) $(TAGS_FILES)'; \
 	unique=`for i in $$list; do \
diff --git a/newlib/libc/machine/arm/memcpy-stub.c b/newlib/libc/machine/arm/memcpy-stub.c
new file mode 100644
index 0000000..c9de7c9
--- /dev/null
+++ b/newlib/libc/machine/arm/memcpy-stub.c
@@ -0,0 +1,39 @@ 
+/* Copyright (c) 2015 ARM Ltd.
+   All rights reserved.
+
+   Redistribution and use in source and binary forms, with or without
+   modification, are permitted provided that the following conditions are met:
+       * Redistributions of source code must retain the above copyright
+	 notice, this list of conditions and the following disclaimer.
+       * 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.
+       * Neither the name of the Linaro 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 COPYRIGHT HOLDERS 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 COPYRIGHT
+   HOLDER 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.  */
+
+/* The structure of the following #if #else #endif conditional chain
+   must match the chain in memcpy.S.  */
+
+#if (defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED))
+# include "../../string/memcpy.c"
+#elif (__ARM_ARCH >= 7 && __ARM_ARCH_PROFILE == 'A' \
+       && defined (__ARM_FEATURE_UNALIGNED))
+/* Defined in memcpy.S.  */
+#elif defined (__ARM_ARCH_7M__) || defined (__ARM_ARCH_7EM__)
+/* Defined in memcpy.S.  */
+#else
+# include "../../string/memcpy.c"
+#endif
diff --git a/newlib/libc/machine/arm/memcpy.S b/newlib/libc/machine/arm/memcpy.S
index b1bab88..854796a 100644
--- a/newlib/libc/machine/arm/memcpy.S
+++ b/newlib/libc/machine/arm/memcpy.S
@@ -26,19 +26,11 @@ 
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#if defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED)
-  /* Leave this field blank.  So the memcpy() is not defined, and this will
-     automatically pull in the default C definition of memcpy() from
-     ../../string/memcpy.c.  No need to include this file explicitely.
-     The lib_a-memcpy.o will not be generated, so it won't replace the default
-     lib_a-memcpy.o which is generated by ../../string/memcpy.c.
-     See the commands in configure.in and Makefile.am for more details.
+/* The structure of the following #if #else #endif conditional chain
+   must match the chain in memcpy-stub.c.  */
 
-     However, if we need to rewrite this function to be more efficient, we
-     can add the corresponding assembly code into this field and change the
-     commands in configure.in and Makefile.am to allow the corresponding
-     lib_a-memcpy.o to be generated.
-  */
+#if defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED)
+  /* Defined in memcpy-stub.c.  */
 
 #elif (__ARM_ARCH >= 7 && __ARM_ARCH_PROFILE == 'A' \
        && defined (__ARM_FEATURE_UNALIGNED))
@@ -48,5 +40,6 @@ 
 #include "memcpy-armv7m.S"
 
 #else
-  /* Leave this filed blank.  See the commands above.  */
+  /* Defined in memcpy-stub.c.  */
+
 #endif
-- 
1.9.1