[Mesa-dev] [PATCH 1/2] configure: check if -latomic is needed for __atomic_*

[Grazvydas Ignotas]

On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas <notasas at gmail.com> wrote:
>> On some platforms, gcc generates library calls when __atomic_* functions
>> are used, but does not link the required library automatically. Detect
>> this and add the library when needed.
>>
>> This change was tested on armel (library was added) and on x86_64 (was
>> not, as expected).
>>
>> Fixes: 8915f0c0 "util: use GCC atomic intrinsics with explicit memory model"
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102573
>> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
>> ---
>>  configure.ac         | 13 +++++++++++++
>>  src/util/Makefile.am |  3 ++-
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 605c9b4..b46f29d 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -377,12 +377,25 @@ int main() {
>>      int n;
>>      return __atomic_load_n(&n, __ATOMIC_ACQUIRE);
>>  }]])], GCC_ATOMIC_BUILTINS_SUPPORTED=1)
>>  if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" = x1; then
>>      DEFINES="$DEFINES -DUSE_GCC_ATOMIC_BUILTINS"
>> +    dnl On some platforms, new-style atomics need a helper library
>> +    AC_MSG_CHECKING(whether -latomic is needed)
>> +    AC_LINK_IFELSE([AC_LANG_SOURCE([[
>> +    #include <stdint.h>
>> +    uint64_t v;
>> +    int main() {
>> +        return (int)__atomic_load_n(&v, __ATOMIC_ACQUIRE);
>> +    }]])], GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=no, GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=yes)
>> +    AC_MSG_RESULT($GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC)
>> +    if test "x$GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC" = xyes; then
>> +        LIBATOMIC_LIBS="-latomic"
>> +    fi
>>  fi
>>  AM_CONDITIONAL([GCC_ATOMIC_BUILTINS_SUPPORTED], [test x$GCC_ATOMIC_BUILTINS_SUPPORTED = x1])
>> +AC_SUBST([LIBATOMIC_LIBS])
>>
>>  dnl Check if host supports 64-bit atomics
>>  dnl note that lack of support usually results in link (not compile) error
>>  AC_MSG_CHECKING(whether __sync_add_and_fetch_8 is supported)
>>  AC_LINK_IFELSE([AC_LANG_SOURCE([[
>> diff --git a/src/util/Makefile.am b/src/util/Makefile.am
>> index 9885bbe..c37afff 100644
>> --- a/src/util/Makefile.am
>> +++ b/src/util/Makefile.am
>> @@ -46,11 +46,12 @@ libmesautil_la_SOURCES = \
>>         $(MESA_UTIL_FILES) \
>>         $(MESA_UTIL_GENERATED_FILES)
>>
>>  libmesautil_la_LIBADD = \
>>         $(CLOCK_LIB) \
>> -       $(ZLIB_LIBS)
>> +       $(ZLIB_LIBS) \
>> +       $(LIBATOMIC_LIBS)
>
> I can't remember how this works -- will $(LIBATOMIC_LIBS) be added
> transitively to anything that LDADDs libmesautil? I hope so...

I'm not sure either, I've simply tried it and it seemed to work. Maybe
Emil can comment if it's the right way...

> Oh, also, these should get a Cc: mesa-stable tag.

It has been said (and documented in docs/submittingpatches.html) that
Fixes: tag is enough for stable, and 2/2 it's strictly necessary I
think.