On 19 September 2017 at 10:12, Grazvydas Ignotas <nota...@gmail.com> wrote: > On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner <matts...@gmail.com> wrote: >> On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas <nota...@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). >>> On one hand I see the reason why things are as-is. On the other its rather odd that I couldn't find an official GCC doc that describes the lot. Most likely I failed at searching.
Can anyone point me to the man page that recommends this type of checks/linking? Objections if we add it to the commit message? >>> 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 <nota...@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... > Yes, adding LIBATOMIC_LIBS here will be propagated into all the targets that use libmesautil.la >> 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. True. Adding the stable won't hurt ;-) Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
