[Mesa-dev] [PATCH] util/u_atomic: provide 64bit atomics where they're missing

[Matt Turner]

On Wed, Mar 29, 2017 at 4:13 PM, Grazvydas Ignotas <notasas at gmail.com> wrote:
> There are still some distributions trying to support unfortunate people
> with old or exotic CPUs that don't have 64bit atomic operations. When
> compiling for such a machine, gcc conveniently inserts a library call to
> a helper, but it's implementation is missing and we get a linker error.
> This allows us to provide our implementation, which is marked weak to
> prefer a better implementation, should one exist.
>
> Cc: Matt Turner <mattst88 at gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93089
> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
> ---

Thanks, this is a really good idea.

>  configure.ac              | 12 ++++++++
>  src/util/Makefile.sources |  1 +
>  src/util/u_atomic.c       | 71 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 84 insertions(+)
>  create mode 100644 src/util/u_atomic.c
>
> diff --git a/configure.ac b/configure.ac
> index ab9a91e..89b615b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -413,10 +413,22 @@ int main() {
>  if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" = x1; then
>      DEFINES="$DEFINES -DUSE_GCC_ATOMIC_BUILTINS"
>  fi
>  AM_CONDITIONAL([GCC_ATOMIC_BUILTINS_SUPPORTED], [test x$GCC_ATOMIC_BUILTINS_SUPPORTED = x1])
>
> +dnl Check if host supports 64bit atomics
> +dnl note that lack of support usually results in link (not compile) error
> +AC_LINK_IFELSE([AC_LANG_SOURCE([[
> +#include <stdint.h>
> +uint64_t v;
> +int main() {
> +    return __sync_add_and_fetch(&v, (uint64_t)1);
> +}]])], GCC_64BIT_ATOMICS_SUPPORTED=1)
> +if test "x$GCC_64BIT_ATOMICS_SUPPORTED" != x1; then
> +    DEFINES="$DEFINES -DMISSING_64BIT_ATOMICS"
> +fi
> +
>  dnl Check for Endianness
>  AC_C_BIGENDIAN(
>     little_endian=no,
>     little_endian=yes,
>     little_endian=no,
> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
> index 8ee45d5..e905734 100644
> --- a/src/util/Makefile.sources
> +++ b/src/util/Makefile.sources
> @@ -41,10 +41,11 @@ MESA_UTIL_FILES := \
>         string_to_uint_map.h \
>         strndup.h \
>         strtod.c \
>         strtod.h \
>         texcompress_rgtc_tmp.h \
> +       u_atomic.c \
>         u_atomic.h \
>         u_endian.h \
>         u_queue.c \
>         u_queue.h \
>         u_string.h \
> diff --git a/src/util/u_atomic.c b/src/util/u_atomic.c
> new file mode 100644
> index 0000000..77ef119
> --- /dev/null
> +++ b/src/util/u_atomic.c
> @@ -0,0 +1,71 @@
> +/*
> + * Copyright © 2017 The Mesa Project

The Mesa Project isn't something that can hold copyright. Your name
should be here.

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#if defined(MISSING_64BIT_ATOMICS) && defined(HAVE_PTHREAD)
> +
> +#include <stdint.h>
> +#include <pthread.h>
> +
> +#if defined(HAVE_FUNC_ATTRIBUTE_WEAK) && !defined(__CYGWIN__)
> +#define WEAK __attribute__((weak))
> +#else
> +#define WEAK
> +#endif
> +
> +static pthread_mutex_t sync_mutex = PTHREAD_MUTEX_INITIALIZER;
> +
> +WEAK uint64_t __sync_add_and_fetch_8(uint64_t *ptr, uint64_t val)

Let's do BSD-style function declarations, with the qualifiers and
return type on their own line.

With those two trivial things changed, this is

Reviewed-by: Matt Turner <mattst88 at gmail.com>

Grazvydas, if you have not already, please file a request for a
Freedesktop account [1] [2] and let's get you commit access.

Jonathan, can you check whether this resolves the bug entirely? Or are
there some other __sync functions we need to implement? I see
__sync_add_and_fetch_4, etc, in the bug report.

[1] https://bugs.freedesktop.org/enter_bug.cgi?product=freedesktop.org&component=New%20Accounts
[2] https://www.freedesktop.org/wiki/AccountRequests/