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

2017-09-19 Thread Emil Velikov 
On 19 September 2017 at 13:01, Grazvydas Ignotas  wrote:
> On Tue, Sep 19, 2017 at 2:04 PM, Emil Velikov  
> wrote:
>> On 19 September 2017 at 10:12, Grazvydas Ignotas  wrote:
>>> On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner  wrote:
 On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas  
 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.
>
> I can't find it either. there is only this wiki that looks more like a
> design document:
> https://gcc.gnu.org/wiki/Atomic/GCCMM
>
> If I understand it right, when gcc can't do the operations with
> instructions it emits calls, but doesn't care how they are satisfied
> and leaves dealing with that to the application. libatomic is provided
> since 4.8, but there is no requirement to use it (to allow using
> something else?).
>
Right, I saw the article but got carried away looking at the provided
implementation ;-)

With the link added to the commit message the series is
Reviewed-by: Emil Velikov 

Barring any objections, I'll push the lot in a couple of days.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-09-19 Thread Grazvydas Ignotas 
On Tue, Sep 19, 2017 at 2:04 PM, Emil Velikov  wrote:
> On 19 September 2017 at 10:12, Grazvydas Ignotas  wrote:
>> On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner  wrote:
>>> On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas  
>>> 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.

I can't find it either. there is only this wiki that looks more like a
design document:
https://gcc.gnu.org/wiki/Atomic/GCCMM

If I understand it right, when gcc can't do the operations with
instructions it emits calls, but doesn't care how they are satisfied
and leaves dealing with that to the application. libatomic is provided
since 4.8, but there is no requirement to use it (to allow using
something else?).

GraÅžvydas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-09-19 Thread Emil Velikov 
On 19 September 2017 at 10:12, Grazvydas Ignotas  wrote:
> On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner  wrote:
>> On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas  
>> 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 
>>> ---
>>>  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(, __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 
>>> +uint64_t v;
>>> +int main() {
>>> +return (int)__atomic_load_n(, __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

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

2017-09-19 Thread Grazvydas Ignotas 
On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner  wrote:
> On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas  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 
>> ---
>>  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(, __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 
>> +uint64_t v;
>> +int main() {
>> +return (int)__atomic_load_n(, __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.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-09-18 Thread Matt Turner 
Oh, also, these should get a Cc: mesa-stable tag.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-09-18 Thread Matt Turner 
On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas  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 
> ---
>  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(, __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 
> +uint64_t v;
> +int main() {
> +return (int)__atomic_load_n(, __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...

Both are

Reviewed-by: Matt Turner 

FWIW, libatomic only exists since gcc-4.8, so we'll still have some gaps.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-09-18 Thread Grazvydas Ignotas 
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 
---
 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(, __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 
+uint64_t v;
+int main() {
+return (int)__atomic_load_n(, __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)
 
 libxmlconfig_la_SOURCES = $(XMLCONFIG_FILES)
 libxmlconfig_la_CFLAGS = \
$(DEFINES) \
-I$(top_srcdir)/include \
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev