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

["Bin.Cheng" <amker dot cheng at gmail dot com>]

On Fri, Aug 8, 2014 at 6:19 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
> 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.

Ah, they are conflict with each other in ABI level.  If __aeabi_memcpy
could be alias to c version memcpy, how could it reserve registers
other than AAPCS allows?

Thanks,
bin
>
> 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);
>>> +}
>
>