[Patch ARM] Provide stub function for __sync_synchronize
Richard Earnshaw
Richard.Earnshaw@foss.arm.com
Tue Sep 15 16:08:00 GMT 2015
On 11/09/15 12:28, Ramana Radhakrishnan wrote:
> Hi,
>
> A recent change to g++ means that we now put out
> calls to __sync_synchronize for C++ guard variables for
> versions of the architecture that do not have a barrier
> instruction. Instead of ignoring the problem in the compiler
> and working around it by providing a stub in libgcc, provide a
> weak definition here in order for testing to succeed but to highlight
> this for users provide a link time warning when this is used.
>
> Additionally libgfortran now pulls in libbacktrace which expects
> to be able to use atomic accesses where required. This creates
> problems when testing in bare-metal configurations.
>
> This means though that developers and testers for arm-none-eabi
> will have to add something to prune additional output in their
> site.exp's with an additional implementation of prune_warnings
> to prune the warnings but this is more prudent than
> just putting out silent calls to __sync_synchronize.
>
> Tested this for arm-none-eabi with --with-multilib-list=aprofile
> with suitable multilibs.
>
> Thoughts ?
>
> Ok ?
>
> Ramana
>
> <DATE> Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
>
> * libc/machine/arm/sync_synchronize.c: New file.
> * libc/machine/arm/Makefile.am: Add sync_synchronize.c
> * libc/machine/arm/Makefile.in: Regenerate.
>
>
> adjust-newlib.txt
>
>
> commit a839594943b59b6ba81d1f37f7684f156598f2d1
> Author: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
> Date: Tue Sep 8 09:48:37 2015 +0100
>
> [Patch ARM] Provide __sync_synchronize stub.
>
> A recent change to g++ means that we now put out
> calls to __sync_synchronize for C++ guard variables for
> versions of the architecture that do not have a barrier
> instruction. Instead of ignoring the problem in the compiler
> and working around it by providing a stub in libgcc, provide a
> weak definition here in order for testing to succeed but for
> users to provide their own definition of the same.
>
> Additionally libgfortran now pulls in libbacktrace which expects
> to be able to use atomic accesses where required. This creates
> problems when testing in bare-metal configurations.
>
> <DATE> Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>
>
> * libc/machine/arm/sync_synchronize.c: New file.
> * libc/machine/arm/Makefile.am: Add sync_synchronize.c
> * libc/machine/arm/Makefile.in: Regenerate.
>
> diff --git a/newlib/libc/machine/arm/Makefile.am b/newlib/libc/machine/arm/Makefile.am
> index 55bc0a7..2f4b2e2 100644
> --- a/newlib/libc/machine/arm/Makefile.am
> +++ b/newlib/libc/machine/arm/Makefile.am
> @@ -51,7 +51,8 @@ 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
> + aeabi_memset.c aeabi_memset-soft.S aeabi_memclr.c \
> + sync_synchronize.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 7a3506e..364b798 100644
> --- a/newlib/libc/machine/arm/Makefile.in
> +++ b/newlib/libc/machine/arm/Makefile.in
> @@ -89,7 +89,7 @@ am_lib_a_OBJECTS = lib_a-setjmp.$(OBJEXT) lib_a-access.$(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-sync_synchronize.$(OBJEXT)
> lib_a_OBJECTS = $(am_lib_a_OBJECTS)
> DEFAULT_INCLUDES = -I.@am__isrc@
> depcomp =
> @@ -238,7 +238,8 @@ 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
> + aeabi_memset.c aeabi_memset-soft.S aeabi_memclr.c \
> + sync_synchronize.c
>
> lib_a_CCASFLAGS = $(AM_CCASFLAGS)
> lib_a_CFLAGS = $(AM_CFLAGS)
> @@ -410,6 +411,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-sync_synchronize.o: sync_synchronize.c
> + $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-sync_synchronize.o `test -f 'sync_synchronize.c' || echo '$(srcdir)/'`sync_synchronize.c
> +
> +lib_a-sync_synchronize.obj: sync_synchronize.c
> + $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) $(lib_a_CFLAGS) $(CFLAGS) -c -o lib_a-sync_synchronize.obj `if test -f 'sync_synchronize.c'; then $(CYGPATH_W) 'sync_synchronize.c'; else $(CYGPATH_W) '$(srcdir)/sync_synchronize.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/sync_synchronize.c b/newlib/libc/machine/arm/sync_synchronize.c
> new file mode 100644
> index 0000000..7695ebc
> --- /dev/null
> +++ b/newlib/libc/machine/arm/sync_synchronize.c
> @@ -0,0 +1,43 @@
> +/*
> + * 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:
> + * 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.
> + */
> +
> +/* This is a fall-back definition only to handle legacy architectures
> + which do not have any expectation of providing multi-core support.
> + This function stub is provided for testsuites to work without
> + issues and for linking to complete, but if a platform requires any
> + kind of multi-threading support it is expected that the platform
> + will override the implementation of this barrier intrinsic. */
> +
> +#include <sys/cdefs.h>
> +void __attribute__((weak))
> +__sync_synchronize (void)
> +{
> + __warn_references (__sync_synchronize,
> + "Warning:Provide a suitable definition for __sync_synchronize");
> + return;
> +}
>
I think this is generally a good idea. But there are some problems with
the patch as it stands.
__warn_references already prints "warning: ", so an additional warning
is redundant.
I think a better message would be something along the lines of "legacy
compatible NOP implementation of __sync_synchronize used. This won't
work if your application is multi-threaded."
Finally, I think that if newlib is built with a suitable version of the
architecture then the implementation should use a real barrier
instruction and suppress the warning.
R.
More information about the Newlib
mailing list