This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.

From: Richard Earnshaw <rearnsha at arm dot com>
To: Hale Wang <Hale dot Wang at arm dot com>
Cc: "'Bin.Cheng'" <amker dot cheng at gmail dot com>, "newlib at sourceware dot org" <newlib at sourceware dot org>
Date: Fri, 08 Aug 2014 11:19:36 +0100
Subject: Re: Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.
Authentication-results: sourceware.org; auth=none
References: <000601cfb2c2$a9386a40$fba93ec0$ at arm dot com> <CAHFci28LV01ZA5i6EZyAngQT=0O8WG-WDH3vmRwdHCgL63C4rA at mail dot gmail dot com> <000a01cfb2df$f5d79070$e186b150$ at arm dot com> <CAHFci2-5cjHnrvH3s-Nw2Mtb8Y+mjWYty5TrYLExL3qX9LjURg at mail dot gmail dot com> <000b01cfb2f0$49dc8e40$dd95aac0$ at arm dot com>

I've two concerns about this, one important.

1) Minor concern.  The point of __aeabi_memcpy[4|8] is that they are
more efficient than __aeabi_memcpy since they know the aligment and size
are a multiple of 4/8 respectively.

2) Major concern.  __aeabi_memcpy* are not permitted to clobber any
callee clobbered registers outside of the core register set
(specifically not to clobber any FP/SIMD registers).  Unfortunately, the
memcpy in newlib does not guarantee this, particularly for ARMv7-a.  You
need to either provide alternative implementations in that case, or to
save the appropriate FP/SIMD registers around a call to memcpy.

R.


On 08/08/14 11:05, Hale Wang wrote:
> The patch and changelog are updated as the attachments.
> 
> Thanks and Best Regards,
> Hale Wang
> 
>> -----Original Message-----
>> From: Bin.Cheng [mailto:amker.cheng@gmail.com]
>> Sent: 2014å8æ8æ 17:16
>> To: Hale Wang
>> Cc: newlib@sourceware.org
>> Subject: Re: Support __aeabi_memcpy, __aeabi_memcpy4 and
>> __aeabi_memcpy8 routines in the arm backend.
>>
>> On Fri, Aug 8, 2014 at 4:08 PM, Hale Wang <Hale.Wang@arm.com> wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Bin.Cheng [mailto:amker.cheng@gmail.com]
>>>> Sent: 2014å8æ8æ 13:48
>>>> To: Hale Wang
>>>> Cc: newlib@sourceware.org
>>>> Subject: Re: Support __aeabi_memcpy, __aeabi_memcpy4 and
>>>> __aeabi_memcpy8 routines in the arm backend.
>>>>
>>>> On Fri, Aug 8, 2014 at 12:38 PM, Hale Wang <Hale.Wang@arm.com>
>> wrote:
>>>>> Hi,
>>>>>
>>>>> The newlib libraries should suppport the presence of the aliases
>>>>> for memcpy routines. The __aeabi_memcpy, __aeabi_memcpy4 and
>>>>> __aeabi_memcpy8 should be defined.
>>>>>
>>>>> In this patch, __aeabi_memcpy, __aeabi_memcpy4 and
>> __aeabi_memcpy8
>>>> are
>>>> What about memmove/memmove4/memmove4?
>>>>
>>>
>>> I will update the memclr/memmove/memset functions in a following patch.
>>>
>>>>> defined in the new file newlib/libc/machine/arm/aeabi_memcpy.c. And
>>>>> they are the aliases for memcpy routines.
>>>>>
>>>>> Bootstrap and no make check regression on X86-64.
>>>> Do you mean no regressions with arm toolchain hosted on x86_64?
>>>>
>>>
>>> Yes.
>>>
>>>>>
>>>>> Patch also attached for convenience.
>>>>>
>>>>> Thanks and Best Regards,
>>>>> Hale Wang
>>>>>
>>>>> newlib/ChangeLog:
>>>>>
>>>>> 2014-07-29  Hale Wang  <hale.wang@arm.com>
>>>>>
>>>>>         * libc/machine/arm/aeabi_memcpy.c: New file.
>>>>>         * libc/machine/arm/Makefile.am: add dependencies.
>>>>>         * libc/machine/arm/Makefile.in: Likewise.
>>>> Makefile.in should be "Regenerated" or something else, rather than
>>> "likewise".
>>>>
>>>
>>> OK, I see. I will update this commend to "Regenerated".
>>>
>>>>>
>>>>>
>>>>
>> ============================================================
>>>> ==========
>>>>> == diff --git a/newlib/libc/machine/arm/Makefile.am
>>>>> b/newlib/libc/machine/arm/Makefile.am
>>>>> index fb33926..e052286 100644
>>>>> --- a/newlib/libc/machine/arm/Makefile.am
>>>>> +++ b/newlib/libc/machine/arm/Makefile.am
>>>>> @@ -10,7 +10,7 @@ noinst_LIBRARIES = lib.a
>>>>>
>>>>>  lib_a_SOURCES = setjmp.S access.c strlen.c strcmp.S strcpy.c \
>>>>>                 memcpy.S memcpy-stub.c memchr-stub.c memchr.S \
>>>>> -               strlen.c strlen-armv7.S
>>>>> +               strlen.c strlen-armv7.S aeabi_memcpy.c
>>>>>  lib_a_CCASFLAGS=$(AM_CCASFLAGS)
>>>>>  lib_a_CFLAGS = $(AM_CFLAGS)
>>>>>
>>>>> diff --git a/newlib/libc/machine/arm/Makefile.in
>>>>> b/newlib/libc/machine/arm/Makefile.in
>>>>> index 1ccfac5..8e20914 100644
>>>>> --- a/newlib/libc/machine/arm/Makefile.in
>>>>> +++ b/newlib/libc/machine/arm/Makefile.in
>>>>> @@ -74,7 +74,7 @@ am_lib_a_OBJECTS = lib_a-setjmp.$(OBJEXT)
>>>>> lib_a-access.$(OBJEXT) \
>>>>>         lib_a-strcpy.$(OBJEXT) lib_a-memcpy.$(OBJEXT) \
>>>>>         lib_a-memcpy-stub.$(OBJEXT) lib_a-memchr-stub.$(OBJEXT) \
>>>>>         lib_a-memchr.$(OBJEXT) lib_a-strlen.$(OBJEXT) \
>>>>> -       lib_a-strlen-armv7.$(OBJEXT)
>>>>> +       lib_a-strlen-armv7.$(OBJEXT) lib_a-aeabi_memcpy.$(OBJEXT)
>>>>>  lib_a_OBJECTS = $(am_lib_a_OBJECTS)  DEFAULT_INCLUDES =
>>>>> -I.@am__isrc@  depcomp = @@ -202,7 +202,7 @@ AM_CCASFLAGS =
>>>>> $(INCLUDES)  noinst_LIBRARIES = lib.a  lib_a_SOURCES = setjmp.S
>>>>> access.c strlen.c strcmp.S strcpy.c \
>>>>>                 memcpy.S memcpy-stub.c memchr-stub.c memchr.S \
>>>>> -               strlen.c strlen-armv7.S
>>>>> +               strlen.c strlen-armv7.S aeabi_memcpy.c
>>>>>
>>>>>  lib_a_CCASFLAGS = $(AM_CCASFLAGS)
>>>>>  lib_a_CFLAGS = $(AM_CFLAGS)
>>>>> @@ -336,6 +336,12 @@ lib_a-memchr-stub.o: memchr-stub.c
>>>>>  lib_a-memchr-stub.obj: memchr-stub.c
>>>>>         $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES)
>>>>> $(AM_CPPFLAGS)
>>>>> $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-memchr-stub.obj
>>>>> `if test -f 'memchr-stub.c'; then $(CYGPATH_W) 'memchr-stub.c';
>>>>> else
>>>>> $(CYGPATH_W) '$(srcdir)/memchr-stub.c'; fi`
>>>>>
>>>>> +lib_a-aeabi_memcpy.o: aeabi_memcpy.c
>>>>> +       $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES)
>>>>> +$(AM_CPPFLAGS)
>>>>> $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-aeabi_memcpy.o
>>>>> `test -f 'aeabi_memcpy.c' || echo '$(srcdir)/'`aeabi_memcpy.c
>>>>> +
>>>>> +lib_a-aeabi_memcpy.obj: aeabi_memcpy.c
>>>>> +       $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES)
>>>>> +$(AM_CPPFLAGS)
>>>>> $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-aeabi_memcpy.obj
>>>>> `if test -f 'aeabi_memcpy.c'; then $(CYGPATH_W) 'aeabi_memcpy.c';
>>>>> else
>>>>> $(CYGPATH_W) '$(srcdir)/aeabi_memcpy.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/aeabi_memcpy.c
>>>>> b/newlib/libc/machine/arm/aeabi_memcpy.c
>>>>> new file mode 100644
>>>>> index 0000000..b3e18ea
>>>>> --- /dev/null
>>>>> +++ b/newlib/libc/machine/arm/aeabi_memcpy.c
>>>>> @@ -0,0 +1,45 @@
>>>>> +/*
>>>>> + * Copyright (c) 2014 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:
>>>>> + * 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. The name of the company may not be used to endorse or promote
>>>>> + *    products derived from this software without specific prior
>>> written
>>>>> + *    permission.
>>>>> + *
>>>>> + * THIS SOFTWARE IS PROVIDED BY ARM LTD ``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 ARM LTD 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 <stddef.h>
>>>>> +
>>>>> +/* Support the alias for the __aeabi_memcpy which may
>>>>> +   assume memory alignment.  */
>>>>> +void __aeabi_memcpy4 (void *dest, const void *source, size_t n)
>>>>> +       __attribute__((alias ("__aeabi_memcpy")));
>>>>> +
>>>>> +void __aeabi_memcpy8 (void *dest, const void *source, size_t n)
>>>>> +       __attribute__((alias ("__aeabi_memcpy")));
>>>>
>>>> Please use _ATTRIBUTE(__alias__) though there are exceptions
>>>> elsewhere, it would be nice to start here.
>>>>
>>>
>>> So do you mean __ATTRIBUTE(__alias__) is the same with
>>> __attribute__((alias))?
>>
>> It's a macro actually.  This is for the sake of portability.  Consider building
>> newlib with a compiler doesn't support __attribute__!
>>
>>
>>>
>>>>> +
>>>>> +/* Support the routine __aeabi_memcpy.  Can't alias to memcpy
>>>>> +   because it's not defined in the same translation unit.  */ void
>>>>> +__aeabi_memcpy (void *dest, const void *source, size_t n) {
>>>>> +       extern void memcpy (void *dest, const void *source, size_t
>>>>> +n);
>>>> Any reason why explicitly declare memcpy function, rather than just
>>> include
>>>> header file?
>>>>
>>>
>>> Because memcpy is defined multi-times in different files
>>> newlib/libc/machine/arm/ and newlib/libc/string/. There was an error
>>> message about conflict definition if using the include header file.
>>
>> I don't get this point very well.  The prototype of memcpy should be same, it's
>> defined by standard, right?
>>
> 
> The memcpy function is defined in newlib/libc/string/memcpy.c. But for arm target, it is redefined in newlib/libc/machine/arm/memcpy.S. 
> So if we include <string.h> in newlib/libc/machine/arm/aeabi_memcpy.c, this will lead to conflict when we generate the libc.a of arm targets.
> 
>>>
>>>>> +       memcpy (dest, source, n);
>>>> Better to use GNU code style?
>>>>
>>>
>>> Do you mean just need two space at the header rather than a TAB?
>> Yes.
>>
>> Thanks,
>> bin
>>
>> aeabi_memcpy_4.patch
>>
>>
>> diff --git a/newlib/libc/machine/arm/Makefile.am b/newlib/libc/machine/arm/Makefile.am
>> index ffba0fe..6acc7e7 100644
>> --- a/newlib/libc/machine/arm/Makefile.am
>> +++ b/newlib/libc/machine/arm/Makefile.am
>> @@ -10,7 +10,7 @@ noinst_LIBRARIES = lib.a
>>  
>>  lib_a_SOURCES = setjmp.S access.c strlen.c strcmp.S strcpy.c \
>>  	        memcpy.S memcpy-stub.c memchr-stub.c memchr.S \
>> -		strlen.c strlen-armv7.S
>> +		strlen.c strlen-armv7.S aeabi_memcpy.c
>>  lib_a_CCASFLAGS=$(AM_CCASFLAGS)
>>  lib_a_CFLAGS = $(AM_CFLAGS)
>>  
>> diff --git a/newlib/libc/machine/arm/Makefile.in b/newlib/libc/machine/arm/Makefile.in
>> index b3d75e8..360a41f 100644
>> --- a/newlib/libc/machine/arm/Makefile.in
>> +++ b/newlib/libc/machine/arm/Makefile.in
>> @@ -74,7 +74,7 @@ am_lib_a_OBJECTS = lib_a-setjmp.$(OBJEXT) lib_a-access.$(OBJEXT) \
>>  	lib_a-strcpy.$(OBJEXT) lib_a-memcpy.$(OBJEXT) \
>>  	lib_a-memcpy-stub.$(OBJEXT) lib_a-memchr-stub.$(OBJEXT) \
>>  	lib_a-memchr.$(OBJEXT) lib_a-strlen.$(OBJEXT) \
>> -	lib_a-strlen-armv7.$(OBJEXT)
>> +	lib_a-strlen-armv7.$(OBJEXT) lib_a-aeabi_memcpy.$(OBJEXT)
>>  lib_a_OBJECTS = $(am_lib_a_OBJECTS)
>>  DEFAULT_INCLUDES = -I.@am__isrc@
>>  depcomp =
>> @@ -202,7 +202,7 @@ AM_CCASFLAGS = $(INCLUDES)
>>  noinst_LIBRARIES = lib.a
>>  lib_a_SOURCES = setjmp.S access.c strlen.c strcmp.S strcpy.c \
>>  	        memcpy.S memcpy-stub.c memchr-stub.c memchr.S \
>> -		strlen.c strlen-armv7.S
>> +		strlen.c strlen-armv7.S aeabi_memcpy.c
>>  
>>  lib_a_CCASFLAGS = $(AM_CCASFLAGS)
>>  lib_a_CFLAGS = $(AM_CFLAGS)
>> @@ -336,6 +336,12 @@ lib_a-memchr-stub.o: memchr-stub.c
>>  lib_a-memchr-stub.obj: memchr-stub.c
>>  	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-memchr-stub.obj `if test -f 'memchr-stub.c'; then $(CYGPATH_W) 'memchr-stub.c'; else $(CYGPATH_W) '$(srcdir)/memchr-stub.c'; fi`
>>  
>> +lib_a-aeabi_memcpy.o: aeabi_memcpy.c
>> +	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-aeabi_memcpy.o `test -f 'aeabi_memcpy.c' || echo '$(srcdir)/'`aeabi_memcpy.c
>> +
>> +lib_a-aeabi_memcpy.obj: aeabi_memcpy.c
>> +	$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-aeabi_memcpy.obj `if test -f 'aeabi_memcpy.c'; then $(CYGPATH_W) 'aeabi_memcpy.c'; else $(CYGPATH_W) '$(srcdir)/aeabi_memcpy.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/aeabi_memcpy.c b/newlib/libc/machine/arm/aeabi_memcpy.c
>> new file mode 100644
>> index 0000000..b9db775
>> --- /dev/null
>> +++ b/newlib/libc/machine/arm/aeabi_memcpy.c
>> @@ -0,0 +1,46 @@
>> +/*
>> + * Copyright (c) 2014 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:
>> + * 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. The name of the company may not be used to endorse or promote
>> + *    products derived from this software without specific prior written
>> + *    permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY ARM LTD ``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 ARM LTD 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 <stddef.h>
>> +#include <_ansi.h>
>> +
>> +/* Support the alias for the __aeabi_memcpy which may
>> +   assume memory alignment.  */
>> +void __aeabi_memcpy4 (void *dest, const void *source, size_t n)
>> +	_ATTRIBUTE ((alias ("__aeabi_memcpy")));
>> +
>> +void __aeabi_memcpy8 (void *dest, const void *source, size_t n)
>> +	_ATTRIBUTE ((alias ("__aeabi_memcpy")));
>> +
>> +/* Support the routine __aeabi_memcpy.  Can't alias to memcpy
>> +   because it's not defined in the same translation unit.  */
>> +void __aeabi_memcpy (void *dest, const void *source, size_t n)
>> +{
>> +  extern void memcpy (void *dest, const void *source, size_t n);
>> +  memcpy (dest, source, n);
>> +}

Follow-Ups:
- Re: Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.
  - From: Bin.Cheng

References:
- Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.
  - From: Hale Wang
- Re: Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.
  - From: Bin.Cheng
- RE: Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.
  - From: Hale Wang
- Re: Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.
  - From: Bin.Cheng
- RE: Support __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8 routines in the arm backend.
  - From: Hale Wang

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]