mailing list of musl libc
 help / color / mirror / code / Atom feed
* [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
@ 2014-11-09 12:53 Joakim Sindholt
  2014-11-09 17:11 ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Joakim Sindholt @ 2014-11-09 12:53 UTC (permalink / raw)
  To: musl; +Cc: Joakim Sindholt

GCC and Clang ship their own stdatomic.h headers but not for all the
versions mentioned above. They ship many other standard headers too that
musl also ships (stdbool/def/int/...).

In the end musl should be a complete libc with all standard headers and
this is one of them. It is up to the individual programmer to ensure
that his or her compiler is properly supported by for example checking
__STD_NO_ATOMICS__ and __STDC_VERSION__ >= 201112L.

For clang the support is complete for versions >= 3.1. GCC added the
__atomic functions in 4.7 but not _Atomic until 4.9 so from 4.1 it will
work correctly so long as you don't use VLAs together with things like
the ++ operator in arguments and is subject to the same limitations as
4.7. From 4.7 it will work as inteded so long as you only use the
function-like-macros and from 4.9 it will have full support.

This patch has only been tested with clang. The __typeof__ magic is to
avoid changes to alltypes. It should be changed before committing this
to musl. This patch is only for people who want to test themselves.

This work is released to the Public Domain.
---
 include/stdatomic.h | 287 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 287 insertions(+)
 create mode 100644 include/stdatomic.h

diff --git a/include/stdatomic.h b/include/stdatomic.h
new file mode 100644
index 0000000..3a61e47
--- /dev/null
+++ b/include/stdatomic.h
@@ -0,0 +1,287 @@
+#ifndef _STDATOMIC_H
+#define _STDATOMIC_H
+
+#ifndef __has_extension
+#define __has_extension(x) 0
+#define __musl_has_extension 1
+#endif
+
+#if defined(__clang__) && __has_extension(c_atomic)
+
+#define kill_dependency(y) (y)
+
+#define ATOMIC_VAR_INIT(value) (value)
+#define atomic_init(obj, value) __c11_atomic_init(obj, value)
+
+#define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(obj))
+#define __atomic_type_is_lock_free(type) \
+    __c11_atomic_is_lock_free(sizeof(type))
+
+#define atomic_thread_fence(order) __c11_atomic_thread_fence(order)
+#define atomic_signal_fence(order) __c11_atomic_signal_fence(order)
+
+#define atomic_store_explicit(object, desired, order) \
+    __c11_atomic_store(object, desired, order)
+#define atomic_load_explicit(object, order) \
+    __c11_atomic_load(object, order)
+
+#define atomic_exchange_explicit(object, value, order) \
+    __c11_atomic_exchange(object, value, order)
+#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
+                                                success, failure) \
+    __c11_atomic_compare_exchange_strong(object, expected, desired, \
+                                         success, failure)
+#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
+                                              success, failure) \
+    __c11_atomic_compare_exchange_weak(object, expected, desired, \
+                                       success, failure)
+
+#define atomic_fetch_add_explicit(object, operand, order) \
+    __c11_atomic_fetch_add(object, operand, order)
+#define atomic_fetch_sub_explicit(object, operand, order) \
+    __c11_atomic_fetch_sub(object, operand, order)
+#define atomic_fetch_or_explicit(object, operand, order) \
+    __c11_atomic_fetch_or(object, operand, order)
+#define atomic_fetch_xor_explicit(object, operand, order) \
+    __c11_atomic_fetch_xor(object, operand, order)
+#define atomic_fetch_and_explicit(object, operand, order) \
+    __c11_atomic_fetch_and(object, operand, order)
+
+#elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) || __GNUC__ > 4
+
+#define ATOMIC_VAR_INIT(value) (value)
+#define atomic_init(obj, value) do { *(obj) = (value); } while (0)
+
+#if (__GNUC__ == 4 && __GNUC_MINOR__ >= 9) || __GNUC__ > 4
+#define kill_dependency(y) __extension__({__auto_type __y = (y); __y;})
+
+#define atomic_is_lock_free(obj) \
+    __extension__({ \
+        __auto_type __obj = (obj); \
+        __atomic_is_lock_free(sizeof(*__obj), __obj); \
+    })
+#else
+#define __NEED__Atomic
+
+#define kill_dependency(y) (y)
+
+#define atomic_is_lock_free(obj) \
+    __atomic_is_lock_free(sizeof(*(obj)), (void *)0)
+#endif
+
+#define __atomic_type_is_lock_free(type) \
+    __atomic_always_lock_free(sizeof(type), (void *)0)
+
+#define atomic_thread_fence(order) __atomic_thread_fence(order)
+#define atomic_signal_fence(order) __atomic_signal_fence(order)
+
+#define atomic_store_explicit(object, value, order) \
+    __atomic_store_n(object, value, order)
+#define atomic_load_explicit(object, order) \
+    __atomic_load_n(object, order)
+
+#define atomic_exchange_explicit(object, desired, order) \
+    __atomic_exchange_n(object, desired, order)
+#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
+                                                success, failure) \
+    __atomic_compare_exchange_n(object, expected, desired, 0, success, failure)
+#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
+                                              success, failure) \
+    __atomic_compare_exchange_n(object, expected, desired, 1, success, failure)
+
+#define atomic_fetch_add_explicit(object, operand, order) \
+    __atomic_fetch_add(object, operand, order)
+#define atomic_fetch_sub_explicit(object, operand, order) \
+    __atomic_fetch_sub(object, operand, order)
+#define atomic_fetch_or_explicit(object, operand, order) \
+    __atomic_fetch_or(object, operand, order)
+#define atomic_fetch_xor_explicit(object, operand, order) \
+    __atomic_fetch_xor(object, operand, order)
+#define atomic_fetch_and_explicit(object, operand, order) \
+    __atomic_fetch_and(object, operand, order)
+
+#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 1
+
+#define __NEED__Atomic
+
+#define kill_dependency(y) (y)
+
+#define ATOMIC_VAR_INIT(value) (value)
+#define atomic_init(obj, value) do { *(obj) = (value); } while (0)
+
+#define atomic_is_lock_free(obj) (sizeof(obj) <= sizeof(void *))
+#define __atomic_type_is_lock_free(type) (sizeof(type) <= sizeof(void *))
+
+#define atomic_thread_fence(order) __sync_synchronize()
+#define atomic_signal_fence(order) __asm__ volatile ("" : : : "memory")
+
+/* for GCC < 4.7 some concessions are made. First off, ordering is ignored and
+ * always treated as a full seq_cst barrier. Second, although these are
+ * described as "generic functions" GCC < 4.7 cannot support uses such as:
+ * int n = 4;
+ * atomic_int arr[n];
+ * atomic_int *i = arr;
+ * if (atomic_compare_exchange_strong(i++, ...)) {...
+ * because of the liberal use of the __typeof__ extension.
+ * Furthermore, GCC specified that test_and_set doesn't necessarily support
+ * arbitrary values. It's used as both exchange and store here. Be careful. */
+
+#define atomic_store_explicit(object, value, order) \
+    do { \
+        __sync_lock_test_and_set(object, value); \
+        __sync_synchronize(); \
+    while (0)
+#define atomic_load_explicit(object, order) \
+    __atomic_fetch_and_add(object, 0)
+
+#define atomic_exchange_explicit(object, desired, order) \
+    __extension__({ \
+        __typeof__(*(object)) __ret; \
+        __ret = __sync_lock_test_and_set(object, desired); \
+        __sync_synchronize(); \
+        __ret; \
+    })
+#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
+                                                success, failure) \
+    __extension__({ \
+        __typeof__(object) __expected = (expected); \
+        __typeof__(*__expected) __prev; \
+        _Bool __ret; \
+        __prev = __sync_val_compare_and_swap(object, *__expected, desired); \
+        __ret = (__prev == *__expected); \
+        *__expected = __prev; \
+        __ret; \
+    })
+#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
+                                              success, failure) \
+    atomic_compare_exchange_strong_explicit(object, expected, desired, \
+                                            success, failure)
+
+#define atomic_fetch_add_explicit(object, operand, order) \
+    __sync_fetch_and_add(object, operand)
+#define atomic_fetch_sub_explicit(object, operand, order) \
+    __sync_fetch_and_sub(object, operand)
+#define atomic_fetch_or_explicit(object, operand, order) \
+    __sync_fetch_and_or(object, operand)
+#define atomic_fetch_xor_explicit(object, operand, order) \
+    __sync_fetch_and_xor(object, operand)
+#define atomic_fetch_and_explicit(object, operand, order) \
+    __sync_fetch_and_and(object, operand)
+
+#else
+
+#error "Musl's stdatomic.h does not support your compiler"
+
+#endif
+
+#ifdef __musl_has_extension
+#undef __musl_has_extension
+#undef __has_extension
+#endif
+
+#ifdef __NEED__Atomic
+#undef __NEED__Atomic
+#define _Atomic volatile
+#define volatile(x) x volatile
+#endif
+
+typedef enum {
+    memory_order_relaxed = 0,
+    memory_order_consume = 1,
+    memory_order_acquire = 2,
+    memory_order_release = 3,
+    memory_order_acq_rel = 4,
+    memory_order_seq_cst = 5
+} memory_order;
+
+typedef _Atomic _Bool atomic_bool;
+typedef _Atomic char atomic_char;
+typedef _Atomic signed char atomic_schar;
+typedef _Atomic short atomic_short;
+typedef _Atomic int atomic_int;
+typedef _Atomic long atomic_long;
+typedef _Atomic long long atomic_llong;
+typedef _Atomic unsigned char atomic_uchar;
+typedef _Atomic unsigned short atomic_ushort;
+typedef _Atomic unsigned int atomic_uint;
+typedef _Atomic unsigned long atomic_ulong;
+typedef _Atomic unsigned long long atomic_ullong;
+typedef _Atomic unsigned short atomic_char16_t;
+typedef _Atomic unsigned atomic_char32_t;
+typedef _Atomic __typeof__(L'\0') atomic_wchar_t;
+typedef _Atomic signed char atomic_int_least8_t;
+typedef _Atomic short atomic_int_least16_t;
+typedef _Atomic int atomic_int_least32_t;
+typedef _Atomic __typeof__(0x100000000) atomic_int_least64_t;
+typedef _Atomic signed char atomic_int_fast8_t;
+typedef _Atomic int atomic_int_fast16_t;
+typedef _Atomic int atomic_int_fast32_t;
+typedef _Atomic __typeof__(0x100000000) atomic_int_fast64_t;
+typedef _Atomic unsigned char atomic_uint_least8_t;
+typedef _Atomic unsigned short atomic_uint_least16_t;
+typedef _Atomic unsigned int atomic_uint_least32_t;
+typedef _Atomic __typeof__(0x100000000U) atomic_uint_least64_t;
+typedef _Atomic unsigned char atomic_uint_fast8_t;
+typedef _Atomic unsigned atomic_uint_fast16_t;
+typedef _Atomic unsigned atomic_uint_fast32_t;
+typedef _Atomic __typeof__(0x100000000U) atomic_uint_fast64_t;
+typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_intptr_t;
+typedef _Atomic __typeof__(sizeof(0)) atomic_uintptr_t;
+typedef _Atomic __typeof__(sizeof(0)) atomic_size_t_t;
+typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_ptrdiff_t;
+typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_intmax_t;
+typedef _Atomic __typeof__(sizeof(0)) atomic_uintmax_t;
+
+#define ATOMIC_BOOL_LOCK_FREE __atomic_type_is_lock_free(_Bool)
+#define ATOMIC_CHAR_LOCK_FREE __atomic_type_is_lock_free(char)
+#define ATOMIC_CHAR16_T_LOCK_FREE __atomic_type_is_lock_free(unsigned short)
+#define ATOMIC_CHAR32_T_LOCK_FREE __atomic_type_is_lock_free(unsigned)
+#define ATOMIC_WCHAR_T_LOCK_FREE __atomic_type_is_lock_free(__typeof__(L'\0'))
+#define ATOMIC_SHORT_LOCK_FREE __atomic_type_is_lock_free(short)
+#define ATOMIC_INT_LOCK_FREE __atomic_type_is_lock_free(int)
+#define ATOMIC_LONG_LOCK_FREE __atomic_type_is_lock_free(long)
+#define ATOMIC_LLONG_LOCK_FREE __atomic_type_is_lock_free(long long)
+#define ATOMIC_POINTER_LOCK_FREE __atomic_type_is_lock_free(void *)
+
+#define atomic_store(object, desired) \
+    atomic_store_explicit(object, desired, memory_order_seq_cst)
+#define atomic_load(object) \
+    atomic_load_explicit(object, memory_order_seq_cst)
+
+#define atomic_exchange(object, desired) \
+    atomic_exchange_explicit(object, desired, memory_order_seq_cst)
+#define atomic_compare_exchange_strong(object, expected, desired) \
+    atomic_compare_exchange_strong_explicit(object, expected, desired, \
+                                            memory_order_seq_cst, \
+                                            memory_order_seq_cst)
+#define atomic_compare_exchange_weak(object, expected, desired) \
+    atomic_compare_exchange_weak_explicit(object, expected, desired, \
+                                          memory_order_seq_cst, \
+                                          memory_order_seq_cst)
+
+#define atomic_fetch_add(object, operand) \
+    atomic_fetch_add_explicit(object, operand, memory_order_seq_cst)
+#define atomic_fetch_sub(object, operand) \
+    atomic_fetch_sub_explicit(object, operand, memory_order_seq_cst)
+#define atomic_fetch_or(object, operand) \
+    atomic_fetch_or_explicit(object, operand, memory_order_seq_cst)
+#define atomic_fetch_xor(object, operand) \
+    atomic_fetch_xor_explicit(object, operand, memory_order_seq_cst)
+#define atomic_fetch_and(object, operand) \
+    atomic_fetch_and_explicit(object, operand, memory_order_seq_cst)
+
+typedef _Atomic _Bool atomic_flag;
+
+#define ATOMIC_FLAG_INIT ATOMIC_VAR_INIT(0)
+
+#define atomic_flag_test_and_set(object) \
+    atomic_exchange(object, 1)
+#define atomic_flag_test_and_set_explicit(object, order) \
+    atomic_exchange_explicit(object, 1, order)
+
+#define atomic_flag_clear(object) \
+    atomic_store(object, 0)
+#define atomic_flag_clear_explicit(object, order) \
+    atomic_store_explicit(object, 0, order)
+
+#endif
-- 
2.0.4






^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-09 12:53 [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1 Joakim Sindholt
@ 2014-11-09 17:11 ` Jens Gustedt
  2014-11-22 20:52   ` Joakim Sindholt
  0 siblings, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-09 17:11 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 19554 bytes --]

Hello,
interesting idea!

I am not at all sure how far we should go with such a support, because
if the compiler is claiming to be C11 (which much of them do with
-std=c11) and we don't provide incomplete atomics support, the
application programmer will have difficulties dealing with the cases
and have to do feature tests which are not trivial at all.

The __STD_NO_ATOMICS__ macro is the only feature test macro that is
foreseen by the standard and it leaves no choice: either you have
*all* atomics support (_Atomic qualifier, _Atomic type specifier,
operations on atomic types, atomic type generic functions) or *none*.

I would propose some rules that should be respected by such a header
file:

 - all syntax that this supports should be correct C11 syntax, but it
   may be a restricted version, that is not all C11 might be
   supported. In such a case the compilation should fail.

 - all C11 syntax that was not in C99 and that compiles with this
   header should compile to correct executables with provable atomic
   sematic as defined by C11

 - all produced object code must be ABI compatible with the newer
   versions of the same compiler

Your file doesn't provide support for atomics for the non-basic
types, but which the C11 standard requests? How to deal with them? Do
you plan to implement libatomic.a ? I think that would be a good idea,
because the gcc implementation is bogus and relies on
pthread_mutex_t instead of atomic_flag.

Would we'd have to mimic the different interfaces on a per compiler
base?

Generally, at least for a start, I think it would be a good idea to
separate the support for different compilers, and in particular don't
claim support for a compiler which you didn't even test :)

Having something for earlier versions of clang would already be a good
start.

Some more comments in the text, but I didn't do a complete review,
yet, less did I do tests.

Jens

Am Sonntag, den 09.11.2014, 13:53 +0100 schrieb Joakim Sindholt:
> GCC and Clang ship their own stdatomic.h headers but not for all the
> versions mentioned above. They ship many other standard headers too that
> musl also ships (stdbool/def/int/...).
> 
> In the end musl should be a complete libc with all standard headers and
> this is one of them. It is up to the individual programmer to ensure
> that his or her compiler is properly supported by for example checking
> __STD_NO_ATOMICS__ and __STDC_VERSION__ >= 201112L.

As said above checking these would have to give the assertion that the
platform supports all atomics, library and compiler, which we
shouldn't give lighthearted

> For clang the support is complete for versions >= 3.1. GCC added the
> __atomic functions in 4.7 but not _Atomic until 4.9 so from 4.1 it will
> work correctly so long as you don't use VLAs together with things like
> the ++ operator in arguments and is subject to the same limitations as
> 4.7.

I don't follow here. Could you explain a bit more?

I have the impression that your patch can't work at all with gcc,
since it is missing _Atomic and you need the typedef's with _Atomic in
them. Then, what is the connection with VLA? and the ++ operator?

> From 4.7 it will work as inteded so long as you only use the
> function-like-macros and from 4.9 it will have full support.

I don't think the functionlike macros will work for arbitrary types

> This patch has only been tested with clang. The __typeof__ magic is to
> avoid changes to alltypes. It should be changed before committing this
> to musl. This patch is only for people who want to test themselves.

I would be easier to read if you would provide the changes in
alltypes, as well. Since we are using git, it should not be a problem
to keep all these changes to different files together in one patch.

> This work is released to the Public Domain.
> ---
>  include/stdatomic.h | 287 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 287 insertions(+)
>  create mode 100644 include/stdatomic.h
> 
> diff --git a/include/stdatomic.h b/include/stdatomic.h
> new file mode 100644
> index 0000000..3a61e47
> --- /dev/null
> +++ b/include/stdatomic.h
> @@ -0,0 +1,287 @@
> +#ifndef _STDATOMIC_H
> +#define _STDATOMIC_H
> +
> +#ifndef __has_extension
> +#define __has_extension(x) 0
> +#define __musl_has_extension 1
> +#endif
> +
> +#if defined(__clang__) && __has_extension(c_atomic)
> +
> +#define kill_dependency(y) (y)
> +
> +#define ATOMIC_VAR_INIT(value) (value)
> +#define atomic_init(obj, value) __c11_atomic_init(obj, value)
> +
> +#define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(obj))
> +#define __atomic_type_is_lock_free(type) \
> +    __c11_atomic_is_lock_free(sizeof(type))
> +
> +#define atomic_thread_fence(order) __c11_atomic_thread_fence(order)
> +#define atomic_signal_fence(order) __c11_atomic_signal_fence(order)
> +
> +#define atomic_store_explicit(object, desired, order) \
> +    __c11_atomic_store(object, desired, order)
> +#define atomic_load_explicit(object, order) \
> +    __c11_atomic_load(object, order)
> +
> +#define atomic_exchange_explicit(object, value, order) \
> +    __c11_atomic_exchange(object, value, order)
> +#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
> +                                                success, failure) \
> +    __c11_atomic_compare_exchange_strong(object, expected, desired, \
> +                                         success, failure)
> +#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
> +                                              success, failure) \
> +    __c11_atomic_compare_exchange_weak(object, expected, desired, \
> +                                       success, failure)
> +
> +#define atomic_fetch_add_explicit(object, operand, order) \
> +    __c11_atomic_fetch_add(object, operand, order)
> +#define atomic_fetch_sub_explicit(object, operand, order) \
> +    __c11_atomic_fetch_sub(object, operand, order)
> +#define atomic_fetch_or_explicit(object, operand, order) \
> +    __c11_atomic_fetch_or(object, operand, order)
> +#define atomic_fetch_xor_explicit(object, operand, order) \
> +    __c11_atomic_fetch_xor(object, operand, order)
> +#define atomic_fetch_and_explicit(object, operand, order) \
> +    __c11_atomic_fetch_and(object, operand, order)
> +
> +#elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) || __GNUC__ > 4
> +
> +#define ATOMIC_VAR_INIT(value) (value)
> +#define atomic_init(obj, value) do { *(obj) = (value); } while (0)
> +
> +#if (__GNUC__ == 4 && __GNUC_MINOR__ >= 9) || __GNUC__ > 4
> +#define kill_dependency(y) __extension__({__auto_type __y = (y); __y;})
> +
> +#define atomic_is_lock_free(obj) \
> +    __extension__({ \
> +        __auto_type __obj = (obj); \
> +        __atomic_is_lock_free(sizeof(*__obj), __obj); \
> +    })
> +#else
> +#define __NEED__Atomic
> +
> +#define kill_dependency(y) (y)
> +
> +#define atomic_is_lock_free(obj) \
> +    __atomic_is_lock_free(sizeof(*(obj)), (void *)0)
> +#endif
> +
> +#define __atomic_type_is_lock_free(type) \
> +    __atomic_always_lock_free(sizeof(type), (void *)0)
> +
> +#define atomic_thread_fence(order) __atomic_thread_fence(order)
> +#define atomic_signal_fence(order) __atomic_signal_fence(order)
> +
> +#define atomic_store_explicit(object, value, order) \
> +    __atomic_store_n(object, value, order)
> +#define atomic_load_explicit(object, order) \
> +    __atomic_load_n(object, order)
> +
> +#define atomic_exchange_explicit(object, desired, order) \
> +    __atomic_exchange_n(object, desired, order)
> +#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
> +                                                success, failure) \
> +    __atomic_compare_exchange_n(object, expected, desired, 0, success, failure)
> +#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
> +                                              success, failure) \
> +    __atomic_compare_exchange_n(object, expected, desired, 1, success, failure)
> +
> +#define atomic_fetch_add_explicit(object, operand, order) \
> +    __atomic_fetch_add(object, operand, order)
> +#define atomic_fetch_sub_explicit(object, operand, order) \
> +    __atomic_fetch_sub(object, operand, order)
> +#define atomic_fetch_or_explicit(object, operand, order) \
> +    __atomic_fetch_or(object, operand, order)
> +#define atomic_fetch_xor_explicit(object, operand, order) \
> +    __atomic_fetch_xor(object, operand, order)
> +#define atomic_fetch_and_explicit(object, operand, order) \
> +    __atomic_fetch_and(object, operand, order)
> +
> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 1
> +
> +#define __NEED__Atomic
> +
> +#define kill_dependency(y) (y)
> +
> +#define ATOMIC_VAR_INIT(value) (value)
> +#define atomic_init(obj, value) do { *(obj) = (value); } while (0)
> +
> +#define atomic_is_lock_free(obj) (sizeof(obj) <= sizeof(void *))
> +#define __atomic_type_is_lock_free(type) (sizeof(type) <= sizeof(void *))
> +
> +#define atomic_thread_fence(order) __sync_synchronize()
> +#define atomic_signal_fence(order) __asm__ volatile ("" : : : "memory")
> +
> +/* for GCC < 4.7 some concessions are made. First off, ordering is ignored and
> + * always treated as a full seq_cst barrier.

ok, this is consistent with the standard

> Second, although these are
> + * described as "generic functions" GCC < 4.7 cannot support uses such as:
> + * int n = 4;
> + * atomic_int arr[n];
> + * atomic_int *i = arr;
> + * if (atomic_compare_exchange_strong(i++, ...)) {...
> + * because of the liberal use of the __typeof__ extension.
> + * Furthermore, GCC specified that test_and_set doesn't necessarily support
> + * arbitrary values. It's used as both exchange and store here. Be careful. */

lock_test_and_set is simply not the right tool for these operations,
this should only be used for atomic_flag

> +#define atomic_store_explicit(object, value, order) \
> +    do { \
> +        __sync_lock_test_and_set(object, value); \
> +        __sync_synchronize(); \
> +    while (0)

A missing "}" ?

> +#define atomic_load_explicit(object, order) \
> +    __atomic_fetch_and_add(object, 0)
> +
> +#define atomic_exchange_explicit(object, desired, order) \
> +    __extension__({ \
> +        __typeof__(*(object)) __ret; \
> +        __ret = __sync_lock_test_and_set(object, desired); \
> +        __sync_synchronize(); \
> +        __ret; \
> +    })
> +#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
> +                                                success, failure) \
> +    __extension__({ \
> +        __typeof__(object) __expected = (expected); \
> +        __typeof__(*__expected) __prev; \
> +        _Bool __ret; \
> +        __prev = __sync_val_compare_and_swap(object, *__expected, desired); \
> +        __ret = (__prev == *__expected); \
> +        *__expected = __prev; \
> +        __ret; \
> +    })
> +#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
> +                                              success, failure) \
> +    atomic_compare_exchange_strong_explicit(object, expected, desired, \
> +                                            success, failure)
> +
> +#define atomic_fetch_add_explicit(object, operand, order) \
> +    __sync_fetch_and_add(object, operand)
> +#define atomic_fetch_sub_explicit(object, operand, order) \
> +    __sync_fetch_and_sub(object, operand)
> +#define atomic_fetch_or_explicit(object, operand, order) \
> +    __sync_fetch_and_or(object, operand)
> +#define atomic_fetch_xor_explicit(object, operand, order) \
> +    __sync_fetch_and_xor(object, operand)
> +#define atomic_fetch_and_explicit(object, operand, order) \
> +    __sync_fetch_and_and(object, operand)
> +
> +#else
> +
> +#error "Musl's stdatomic.h does not support your compiler"
> +
> +#endif
> +
> +#ifdef __musl_has_extension
> +#undef __musl_has_extension
> +#undef __has_extension
> +#endif
> +
> +#ifdef __NEED__Atomic
> +#undef __NEED__Atomic
> +#define _Atomic volatile
> +#define volatile(x) x volatile
> +#endif

argh, redefining volatile, no way

this messes the syntax completely think of contexts where () can
appear after volatile. E.g

int volatile (*toto)[5];

is a valid array declaration. Probably there are other such uses of
volatile that would mess up completely.

I suppose that you want to ensure that your _Atomic macro works with
both, the qualifier version and the type specifier. I don't think that
any of this is possible. This is a language feature, and nothing the
library can deal with.

In P99 I made the choice to just support the specifier variant,
definining _Atomic to be a function like macro. First, this is then
still valid C99.

Then, this avoids to give the false impression to the application
programmer that _Atomic qualifiers with all the operations would be
supported.

E.g with your patch something like

_Atomic unsigned counter = ATOMIC_VAR_INIT(0);

...

if (counter++) {
  ... so something ..
} else {
  ... we are the first ...
}

would compile and pass completely unnoticed, but wouldn't implement
atomic semantics at all.

With P99, the eqivalent line with type specifiers

 Atomic(unsigned) counter = ATOMIC_VAR_INIT(0);

would compile, but the line

if (counter++) {

would fail.

In contrast, with the line

if (atomic_fetch_add(&counter, 1)) {

which also is correct C11 would work.

> +typedef enum {
> +    memory_order_relaxed = 0,
> +    memory_order_consume = 1,
> +    memory_order_acquire = 2,
> +    memory_order_release = 3,
> +    memory_order_acq_rel = 4,
> +    memory_order_seq_cst = 5
> +} memory_order;

I'd prefer to provide the corresponding macros as gcc defines them
with #ifdef and then just use these macros here

> +typedef _Atomic _Bool atomic_bool;
> +typedef _Atomic char atomic_char;
> +typedef _Atomic signed char atomic_schar;
> +typedef _Atomic short atomic_short;
> +typedef _Atomic int atomic_int;
> +typedef _Atomic long atomic_long;
> +typedef _Atomic long long atomic_llong;
> +typedef _Atomic unsigned char atomic_uchar;
> +typedef _Atomic unsigned short atomic_ushort;
> +typedef _Atomic unsigned int atomic_uint;
> +typedef _Atomic unsigned long atomic_ulong;
> +typedef _Atomic unsigned long long atomic_ullong;
> +typedef _Atomic unsigned short atomic_char16_t;
> +typedef _Atomic unsigned atomic_char32_t;
> +typedef _Atomic __typeof__(L'\0') atomic_wchar_t;
> +typedef _Atomic signed char atomic_int_least8_t;
> +typedef _Atomic short atomic_int_least16_t;
> +typedef _Atomic int atomic_int_least32_t;
> +typedef _Atomic __typeof__(0x100000000) atomic_int_least64_t;
> +typedef _Atomic signed char atomic_int_fast8_t;
> +typedef _Atomic int atomic_int_fast16_t;
> +typedef _Atomic int atomic_int_fast32_t;
> +typedef _Atomic __typeof__(0x100000000) atomic_int_fast64_t;
> +typedef _Atomic unsigned char atomic_uint_least8_t;
> +typedef _Atomic unsigned short atomic_uint_least16_t;
> +typedef _Atomic unsigned int atomic_uint_least32_t;
> +typedef _Atomic __typeof__(0x100000000U) atomic_uint_least64_t;
> +typedef _Atomic unsigned char atomic_uint_fast8_t;
> +typedef _Atomic unsigned atomic_uint_fast16_t;
> +typedef _Atomic unsigned atomic_uint_fast32_t;
> +typedef _Atomic __typeof__(0x100000000U) atomic_uint_fast64_t;
> +typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_intptr_t;
> +typedef _Atomic __typeof__(sizeof(0)) atomic_uintptr_t;
> +typedef _Atomic __typeof__(sizeof(0)) atomic_size_t_t;
> +typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_ptrdiff_t;
> +typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_intmax_t;
> +typedef _Atomic __typeof__(sizeof(0)) atomic_uintmax_t;
> +
> +#define ATOMIC_BOOL_LOCK_FREE __atomic_type_is_lock_free(_Bool)
> +#define ATOMIC_CHAR_LOCK_FREE __atomic_type_is_lock_free(char)
> +#define ATOMIC_CHAR16_T_LOCK_FREE __atomic_type_is_lock_free(unsigned short)
> +#define ATOMIC_CHAR32_T_LOCK_FREE __atomic_type_is_lock_free(unsigned)
> +#define ATOMIC_WCHAR_T_LOCK_FREE __atomic_type_is_lock_free(__typeof__(L'\0'))
> +#define ATOMIC_SHORT_LOCK_FREE __atomic_type_is_lock_free(short)
> +#define ATOMIC_INT_LOCK_FREE __atomic_type_is_lock_free(int)
> +#define ATOMIC_LONG_LOCK_FREE __atomic_type_is_lock_free(long)
> +#define ATOMIC_LLONG_LOCK_FREE __atomic_type_is_lock_free(long long)
> +#define ATOMIC_POINTER_LOCK_FREE __atomic_type_is_lock_free(void *)
> +
> +#define atomic_store(object, desired) \
> +    atomic_store_explicit(object, desired, memory_order_seq_cst)
> +#define atomic_load(object) \
> +    atomic_load_explicit(object, memory_order_seq_cst)
> +
> +#define atomic_exchange(object, desired) \
> +    atomic_exchange_explicit(object, desired, memory_order_seq_cst)
> +#define atomic_compare_exchange_strong(object, expected, desired) \
> +    atomic_compare_exchange_strong_explicit(object, expected, desired, \
> +                                            memory_order_seq_cst, \
> +                                            memory_order_seq_cst)
> +#define atomic_compare_exchange_weak(object, expected, desired) \
> +    atomic_compare_exchange_weak_explicit(object, expected, desired, \
> +                                          memory_order_seq_cst, \
> +                                          memory_order_seq_cst)
> +
> +#define atomic_fetch_add(object, operand) \
> +    atomic_fetch_add_explicit(object, operand, memory_order_seq_cst)
> +#define atomic_fetch_sub(object, operand) \
> +    atomic_fetch_sub_explicit(object, operand, memory_order_seq_cst)
> +#define atomic_fetch_or(object, operand) \
> +    atomic_fetch_or_explicit(object, operand, memory_order_seq_cst)
> +#define atomic_fetch_xor(object, operand) \
> +    atomic_fetch_xor_explicit(object, operand, memory_order_seq_cst)
> +#define atomic_fetch_and(object, operand) \
> +    atomic_fetch_and_explicit(object, operand, memory_order_seq_cst)
> +
> +typedef _Atomic _Bool atomic_flag;

that has chances to be wrong base type, probably this would be better

typedef _Atomic int atomic_flag;

at least you'd really have to be sure about the ABI that later
versions of the corresponding compiler suppose

> +#define ATOMIC_FLAG_INIT ATOMIC_VAR_INIT(0)
> +
> +#define atomic_flag_test_and_set(object) \
> +    atomic_exchange(object, 1)
> +#define atomic_flag_test_and_set_explicit(object, order) \
> +    atomic_exchange_explicit(object, 1, order)

here also this should definitively chose the ABI that is provided,
namely the __sync_lock_test_and_set builtins.

Operations on atomic_flag must be guaranteed to be lockfree, and the
__sync_lock builtins are the only ones where gcc seem to give such a
guarantee.

> +#define atomic_flag_clear(object) \
> +    atomic_store(object, 0)
> +#define atomic_flag_clear_explicit(object, order) \
> +    atomic_store_explicit(object, 0, order)
> +
> +#endif


-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-09 17:11 ` Jens Gustedt
@ 2014-11-22 20:52   ` Joakim Sindholt
  2014-11-22 23:09     ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Joakim Sindholt @ 2014-11-22 20:52 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 22891 bytes --]

On Sun, 2014-11-09 at 18:11 +0100, Jens Gustedt wrote:
> Hello,
> interesting idea!
> 
> I am not at all sure how far we should go with such a support, because
> if the compiler is claiming to be C11 (which much of them do with
> -std=c11) and we don't provide incomplete atomics support, the
> application programmer will have difficulties dealing with the cases
> and have to do feature tests which are not trivial at all.
> 
> The __STD_NO_ATOMICS__ macro is the only feature test macro that is
> foreseen by the standard and it leaves no choice: either you have
> *all* atomics support (_Atomic qualifier, _Atomic type specifier,
> operations on atomic types, atomic type generic functions) or *none*.
> 
> I would propose some rules that should be respected by such a header
> file:
> 
>  - all syntax that this supports should be correct C11 syntax, but it
>    may be a restricted version, that is not all C11 might be
>    supported. In such a case the compilation should fail.
> 
>  - all C11 syntax that was not in C99 and that compiles with this
>    header should compile to correct executables with provable atomic
>    sematic as defined by C11
> 
>  - all produced object code must be ABI compatible with the newer
>    versions of the same compiler

Agreed.

I'm sorry it's taken so long to respond but I didn't want to do it from
my phone and I haven't been home in a while.

> Your file doesn't provide support for atomics for the non-basic
> types, but which the C11 standard requests? How to deal with them? Do
> you plan to implement libatomic.a ? I think that would be a good idea,
> because the gcc implementation is bogus and relies on
> pthread_mutex_t instead of atomic_flag.

I'm not sure what you mean here? Does __atomic_stuff in GCC not get
translated into assembly level atomics? I just disassembled a program
compiled with this revised version of stdatomic.h and GCC 4.7 and I see
no special calls and plenty of atomic asm.
And I do wish I had the time to go and implement atomic functions for
libc.a but I'm not really sure how to do it right now.

> Would we'd have to mimic the different interfaces on a per compiler
> base?

As it stands right now clang has their __c11_atomic stuff and gcc has
__atomic which doesn't mimic C11 exactly but close enough. Other
compilers will probably provide one or the other but the problem here is
obviously that we might end up trampling on a compiler's own
implementation.
There are 2 reasons I propose this be included in musl at some point
after rigorous testing:
1) clang provides full C11 atomic support as of 3.1 but doesn't ship
stdatomic.h until 3.6, which is not even released yet.
2) It would be ideal if we were to provide stdatomic.h support for
compilers that do not have it built in.
Number 2 has a few problems as it requires generic function-like macros
and I cannot see how to implement it properly without compiler-specific
features. That might be alright too since we can at least support old
GCC.

> Generally, at least for a start, I think it would be a good idea to
> separate the support for different compilers, and in particular don't
> claim support for a compiler which you didn't even test :)

In my defense, I did say it was only for personal testing purposes. I'm
not insane enough to propose committing this to musl without testing.

> Having something for earlier versions of clang would already be a good
> start.
> 
> Some more comments in the text, but I didn't do a complete review,
> yet, less did I do tests.
> 
> Jens
> 
> Am Sonntag, den 09.11.2014, 13:53 +0100 schrieb Joakim Sindholt:
> > GCC and Clang ship their own stdatomic.h headers but not for all the
> > versions mentioned above. They ship many other standard headers too that
> > musl also ships (stdbool/def/int/...).
> > 
> > In the end musl should be a complete libc with all standard headers and
> > this is one of them. It is up to the individual programmer to ensure
> > that his or her compiler is properly supported by for example checking
> > __STD_NO_ATOMICS__ and __STDC_VERSION__ >= 201112L.
> 
> As said above checking these would have to give the assertion that the
> platform supports all atomics, library and compiler, which we
> shouldn't give lighthearted
> 
> > For clang the support is complete for versions >= 3.1. GCC added the
> > __atomic functions in 4.7 but not _Atomic until 4.9 so from 4.1 it will
> > work correctly so long as you don't use VLAs together with things like
> > the ++ operator in arguments and is subject to the same limitations as
> > 4.7.
> 
> I don't follow here. Could you explain a bit more?
> 
> I have the impression that your patch can't work at all with gcc,
> since it is missing _Atomic and you need the typedef's with _Atomic in
> them. Then, what is the connection with VLA? and the ++ operator?

GCC was nice enough to add all the __atomic stuff sans _Atomic for 4.7.
The _Atomic keyword didn't come along until 4.9. So basically you can
trivially implement the function-like macros for 4.7 but you won't have
4.7.

> > From 4.7 it will work as inteded so long as you only use the
> > function-like-macros and from 4.9 it will have full support.
> 
> I don't think the functionlike macros will work for arbitrary types

GCC and Clang both implement the header this way.

> > This patch has only been tested with clang. The __typeof__ magic is to
> > avoid changes to alltypes. It should be changed before committing this
> > to musl. This patch is only for people who want to test themselves.
> 
> I would be easier to read if you would provide the changes in
> alltypes, as well. Since we are using git, it should not be a problem
> to keep all these changes to different files together in one patch.

I know but dalias suggested making it independent of the rest of musl so
people could just drop it into include/ and get testing. I originally
did it with a heap of changes to alltypes.h and if committed it would be
with those changes in place.

> > This work is released to the Public Domain.
> > ---
> >  include/stdatomic.h | 287 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 287 insertions(+)
> >  create mode 100644 include/stdatomic.h
> > 
> > diff --git a/include/stdatomic.h b/include/stdatomic.h
> > new file mode 100644
> > index 0000000..3a61e47
> > --- /dev/null
> > +++ b/include/stdatomic.h
> > @@ -0,0 +1,287 @@
> > +#ifndef _STDATOMIC_H
> > +#define _STDATOMIC_H
> > +
> > +#ifndef __has_extension
> > +#define __has_extension(x) 0
> > +#define __musl_has_extension 1
> > +#endif
> > +
> > +#if defined(__clang__) && __has_extension(c_atomic)
> > +
> > +#define kill_dependency(y) (y)
> > +
> > +#define ATOMIC_VAR_INIT(value) (value)
> > +#define atomic_init(obj, value) __c11_atomic_init(obj, value)
> > +
> > +#define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(obj))
> > +#define __atomic_type_is_lock_free(type) \
> > +    __c11_atomic_is_lock_free(sizeof(type))
> > +
> > +#define atomic_thread_fence(order) __c11_atomic_thread_fence(order)
> > +#define atomic_signal_fence(order) __c11_atomic_signal_fence(order)
> > +
> > +#define atomic_store_explicit(object, desired, order) \
> > +    __c11_atomic_store(object, desired, order)
> > +#define atomic_load_explicit(object, order) \
> > +    __c11_atomic_load(object, order)
> > +
> > +#define atomic_exchange_explicit(object, value, order) \
> > +    __c11_atomic_exchange(object, value, order)
> > +#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
> > +                                                success, failure) \
> > +    __c11_atomic_compare_exchange_strong(object, expected, desired, \
> > +                                         success, failure)
> > +#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
> > +                                              success, failure) \
> > +    __c11_atomic_compare_exchange_weak(object, expected, desired, \
> > +                                       success, failure)
> > +
> > +#define atomic_fetch_add_explicit(object, operand, order) \
> > +    __c11_atomic_fetch_add(object, operand, order)
> > +#define atomic_fetch_sub_explicit(object, operand, order) \
> > +    __c11_atomic_fetch_sub(object, operand, order)
> > +#define atomic_fetch_or_explicit(object, operand, order) \
> > +    __c11_atomic_fetch_or(object, operand, order)
> > +#define atomic_fetch_xor_explicit(object, operand, order) \
> > +    __c11_atomic_fetch_xor(object, operand, order)
> > +#define atomic_fetch_and_explicit(object, operand, order) \
> > +    __c11_atomic_fetch_and(object, operand, order)
> > +
> > +#elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) || __GNUC__ > 4
> > +
> > +#define ATOMIC_VAR_INIT(value) (value)
> > +#define atomic_init(obj, value) do { *(obj) = (value); } while (0)
> > +
> > +#if (__GNUC__ == 4 && __GNUC_MINOR__ >= 9) || __GNUC__ > 4
> > +#define kill_dependency(y) __extension__({__auto_type __y = (y); __y;})
> > +
> > +#define atomic_is_lock_free(obj) \
> > +    __extension__({ \
> > +        __auto_type __obj = (obj); \
> > +        __atomic_is_lock_free(sizeof(*__obj), __obj); \
> > +    })
> > +#else
> > +#define __NEED__Atomic
> > +
> > +#define kill_dependency(y) (y)
> > +
> > +#define atomic_is_lock_free(obj) \
> > +    __atomic_is_lock_free(sizeof(*(obj)), (void *)0)
> > +#endif
> > +
> > +#define __atomic_type_is_lock_free(type) \
> > +    __atomic_always_lock_free(sizeof(type), (void *)0)
> > +
> > +#define atomic_thread_fence(order) __atomic_thread_fence(order)
> > +#define atomic_signal_fence(order) __atomic_signal_fence(order)
> > +
> > +#define atomic_store_explicit(object, value, order) \
> > +    __atomic_store_n(object, value, order)
> > +#define atomic_load_explicit(object, order) \
> > +    __atomic_load_n(object, order)
> > +
> > +#define atomic_exchange_explicit(object, desired, order) \
> > +    __atomic_exchange_n(object, desired, order)
> > +#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
> > +                                                success, failure) \
> > +    __atomic_compare_exchange_n(object, expected, desired, 0, success, failure)
> > +#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
> > +                                              success, failure) \
> > +    __atomic_compare_exchange_n(object, expected, desired, 1, success, failure)
> > +
> > +#define atomic_fetch_add_explicit(object, operand, order) \
> > +    __atomic_fetch_add(object, operand, order)
> > +#define atomic_fetch_sub_explicit(object, operand, order) \
> > +    __atomic_fetch_sub(object, operand, order)
> > +#define atomic_fetch_or_explicit(object, operand, order) \
> > +    __atomic_fetch_or(object, operand, order)
> > +#define atomic_fetch_xor_explicit(object, operand, order) \
> > +    __atomic_fetch_xor(object, operand, order)
> > +#define atomic_fetch_and_explicit(object, operand, order) \
> > +    __atomic_fetch_and(object, operand, order)
> > +
> > +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 1
> > +
> > +#define __NEED__Atomic
> > +
> > +#define kill_dependency(y) (y)
> > +
> > +#define ATOMIC_VAR_INIT(value) (value)
> > +#define atomic_init(obj, value) do { *(obj) = (value); } while (0)
> > +
> > +#define atomic_is_lock_free(obj) (sizeof(obj) <= sizeof(void *))
> > +#define __atomic_type_is_lock_free(type) (sizeof(type) <= sizeof(void *))
> > +
> > +#define atomic_thread_fence(order) __sync_synchronize()
> > +#define atomic_signal_fence(order) __asm__ volatile ("" : : : "memory")
> > +
> > +/* for GCC < 4.7 some concessions are made. First off, ordering is ignored and
> > + * always treated as a full seq_cst barrier.
> 
> ok, this is consistent with the standard
> 
> > Second, although these are
> > + * described as "generic functions" GCC < 4.7 cannot support uses such as:
> > + * int n = 4;
> > + * atomic_int arr[n];
> > + * atomic_int *i = arr;
> > + * if (atomic_compare_exchange_strong(i++, ...)) {...
> > + * because of the liberal use of the __typeof__ extension.
> > + * Furthermore, GCC specified that test_and_set doesn't necessarily support
> > + * arbitrary values. It's used as both exchange and store here. Be careful. */
> 
> lock_test_and_set is simply not the right tool for these operations,
> this should only be used for atomic_flag

I know. It was just a quick way of approximating. It's been removed now
as it didn't make much sense. GCC < 4.7 really should be implemented as
function calls.

> > +#define atomic_store_explicit(object, value, order) \
> > +    do { \
> > +        __sync_lock_test_and_set(object, value); \
> > +        __sync_synchronize(); \
> > +    while (0)
> 
> A missing "}" ?

Fixed.

> > +#define atomic_load_explicit(object, order) \
> > +    __atomic_fetch_and_add(object, 0)
> > +
> > +#define atomic_exchange_explicit(object, desired, order) \
> > +    __extension__({ \
> > +        __typeof__(*(object)) __ret; \
> > +        __ret = __sync_lock_test_and_set(object, desired); \
> > +        __sync_synchronize(); \
> > +        __ret; \
> > +    })
> > +#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
> > +                                                success, failure) \
> > +    __extension__({ \
> > +        __typeof__(object) __expected = (expected); \
> > +        __typeof__(*__expected) __prev; \
> > +        _Bool __ret; \
> > +        __prev = __sync_val_compare_and_swap(object, *__expected, desired); \
> > +        __ret = (__prev == *__expected); \
> > +        *__expected = __prev; \
> > +        __ret; \
> > +    })
> > +#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
> > +                                              success, failure) \
> > +    atomic_compare_exchange_strong_explicit(object, expected, desired, \
> > +                                            success, failure)
> > +
> > +#define atomic_fetch_add_explicit(object, operand, order) \
> > +    __sync_fetch_and_add(object, operand)
> > +#define atomic_fetch_sub_explicit(object, operand, order) \
> > +    __sync_fetch_and_sub(object, operand)
> > +#define atomic_fetch_or_explicit(object, operand, order) \
> > +    __sync_fetch_and_or(object, operand)
> > +#define atomic_fetch_xor_explicit(object, operand, order) \
> > +    __sync_fetch_and_xor(object, operand)
> > +#define atomic_fetch_and_explicit(object, operand, order) \
> > +    __sync_fetch_and_and(object, operand)
> > +
> > +#else
> > +
> > +#error "Musl's stdatomic.h does not support your compiler"
> > +
> > +#endif
> > +
> > +#ifdef __musl_has_extension
> > +#undef __musl_has_extension
> > +#undef __has_extension
> > +#endif
> > +
> > +#ifdef __NEED__Atomic
> > +#undef __NEED__Atomic
> > +#define _Atomic volatile
> > +#define volatile(x) x volatile
> > +#endif
> 
> argh, redefining volatile, no way
> 
> this messes the syntax completely think of contexts where () can
> appear after volatile. E.g
> 
> int volatile (*toto)[5];
> 
> is a valid array declaration. Probably there are other such uses of
> volatile that would mess up completely.
> 
> I suppose that you want to ensure that your _Atomic macro works with
> both, the qualifier version and the type specifier. I don't think that
> any of this is possible. This is a language feature, and nothing the
> library can deal with.
> 
> In P99 I made the choice to just support the specifier variant,
> definining _Atomic to be a function like macro. First, this is then
> still valid C99.
> 
> Then, this avoids to give the false impression to the application
> programmer that _Atomic qualifiers with all the operations would be
> supported.
> 
> E.g with your patch something like
> 
> _Atomic unsigned counter = ATOMIC_VAR_INIT(0);
> 
> ...
> 
> if (counter++) {
>   ... so something ..
> } else {
>   ... we are the first ...
> }
> 
> would compile and pass completely unnoticed, but wouldn't implement
> atomic semantics at all.
> 
> With P99, the eqivalent line with type specifiers
> 
>  Atomic(unsigned) counter = ATOMIC_VAR_INIT(0);
> 
> would compile, but the line
> 
> if (counter++) {
> 
> would fail.
> 
> In contrast, with the line
> 
> if (atomic_fetch_add(&counter, 1)) {
> 
> which also is correct C11 would work.

Noted and changed to an anonymous struct. I realized after sending it
how idiotic #define volatile(x) was.

> > +typedef enum {
> > +    memory_order_relaxed = 0,
> > +    memory_order_consume = 1,
> > +    memory_order_acquire = 2,
> > +    memory_order_release = 3,
> > +    memory_order_acq_rel = 4,
> > +    memory_order_seq_cst = 5
> > +} memory_order;
> 
> I'd prefer to provide the corresponding macros as gcc defines them
> with #ifdef and then just use these macros here

I'd prefer to just keep them as these constants. I would go so far as to
say that this is a valid interpretation of the standard.
http://port70.net/~nsz/c/c11/n1570.html#7.17.3

> > +typedef _Atomic _Bool atomic_bool;
> > +typedef _Atomic char atomic_char;
> > +typedef _Atomic signed char atomic_schar;
> > +typedef _Atomic short atomic_short;
> > +typedef _Atomic int atomic_int;
> > +typedef _Atomic long atomic_long;
> > +typedef _Atomic long long atomic_llong;
> > +typedef _Atomic unsigned char atomic_uchar;
> > +typedef _Atomic unsigned short atomic_ushort;
> > +typedef _Atomic unsigned int atomic_uint;
> > +typedef _Atomic unsigned long atomic_ulong;
> > +typedef _Atomic unsigned long long atomic_ullong;
> > +typedef _Atomic unsigned short atomic_char16_t;
> > +typedef _Atomic unsigned atomic_char32_t;
> > +typedef _Atomic __typeof__(L'\0') atomic_wchar_t;
> > +typedef _Atomic signed char atomic_int_least8_t;
> > +typedef _Atomic short atomic_int_least16_t;
> > +typedef _Atomic int atomic_int_least32_t;
> > +typedef _Atomic __typeof__(0x100000000) atomic_int_least64_t;
> > +typedef _Atomic signed char atomic_int_fast8_t;
> > +typedef _Atomic int atomic_int_fast16_t;
> > +typedef _Atomic int atomic_int_fast32_t;
> > +typedef _Atomic __typeof__(0x100000000) atomic_int_fast64_t;
> > +typedef _Atomic unsigned char atomic_uint_least8_t;
> > +typedef _Atomic unsigned short atomic_uint_least16_t;
> > +typedef _Atomic unsigned int atomic_uint_least32_t;
> > +typedef _Atomic __typeof__(0x100000000U) atomic_uint_least64_t;
> > +typedef _Atomic unsigned char atomic_uint_fast8_t;
> > +typedef _Atomic unsigned atomic_uint_fast16_t;
> > +typedef _Atomic unsigned atomic_uint_fast32_t;
> > +typedef _Atomic __typeof__(0x100000000U) atomic_uint_fast64_t;
> > +typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_intptr_t;
> > +typedef _Atomic __typeof__(sizeof(0)) atomic_uintptr_t;
> > +typedef _Atomic __typeof__(sizeof(0)) atomic_size_t_t;
> > +typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_ptrdiff_t;
> > +typedef _Atomic __typeof__((char *)0 - (char *)0) atomic_intmax_t;
> > +typedef _Atomic __typeof__(sizeof(0)) atomic_uintmax_t;
> > +
> > +#define ATOMIC_BOOL_LOCK_FREE __atomic_type_is_lock_free(_Bool)
> > +#define ATOMIC_CHAR_LOCK_FREE __atomic_type_is_lock_free(char)
> > +#define ATOMIC_CHAR16_T_LOCK_FREE __atomic_type_is_lock_free(unsigned short)
> > +#define ATOMIC_CHAR32_T_LOCK_FREE __atomic_type_is_lock_free(unsigned)
> > +#define ATOMIC_WCHAR_T_LOCK_FREE __atomic_type_is_lock_free(__typeof__(L'\0'))
> > +#define ATOMIC_SHORT_LOCK_FREE __atomic_type_is_lock_free(short)
> > +#define ATOMIC_INT_LOCK_FREE __atomic_type_is_lock_free(int)
> > +#define ATOMIC_LONG_LOCK_FREE __atomic_type_is_lock_free(long)
> > +#define ATOMIC_LLONG_LOCK_FREE __atomic_type_is_lock_free(long long)
> > +#define ATOMIC_POINTER_LOCK_FREE __atomic_type_is_lock_free(void *)
> > +
> > +#define atomic_store(object, desired) \
> > +    atomic_store_explicit(object, desired, memory_order_seq_cst)
> > +#define atomic_load(object) \
> > +    atomic_load_explicit(object, memory_order_seq_cst)
> > +
> > +#define atomic_exchange(object, desired) \
> > +    atomic_exchange_explicit(object, desired, memory_order_seq_cst)
> > +#define atomic_compare_exchange_strong(object, expected, desired) \
> > +    atomic_compare_exchange_strong_explicit(object, expected, desired, \
> > +                                            memory_order_seq_cst, \
> > +                                            memory_order_seq_cst)
> > +#define atomic_compare_exchange_weak(object, expected, desired) \
> > +    atomic_compare_exchange_weak_explicit(object, expected, desired, \
> > +                                          memory_order_seq_cst, \
> > +                                          memory_order_seq_cst)
> > +
> > +#define atomic_fetch_add(object, operand) \
> > +    atomic_fetch_add_explicit(object, operand, memory_order_seq_cst)
> > +#define atomic_fetch_sub(object, operand) \
> > +    atomic_fetch_sub_explicit(object, operand, memory_order_seq_cst)
> > +#define atomic_fetch_or(object, operand) \
> > +    atomic_fetch_or_explicit(object, operand, memory_order_seq_cst)
> > +#define atomic_fetch_xor(object, operand) \
> > +    atomic_fetch_xor_explicit(object, operand, memory_order_seq_cst)
> > +#define atomic_fetch_and(object, operand) \
> > +    atomic_fetch_and_explicit(object, operand, memory_order_seq_cst)
> > +
> > +typedef _Atomic _Bool atomic_flag;
> 
> that has chances to be wrong base type, probably this would be better
> 
> typedef _Atomic int atomic_flag;
> 
> at least you'd really have to be sure about the ABI that later
> versions of the corresponding compiler suppose

The GCC test_and_set function explicitly takes a bool * and both GCC and
Clang implement it as a _Bool. GCC of course makes it significantly more
complicated than that and will on occasion make it an unsigned char but
nonetheless bool is the correct type here.
I have changed it to be an atomic_bool in a struct as both GCC and Clang
has it in a struct. Presumably to separate it from the generic _Atomic
stuff.

> > +#define ATOMIC_FLAG_INIT ATOMIC_VAR_INIT(0)
> > +
> > +#define atomic_flag_test_and_set(object) \
> > +    atomic_exchange(object, 1)
> > +#define atomic_flag_test_and_set_explicit(object, order) \
> > +    atomic_exchange_explicit(object, 1, order)
> 
> here also this should definitively chose the ABI that is provided,
> namely the __sync_lock_test_and_set builtins.
> 
> Operations on atomic_flag must be guaranteed to be lockfree, and the
> __sync_lock builtins are the only ones where gcc seem to give such a
> guarantee.

Noted and changed.

> > +#define atomic_flag_clear(object) \
> > +    atomic_store(object, 0)
> > +#define atomic_flag_clear_explicit(object, order) \
> > +    atomic_store_explicit(object, 0, order)
> > +
> > +#endif

And this time I actually did test it with GCC 4.7. I don't have GCC 4.9
to test with here so I will leave that as an exercise to the reader.

I know this is a lot of crap to sift through but I would really like to
eventually see an stdatomic.h in musl.

-- Joakim



[-- Attachment #2: stdatomic.h --]
[-- Type: text/x-chdr, Size: 11452 bytes --]

#ifndef _STDATOMIC_H
#define _STDATOMIC_H

#ifndef __has_extension
#define __has_extension(x) 0
#define __musl_has_extension 1
#endif

#if defined(__clang__) && __has_extension(c_atomic)

#define kill_dependency(y) (y)

#define ATOMIC_VAR_INIT(value) (value)
#define atomic_init(obj, value) __c11_atomic_init(obj, value)

#define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(*(obj)))
#define __atomic_type_is_lock_free(type) \
    __c11_atomic_is_lock_free(sizeof(type))

#define atomic_thread_fence(order) __c11_atomic_thread_fence(order)
#define atomic_signal_fence(order) __c11_atomic_signal_fence(order)

#define atomic_store_explicit(object, desired, order) \
    __c11_atomic_store(object, desired, order)
#define atomic_load_explicit(object, order) \
    __c11_atomic_load(object, order)

#define atomic_exchange_explicit(object, value, order) \
    __c11_atomic_exchange(object, value, order)
#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
                                                success, failure) \
    __c11_atomic_compare_exchange_strong(object, expected, desired, \
                                         success, failure)
#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
                                              success, failure) \
    __c11_atomic_compare_exchange_weak(object, expected, desired, \
                                       success, failure)

#define atomic_fetch_add_explicit(object, operand, order) \
    __c11_atomic_fetch_add(object, operand, order)
#define atomic_fetch_sub_explicit(object, operand, order) \
    __c11_atomic_fetch_sub(object, operand, order)
#define atomic_fetch_or_explicit(object, operand, order) \
    __c11_atomic_fetch_or(object, operand, order)
#define atomic_fetch_xor_explicit(object, operand, order) \
    __c11_atomic_fetch_xor(object, operand, order)
#define atomic_fetch_and_explicit(object, operand, order) \
    __c11_atomic_fetch_and(object, operand, order)

#define atomic_flag_test_and_set_explicit(object, order) \
    __c11_atomic_exchange(&(object)->__value, 1, order)
#define atomic_flag_clear_explicit(object, order) \
    __c11_atomic_store(&(object)->__value, 0, order)

#elif (__GNUC__ == 4 && __GNUC_MINOR__ >= 9) || __GNUC__ > 4

#define ATOMIC_VAR_INIT(value) (value)
#define atomic_init(obj, value) do { *(obj) = (value); } while (0)

#define kill_dependency(y) __extension__({__auto_type __y = (y); __y;})

#define atomic_is_lock_free(obj) \
    __extension__({ \
        __auto_type __obj = (obj); \
        __atomic_is_lock_free(sizeof(*__obj), __obj); \
    })
#define __atomic_type_is_lock_free(type) \
    __atomic_always_lock_free(sizeof(type), (void *)0)

#define atomic_thread_fence(order) __atomic_thread_fence(order)
#define atomic_signal_fence(order) __atomic_signal_fence(order)

#define atomic_store_explicit(object, value, order) \
    __atomic_store_n(object, value, order)
#define atomic_load_explicit(object, order) \
    __atomic_load_n(object, order)

#define atomic_exchange_explicit(object, desired, order) \
    __atomic_exchange_n(object, desired, order)
#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
                                                success, failure) \
    __atomic_compare_exchange_n(object, expected, desired, 0, success, failure)
#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
                                              success, failure) \
    __atomic_compare_exchange_n(object, expected, desired, 1, success, failure)

#define atomic_fetch_add_explicit(object, operand, order) \
    __atomic_fetch_add(object, operand, order)
#define atomic_fetch_sub_explicit(object, operand, order) \
    __atomic_fetch_sub(object, operand, order)
#define atomic_fetch_or_explicit(object, operand, order) \
    __atomic_fetch_or(object, operand, order)
#define atomic_fetch_xor_explicit(object, operand, order) \
    __atomic_fetch_xor(object, operand, order)
#define atomic_fetch_and_explicit(object, operand, order) \
    __atomic_fetch_and(object, operand, order)

#define atomic_flag_test_and_set_explicit(object, order) \
    __atomic_test_and_set(&(object)->__value, order)
#define atomic_flag_clear_explicit(object, order) \
    __atomic_clear(&(object)->__value, order)

#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 7

#define __NEED__Atomic

#define kill_dependency(y) (y)

#define ATOMIC_VAR_INIT(value) { value }
#define atomic_init(obj, value) do { (obj)->__value = (value); } while (0)

#define atomic_is_lock_free(obj) \
    __atomic_is_lock_free(sizeof((obj)->__value), (void *)0)
#define __atomic_type_is_lock_free(type) \
    __atomic_always_lock_free(sizeof(type), (void *)0)

#define atomic_thread_fence(order) __atomic_thread_fence(order)
#define atomic_signal_fence(order) __atomic_signal_fence(order)

#define atomic_store_explicit(object, value, order) \
    __atomic_store_n(&(object)->__value, value, order)
#define atomic_load_explicit(object, order) \
    __atomic_load_n(&(object)->__value, order)

#define atomic_exchange_explicit(object, desired, order) \
    __atomic_exchange_n(&(object)->__value, desired, order)
#define atomic_compare_exchange_strong_explicit(object, expected, desired, \
                                                success, failure) \
    __atomic_compare_exchange_n(&(object)->__value, expected, desired, \
                                0, success, failure)
#define atomic_compare_exchange_weak_explicit(object, expected, desired, \
                                              success, failure) \
    __atomic_compare_exchange_n(&(object)->__value, expected, desired, \
                                1, success, failure)

#define atomic_fetch_add_explicit(object, operand, order) \
    __atomic_fetch_add(&(object)->__value, operand, order)
#define atomic_fetch_sub_explicit(object, operand, order) \
    __atomic_fetch_sub(&(object)->__value, operand, order)
#define atomic_fetch_or_explicit(object, operand, order) \
    __atomic_fetch_or(&(object)->__value, operand, order)
#define atomic_fetch_xor_explicit(object, operand, order) \
    __atomic_fetch_xor(&(object)->__value, operand, order)
#define atomic_fetch_and_explicit(object, operand, order) \
    __atomic_fetch_and(&(object)->__value, operand, order)

#define atomic_flag_test_and_set_explicit(object, order) \
    __atomic_test_and_set(&(object)->__value.__value, order)
#define atomic_flag_clear_explicit(object, order) \
    __atomic_clear(&(object)->__value.__value, order)

#else

#error "Musl's stdatomic.h does not support your compiler"

#endif

#ifdef __musl_has_extension
#undef __musl_has_extension
#undef __has_extension
#endif

#ifdef __NEED__Atomic
#undef __NEED__Atomic
#define _Atomic(type) struct { type volatile __value; }
#endif

typedef enum {
    memory_order_relaxed = 0,
    memory_order_consume = 1,
    memory_order_acquire = 2,
    memory_order_release = 3,
    memory_order_acq_rel = 4,
    memory_order_seq_cst = 5
} memory_order;

#ifdef __cplusplus
typedef _Atomic(bool) atomic_bool;
#else
typedef _Atomic(_Bool) atomic_bool;
#endif
typedef _Atomic(char) atomic_char;
typedef _Atomic(signed char) atomic_schar;
typedef _Atomic(short) atomic_short;
typedef _Atomic(int) atomic_int;
typedef _Atomic(long) atomic_long;
typedef _Atomic(long long) atomic_llong;
typedef _Atomic(unsigned char) atomic_uchar;
typedef _Atomic(unsigned short) atomic_ushort;
typedef _Atomic(unsigned int) atomic_uint;
typedef _Atomic(unsigned long) atomic_ulong;
typedef _Atomic(unsigned long long) atomic_ullong;
typedef _Atomic(unsigned short) atomic_char16_t;
typedef _Atomic(unsigned) atomic_char32_t;
typedef _Atomic(__typeof__(L'\0')) atomic_wchar_t;
typedef _Atomic(signed char) atomic_int_least8_t;
typedef _Atomic(short) atomic_int_least16_t;
typedef _Atomic(int) atomic_int_least32_t;
typedef _Atomic(__typeof__(0x100000000)) atomic_int_least64_t;
typedef _Atomic(signed char) atomic_int_fast8_t;
typedef _Atomic(int) atomic_int_fast16_t;
typedef _Atomic(int) atomic_int_fast32_t;
typedef _Atomic(__typeof__(0x100000000)) atomic_int_fast64_t;
typedef _Atomic(unsigned char) atomic_uint_least8_t;
typedef _Atomic(unsigned short) atomic_uint_least16_t;
typedef _Atomic(unsigned) atomic_uint_least32_t;
typedef _Atomic(__typeof__(0x100000000U)) atomic_uint_least64_t;
typedef _Atomic(unsigned char) atomic_uint_fast8_t;
typedef _Atomic(unsigned) atomic_uint_fast16_t;
typedef _Atomic(unsigned) atomic_uint_fast32_t;
typedef _Atomic(__typeof__(0x100000000U)) atomic_uint_fast64_t;
typedef _Atomic(__typeof__((char *)0 - (char *)0)) atomic_intptr_t;
typedef _Atomic(__typeof__(sizeof(0))) atomic_uintptr_t;
typedef _Atomic(__typeof__(sizeof(0))) atomic_size_t_t;
typedef _Atomic(__typeof__((char *)0 - (char *)0)) atomic_ptrdiff_t;
typedef _Atomic(__typeof__((char *)0 - (char *)0)) atomic_intmax_t;
typedef _Atomic(__typeof__(sizeof(0))) atomic_uintmax_t;

#ifdef __cplusplus
#define ATOMIC_BOOL_LOCK_FREE __atomic_type_is_lock_free(bool)
#else
#define ATOMIC_BOOL_LOCK_FREE __atomic_type_is_lock_free(_Bool)
#endif
#define ATOMIC_CHAR_LOCK_FREE __atomic_type_is_lock_free(char)
#define ATOMIC_CHAR16_T_LOCK_FREE __atomic_type_is_lock_free(unsigned short)
#define ATOMIC_CHAR32_T_LOCK_FREE __atomic_type_is_lock_free(unsigned)
#define ATOMIC_WCHAR_T_LOCK_FREE __atomic_type_is_lock_free(__typeof__(L'\0'))
#define ATOMIC_SHORT_LOCK_FREE __atomic_type_is_lock_free(short)
#define ATOMIC_INT_LOCK_FREE __atomic_type_is_lock_free(int)
#define ATOMIC_LONG_LOCK_FREE __atomic_type_is_lock_free(long)
#define ATOMIC_LLONG_LOCK_FREE __atomic_type_is_lock_free(long long)
#define ATOMIC_POINTER_LOCK_FREE __atomic_type_is_lock_free(void *)

#define atomic_store(object, desired) \
    atomic_store_explicit(object, desired, memory_order_seq_cst)
#define atomic_load(object) \
    atomic_load_explicit(object, memory_order_seq_cst)

#define atomic_exchange(object, desired) \
    atomic_exchange_explicit(object, desired, memory_order_seq_cst)
#define atomic_compare_exchange_strong(object, expected, desired) \
    atomic_compare_exchange_strong_explicit(object, expected, desired, \
                                            memory_order_seq_cst, \
                                            memory_order_seq_cst)
#define atomic_compare_exchange_weak(object, expected, desired) \
    atomic_compare_exchange_weak_explicit(object, expected, desired, \
                                          memory_order_seq_cst, \
                                          memory_order_seq_cst)

#define atomic_fetch_add(object, operand) \
    atomic_fetch_add_explicit(object, operand, memory_order_seq_cst)
#define atomic_fetch_sub(object, operand) \
    atomic_fetch_sub_explicit(object, operand, memory_order_seq_cst)
#define atomic_fetch_or(object, operand) \
    atomic_fetch_or_explicit(object, operand, memory_order_seq_cst)
#define atomic_fetch_xor(object, operand) \
    atomic_fetch_xor_explicit(object, operand, memory_order_seq_cst)
#define atomic_fetch_and(object, operand) \
    atomic_fetch_and_explicit(object, operand, memory_order_seq_cst)

typedef struct { atomic_bool __value; } atomic_flag;

#define ATOMIC_FLAG_INIT { ATOMIC_VAR_INIT(0) }

#define atomic_flag_test_and_set(object) \
    atomic_flag_test_and_set_explicit(object, memory_order_seq_cst)
#define atomic_flag_clear(object) \
    atomic_flag_clear_explicit(object, memory_order_seq_cst)

#endif



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-22 20:52   ` Joakim Sindholt
@ 2014-11-22 23:09     ` Jens Gustedt
  2014-11-22 23:30       ` Rich Felker
  0 siblings, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-22 23:09 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 8576 bytes --]

Hello,

Am Samstag, den 22.11.2014, 21:52 +0100 schrieb Joakim Sindholt:
> On Sun, 2014-11-09 at 18:11 +0100, Jens Gustedt wrote:
> > I would propose some rules that should be respected by such a header
> > file:
> > 
> >  - all syntax that this supports should be correct C11 syntax, but it
> >    may be a restricted version, that is not all C11 might be
> >    supported. In such a case the compilation should fail.
> > 
> >  - all C11 syntax that was not in C99 and that compiles with this
> >    header should compile to correct executables with provable atomic
> >    sematic as defined by C11
> > 
> >  - all produced object code must be ABI compatible with the newer
> >    versions of the same compiler
> 
> Agreed.
> 
> I'm sorry it's taken so long to respond but I didn't want to do it from
> my phone and I haven't been home in a while.
> 
> > Your file doesn't provide support for atomics for the non-basic
> > types, but which the C11 standard requests? How to deal with them? Do
> > you plan to implement libatomic.a ? I think that would be a good idea,
> > because the gcc implementation is bogus and relies on
> > pthread_mutex_t instead of atomic_flag.
> 
> I'm not sure what you mean here? Does __atomic_stuff in GCC not get
> translated into assembly level atomics? I just disassembled a program
> compiled with this revised version of stdatomic.h and GCC 4.7 and I see
> no special calls and plenty of atomic asm.
> And I do wish I had the time to go and implement atomic functions for
> libc.a but I'm not really sure how to do it right now.

No, not all atomics get translated directly into assembler, some need
auxiliary functions. This is for all types that don't have atomic
support in the processor, the assembler is missing the instruction,
... or the type is simply not a basic type such as a struct or so.

This then usually results in atomic types that are not "lockfree". As
far as I see Gcc implements them with some sort of hash table of locks
that uses the address of the protected object as hash key.

With gcc 4.9 on my machine I have 204 symbols exported by libatomic.a

This hash table uses pthread_mutex_t to lock the access. This is where
I'd like to have a replacement. C mutex's (mtx_t) would already be a
progress, but I actually think that atomic_flag should be enough.

> > Would we'd have to mimic the different interfaces on a per compiler
> > base?
> 
> As it stands right now clang has their __c11_atomic stuff and gcc has
> __atomic which doesn't mimic C11 exactly but close enough. Other
> compilers will probably provide one or the other but the problem here is
> obviously that we might end up trampling on a compiler's own
> implementation.
> There are 2 reasons I propose this be included in musl at some point
> after rigorous testing:
> 1) clang provides full C11 atomic support as of 3.1 but doesn't ship
> stdatomic.h until 3.6, which is not even released yet.
> 2) It would be ideal if we were to provide stdatomic.h support for
> compilers that do not have it built in.
> Number 2 has a few problems as it requires generic function-like macros
> and I cannot see how to implement it properly without compiler-specific
> features. That might be alright too since we can at least support old
> GCC.

In effect, for the similar feature in P99 I did that with gcc'ish
extensions, so it only works for gcc and its cousins (clang, icc).

But even without such stuff you could get things working at least for
the basic types by switching with respect to the size of the base
type. Here only 1, 2, 4, and 8 are interesting, so not a big deal. If
you want to map to assembler directly (in absence of sync_ builtins)
you could probably chose the right word size for the assembler
instruction.

> > Generally, at least for a start, I think it would be a good idea to
> > separate the support for different compilers, and in particular don't
> > claim support for a compiler which you didn't even test :)
> 
> In my defense, I did say it was only for personal testing purposes. I'm
> not insane enough to propose committing this to musl without testing.
> 
> > Having something for earlier versions of clang would already be a good
> > start.
> > 
> > Some more comments in the text, but I didn't do a complete review,
> > yet, less did I do tests.
> > 
> > Jens
> > 
> > Am Sonntag, den 09.11.2014, 13:53 +0100 schrieb Joakim Sindholt:
> > > GCC and Clang ship their own stdatomic.h headers but not for all the
> > > versions mentioned above. They ship many other standard headers too that
> > > musl also ships (stdbool/def/int/...).
> > > 
> > > In the end musl should be a complete libc with all standard headers and
> > > this is one of them. It is up to the individual programmer to ensure
> > > that his or her compiler is properly supported by for example checking
> > > __STD_NO_ATOMICS__ and __STDC_VERSION__ >= 201112L.
> > 
> > As said above checking these would have to give the assertion that the
> > platform supports all atomics, library and compiler, which we
> > shouldn't give lighthearted
> > 
> > > For clang the support is complete for versions >= 3.1. GCC added the
> > > __atomic functions in 4.7 but not _Atomic until 4.9 so from 4.1 it will
> > > work correctly so long as you don't use VLAs together with things like
> > > the ++ operator in arguments and is subject to the same limitations as
> > > 4.7.
> > 
> > I don't follow here. Could you explain a bit more?
> > 
> > I have the impression that your patch can't work at all with gcc,
> > since it is missing _Atomic and you need the typedef's with _Atomic in
> > them. Then, what is the connection with VLA? and the ++ operator?
> 
> GCC was nice enough to add all the __atomic stuff sans _Atomic for 4.7.
> The _Atomic keyword didn't come along until 4.9. So basically you can
> trivially implement the function-like macros for 4.7 but you won't have
> 4.7.

What has all of this to do with VLA? I am lost.

> > > +typedef enum {
> > > +    memory_order_relaxed = 0,
> > > +    memory_order_consume = 1,
> > > +    memory_order_acquire = 2,
> > > +    memory_order_release = 3,
> > > +    memory_order_acq_rel = 4,
> > > +    memory_order_seq_cst = 5
> > > +} memory_order;
> > 
> > I'd prefer to provide the corresponding macros as gcc defines them
> > with #ifdef and then just use these macros here

> I'd prefer to just keep them as these constants.

You really have to be careful here, because this is part of the ABI
and the compiler provides the values for this, so you should use
these.

> I would go so far as to
> say that this is a valid interpretation of the standard.
> http://port70.net/~nsz/c/c11/n1570.html#7.17.3

sure it is a valid interpretation, this is not the question. We don't
live in an isolated world, and so those choices are not ours to make.

> > > +typedef _Atomic _Bool atomic_flag;
> > 
> > that has chances to be wrong base type, probably this would be better
> > 
> > typedef _Atomic int atomic_flag;
> > 
> > at least you'd really have to be sure about the ABI that later
> > versions of the corresponding compiler suppose
> 
> The GCC test_and_set function explicitly takes a bool * and both GCC and
> Clang implement it as a _Bool. GCC of course makes it significantly more
> complicated than that and will on occasion make it an unsigned char but
> nonetheless bool is the correct type here.

I think you missed the point here. It is much, much different than you
describe, I looked it up for gcc. It is a struct that contains such
types. Don't mix these up, typedef to a basic type and typedef to a
struct are two completely different things, they may have very
different sizeof and alignment properties, because of padding.

> I have changed it to be an atomic_bool in a struct as both GCC and Clang
> has it in a struct. Presumably to separate it from the generic _Atomic
> stuff.

Again, since we want to have ABI compatibility, it is not your choice
to make. You'd simply have to stick to the choice that gcc made. So
you have to copy the declaration of the struct, including all the
ifdef fuzz.

Jens


-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::






[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-22 23:09     ` Jens Gustedt
@ 2014-11-22 23:30       ` Rich Felker
  2014-11-23  1:31         ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Rich Felker @ 2014-11-22 23:30 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 12:09:55AM +0100, Jens Gustedt wrote:
> No, not all atomics get translated directly into assembler, some need
> auxiliary functions. This is for all types that don't have atomic
> support in the processor, the assembler is missing the instruction,
> .... or the type is simply not a basic type such as a struct or so.
> 
> This then usually results in atomic types that are not "lockfree". As
> far as I see Gcc implements them with some sort of hash table of locks
> that uses the address of the protected object as hash key.
> 
> With gcc 4.9 on my machine I have 204 symbols exported by libatomic.a
> 
> This hash table uses pthread_mutex_t to lock the access. This is where
> I'd like to have a replacement. C mutex's (mtx_t) would already be a
> progress, but I actually think that atomic_flag should be enough.

atomic_flag is not viable for this because it does not have a
wait/wake mechanism. You'd be spinning, which means in processes with
different priorities involved, you could easily get deadlock if the
lower-priority thread got suspended while holding the lock. You really
do need mutexes.

> > > > For clang the support is complete for versions >= 3.1. GCC added the
> > > > __atomic functions in 4.7 but not _Atomic until 4.9 so from 4.1 it will
> > > > work correctly so long as you don't use VLAs together with things like
> > > > the ++ operator in arguments and is subject to the same limitations as
> > > > 4.7.
> > > 
> > > I don't follow here. Could you explain a bit more?
> > > 
> > > I have the impression that your patch can't work at all with gcc,
> > > since it is missing _Atomic and you need the typedef's with _Atomic in
> > > them. Then, what is the connection with VLA? and the ++ operator?
> > 
> > GCC was nice enough to add all the __atomic stuff sans _Atomic for 4.7.
> > The _Atomic keyword didn't come along until 4.9. So basically you can
> > trivially implement the function-like macros for 4.7 but you won't have
> > 4.7.
> 
> What has all of this to do with VLA? I am lost.

The operands of __typeof__ and sizeof get evaluated when they have VLA
type. I think this is the problem.

> > > > +typedef enum {
> > > > +    memory_order_relaxed = 0,
> > > > +    memory_order_consume = 1,
> > > > +    memory_order_acquire = 2,
> > > > +    memory_order_release = 3,
> > > > +    memory_order_acq_rel = 4,
> > > > +    memory_order_seq_cst = 5
> > > > +} memory_order;
> > > 
> > > I'd prefer to provide the corresponding macros as gcc defines them
> > > with #ifdef and then just use these macros here
> 
> > I'd prefer to just keep them as these constants.
> 
> You really have to be careful here, because this is part of the ABI
> and the compiler provides the values for this, so you should use
> these.

I was the one why requested doing them explicitly. They're part of the
ABI and can't change, so pretending they're something that could
change is unnecessary and obfuscating (IMO).

> > I would go so far as to
> > say that this is a valid interpretation of the standard.
> > http://port70.net/~nsz/c/c11/n1570.html#7.17.3
> 
> sure it is a valid interpretation, this is not the question. We don't
> live in an isolated world, and so those choices are not ours to make.

The choice was made by whichever of GCC and clang first implemented
them and added them to the ABI. Now they're fixed with the values they
were added with.

> > > > +typedef _Atomic _Bool atomic_flag;
> > > 
> > > that has chances to be wrong base type, probably this would be better
> > > 
> > > typedef _Atomic int atomic_flag;
> > > 
> > > at least you'd really have to be sure about the ABI that later
> > > versions of the corresponding compiler suppose
> > 
> > The GCC test_and_set function explicitly takes a bool * and both GCC and
> > Clang implement it as a _Bool. GCC of course makes it significantly more
> > complicated than that and will on occasion make it an unsigned char but
> > nonetheless bool is the correct type here.
> 
> I think you missed the point here. It is much, much different than you
> describe, I looked it up for gcc. It is a struct that contains such
> types. Don't mix these up, typedef to a basic type and typedef to a
> struct are two completely different things, they may have very
> different sizeof and alignment properties, because of padding.

Yes, I recall we looked into this mess dissucsing it on IRC...

> > I have changed it to be an atomic_bool in a struct as both GCC and Clang
> > has it in a struct. Presumably to separate it from the generic _Atomic
> > stuff.
> 
> Again, since we want to have ABI compatibility, it is not your choice
> to make. You'd simply have to stick to the choice that gcc made. So
> you have to copy the declaration of the struct, including all the
> ifdef fuzz.

I'd have to look at it again, but IIRC only one case of the #ifdef
mess was actually possible. The others were for hypothetical archs
without real atomics which we can't support anyway.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-22 23:30       ` Rich Felker
@ 2014-11-23  1:31         ` Jens Gustedt
  2014-11-23  1:43           ` Rich Felker
  0 siblings, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23  1:31 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2596 bytes --]

Hi Rich,

Am Samstag, den 22.11.2014, 18:30 -0500 schrieb Rich Felker:
> atomic_flag is not viable for this because it does not have a
> wait/wake mechanism. You'd be spinning, which means in processes with
> different priorities involved, you could easily get deadlock if the
> lower-priority thread got suspended while holding the lock. You really
> do need mutexes.

I am probably still too much thinking in C11, only, which doesn't have
the notion of priorities.

Actually, I think a specially cooked synchronization tool would be
better. Something that combines an atomic pointer (to point to the
object) with a futex living on it for the waiting. This would probably
be a bit more challenging to implement, but here we really have an
interest to have the fast path really fast, just one CAS of the
pointer.

> > What has all of this to do with VLA? I am lost.
> 
> The operands of __typeof__ and sizeof get evaluated when they have VLA
> type. I think this is the problem.

ah, ok

No, this isn't a problem, I think. Arrays aren't allowed to be subject
of an _Atomic qualification (arrays are never qualified
themselves). For _Atomic type, the standard explicitly excludes
arrays. So arrays in general and VLA in particular should never be
passed as such into any of these generic functions, only pointers to
atomic objects can.

> > > I have changed it to be an atomic_bool in a struct as both GCC and Clang
> > > has it in a struct. Presumably to separate it from the generic _Atomic
> > > stuff.
> > 
> > Again, since we want to have ABI compatibility, it is not your choice
> > to make. You'd simply have to stick to the choice that gcc made. So
> > you have to copy the declaration of the struct, including all the
> > ifdef fuzz.
> 
> I'd have to look at it again, but IIRC only one case of the #ifdef
> mess was actually possible. The others were for hypothetical archs
> without real atomics which we can't support anyway.

We should have it as a struct, if the implementations have it like
that, I think:

 - It should have same alignment properties for ABI compatibility.
 - It should lead to the same typename when included in C++.

The ifdef is a single one to switch between _Bool or unsigned char or
so.

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::





[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23  1:31         ` Jens Gustedt
@ 2014-11-23  1:43           ` Rich Felker
  2014-11-23  1:47             ` Joakim Sindholt
  2014-11-23  8:49             ` Jens Gustedt
  0 siblings, 2 replies; 23+ messages in thread
From: Rich Felker @ 2014-11-23  1:43 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 02:31:35AM +0100, Jens Gustedt wrote:
> Hi Rich,
> 
> Am Samstag, den 22.11.2014, 18:30 -0500 schrieb Rich Felker:
> > atomic_flag is not viable for this because it does not have a
> > wait/wake mechanism. You'd be spinning, which means in processes with
> > different priorities involved, you could easily get deadlock if the
> > lower-priority thread got suspended while holding the lock. You really
> > do need mutexes.
> 
> I am probably still too much thinking in C11, only, which doesn't have
> the notion of priorities.
> 
> Actually, I think a specially cooked synchronization tool would be
> better. Something that combines an atomic pointer (to point to the
> object) with a futex living on it for the waiting. This would probably
> be a bit more challenging to implement, but here we really have an
> interest to have the fast path really fast, just one CAS of the
> pointer.

I don't get what you mean. To access an atomic object larger than the
hardware supports, you have to hold a lock for the whole interval of
reading/writing. This is O(n) in the size of the object. I don't see
where your ideas about pointers and CAS are coming in.

> > > What has all of this to do with VLA? I am lost.
> > 
> > The operands of __typeof__ and sizeof get evaluated when they have VLA
> > type. I think this is the problem.
> 
> ah, ok
> 
> No, this isn't a problem, I think. Arrays aren't allowed to be subject
> of an _Atomic qualification (arrays are never qualified
> themselves). For _Atomic type, the standard explicitly excludes
> arrays. So arrays in general and VLA in particular should never be
> passed as such into any of these generic functions, only pointers to
> atomic objects can.

Is a pointer to a variably modified type considered variably modified?
If so maybe these are affected too...

> > > > I have changed it to be an atomic_bool in a struct as both GCC and Clang
> > > > has it in a struct. Presumably to separate it from the generic _Atomic
> > > > stuff.
> > > 
> > > Again, since we want to have ABI compatibility, it is not your choice
> > > to make. You'd simply have to stick to the choice that gcc made. So
> > > you have to copy the declaration of the struct, including all the
> > > ifdef fuzz.
> > 
> > I'd have to look at it again, but IIRC only one case of the #ifdef
> > mess was actually possible. The others were for hypothetical archs
> > without real atomics which we can't support anyway.
> 
> We should have it as a struct, if the implementations have it like
> that, I think:
> 
>  - It should have same alignment properties for ABI compatibility.
>  - It should lead to the same typename when included in C++.

Yes.

> The ifdef is a single one to switch between _Bool or unsigned char or
> so.

Yes, but I think the #ifdef always comes out one way anyway, though I
don't remember which one and don't have the file in front of me.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23  1:43           ` Rich Felker
@ 2014-11-23  1:47             ` Joakim Sindholt
  2014-11-23  2:42               ` Rich Felker
  2014-11-23  9:43               ` Jens Gustedt
  2014-11-23  8:49             ` Jens Gustedt
  1 sibling, 2 replies; 23+ messages in thread
From: Joakim Sindholt @ 2014-11-23  1:47 UTC (permalink / raw)
  To: musl

On Sat, 2014-11-22 at 20:43 -0500, Rich Felker wrote:
> On Sun, Nov 23, 2014 at 02:31:35AM +0100, Jens Gustedt wrote:
> > Hi Rich,
> > 
> > Am Samstag, den 22.11.2014, 18:30 -0500 schrieb Rich Felker:
> > > atomic_flag is not viable for this because it does not have a
> > > wait/wake mechanism. You'd be spinning, which means in processes with
> > > different priorities involved, you could easily get deadlock if the
> > > lower-priority thread got suspended while holding the lock. You really
> > > do need mutexes.
> > 
> > I am probably still too much thinking in C11, only, which doesn't have
> > the notion of priorities.
> > 
> > Actually, I think a specially cooked synchronization tool would be
> > better. Something that combines an atomic pointer (to point to the
> > object) with a futex living on it for the waiting. This would probably
> > be a bit more challenging to implement, but here we really have an
> > interest to have the fast path really fast, just one CAS of the
> > pointer.
> 
> I don't get what you mean. To access an atomic object larger than the
> hardware supports, you have to hold a lock for the whole interval of
> reading/writing. This is O(n) in the size of the object. I don't see
> where your ideas about pointers and CAS are coming in.
> 
> > > > What has all of this to do with VLA? I am lost.
> > > 
> > > The operands of __typeof__ and sizeof get evaluated when they have VLA
> > > type. I think this is the problem.
> > 
> > ah, ok
> > 
> > No, this isn't a problem, I think. Arrays aren't allowed to be subject
> > of an _Atomic qualification (arrays are never qualified
> > themselves). For _Atomic type, the standard explicitly excludes
> > arrays. So arrays in general and VLA in particular should never be
> > passed as such into any of these generic functions, only pointers to
> > atomic objects can.
> 
> Is a pointer to a variably modified type considered variably modified?
> If so maybe these are affected too...
> 
> > > > > I have changed it to be an atomic_bool in a struct as both GCC and Clang
> > > > > has it in a struct. Presumably to separate it from the generic _Atomic
> > > > > stuff.
> > > > 
> > > > Again, since we want to have ABI compatibility, it is not your choice
> > > > to make. You'd simply have to stick to the choice that gcc made. So
> > > > you have to copy the declaration of the struct, including all the
> > > > ifdef fuzz.
> > > 
> > > I'd have to look at it again, but IIRC only one case of the #ifdef
> > > mess was actually possible. The others were for hypothetical archs
> > > without real atomics which we can't support anyway.
> > 
> > We should have it as a struct, if the implementations have it like
> > that, I think:
> > 
> >  - It should have same alignment properties for ABI compatibility.
> >  - It should lead to the same typename when included in C++.
> 
> Yes.
> 
> > The ifdef is a single one to switch between _Bool or unsigned char or
> > so.
> 
> Yes, but I think the #ifdef always comes out one way anyway, though I
> don't remember which one and don't have the file in front of me.

GCC 4.9:

typedef _Atomic struct
{
#if __GCC_ATOMIC_TEST_AND_SET_TRUEVAL == 1
  _Bool __val;
#else
  unsigned char __val;
#endif
} atomic_flag;

Clang 3.6:

#ifdef __cplusplus
typedef _Atomic(bool)               atomic_bool;
#else
typedef _Atomic(_Bool)              atomic_bool;
#endif

typedef struct atomic_flag { atomic_bool _Value; } atomic_flag;






^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23  1:47             ` Joakim Sindholt
@ 2014-11-23  2:42               ` Rich Felker
  2014-11-23  9:43               ` Jens Gustedt
  1 sibling, 0 replies; 23+ messages in thread
From: Rich Felker @ 2014-11-23  2:42 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 02:47:12AM +0100, Joakim Sindholt wrote:
> > > > > > I have changed it to be an atomic_bool in a struct as both GCC and Clang
> > > > > > has it in a struct. Presumably to separate it from the generic _Atomic
> > > > > > stuff.
> > > > > 
> > > > > Again, since we want to have ABI compatibility, it is not your choice
> > > > > to make. You'd simply have to stick to the choice that gcc made. So
> > > > > you have to copy the declaration of the struct, including all the
> > > > > ifdef fuzz.
> > > > 
> > > > I'd have to look at it again, but IIRC only one case of the #ifdef
> > > > mess was actually possible. The others were for hypothetical archs
> > > > without real atomics which we can't support anyway.
> > > 
> > > We should have it as a struct, if the implementations have it like
> > > that, I think:
> > > 
> > >  - It should have same alignment properties for ABI compatibility.
> > >  - It should lead to the same typename when included in C++.
> > 
> > Yes.
> > 
> > > The ifdef is a single one to switch between _Bool or unsigned char or
> > > so.
> > 
> > Yes, but I think the #ifdef always comes out one way anyway, though I
> > don't remember which one and don't have the file in front of me.
> 
> GCC 4.9:
> 
> typedef _Atomic struct
> {
> #if __GCC_ATOMIC_TEST_AND_SET_TRUEVAL == 1
>   _Bool __val;
> #else
>   unsigned char __val;
> #endif
> } atomic_flag;
> 
> Clang 3.6:
> 
> #ifdef __cplusplus
> typedef _Atomic(bool)               atomic_bool;
> #else
> typedef _Atomic(_Bool)              atomic_bool;
> #endif
> 
> typedef struct atomic_flag { atomic_bool _Value; } atomic_flag;

So yes, only the true case of the #if is needed, and that's what clang
implements. The other is for hypothetical archs which lack the ability
to CAS 0 and 1 into an object of type _Bool -- for example, perhaps
they can do a lock-free boolean test-and-set, but only with values 0
and -1, whereas _Bool needs to store 1 for true. This is just a guess
what the GCC folks might have had in mind, but in any case, it doesn't
matter, since we assume a full CAS (and require a kernel that emulates
one if the hardware does not have one).

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23  1:43           ` Rich Felker
  2014-11-23  1:47             ` Joakim Sindholt
@ 2014-11-23  8:49             ` Jens Gustedt
  2014-11-23 15:06               ` Rich Felker
  1 sibling, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23  8:49 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 3565 bytes --]

Hello,

Am Samstag, den 22.11.2014, 20:43 -0500 schrieb Rich Felker:
> On Sun, Nov 23, 2014 at 02:31:35AM +0100, Jens Gustedt wrote:
> > Actually, I think a specially cooked synchronization tool would be
> > better. Something that combines an atomic pointer (to point to the
> > object) with a futex living on it for the waiting. This would probably
> > be a bit more challenging to implement, but here we really have an
> > interest to have the fast path really fast, just one CAS of the
> > pointer.
> 
> I don't get what you mean. To access an atomic object larger than the
> hardware supports, you have to hold a lock for the whole interval of
> reading/writing.

No, why do you think that? If you implement access to a critical
resource through a mutex, you only need one mutex and not several
ones. The association to the whole range of the resource is only
logical.

Basically there are two possibilities to implement lockful atomics on
types that don't support atomics directly. One is to construct a
struct around it that also contains the lock/mutex. It has the
disadvantage that e.g sizeof for the _Atomic type and the underlying
type are different. The second one is to realize the locks in a sort
of hash table as I tried to describe.

gcc chose the table approach, so we are stuck with it.

In the case we have here, we can always assume that the type of the
object that we are protecting is fixed and in particular its size is
fixed. So all threads will see the object with the same size and will
only ask for a lock with the start adress of the object. (Otherwise we
have UB.)

This is why C requires that atomicity is integrated in the type, so
you can't ask for atomic operations of arbitrary objects.

Thinking of it, there might be an unintended loophole in the
standard, due to a difference in _Atomic as a qualifier and as
specifier. The qualifier version seems to permit to be applied to a
struct that itself contains other _Atomic types. This then would not
work with the table approach. I'll investigate.

> This is O(n) in the size of the object.

This would be prohibitive, indeed. Luckily we don't need that, so only
the copy operation is O(n), and not the lock.

> I don't see
> where your ideas about pointers and CAS are coming in.



> > > > What has all of this to do with VLA? I am lost.
> > > 
> > > The operands of __typeof__ and sizeof get evaluated when they have VLA
> > > type. I think this is the problem.
> > 
> > ah, ok
> > 
> > No, this isn't a problem, I think. Arrays aren't allowed to be subject
> > of an _Atomic qualification (arrays are never qualified
> > themselves). For _Atomic type, the standard explicitly excludes
> > arrays. So arrays in general and VLA in particular should never be
> > passed as such into any of these generic functions, only pointers to
> > atomic objects can.
> 
> Is a pointer to a variably modified type considered variably modified?

yes

> If so maybe these are affected too...

no, the pointers that can be passed to the atomic "functions" are
always pointers to atomic objects, so they can't be arrays (and so
VLA) themselves, nor can an atomic struct containt a VLA or a pointer
to VLA.

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23  1:47             ` Joakim Sindholt
  2014-11-23  2:42               ` Rich Felker
@ 2014-11-23  9:43               ` Jens Gustedt
  2014-11-23 15:21                 ` Rich Felker
  1 sibling, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23  9:43 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

Hello,

Am Sonntag, den 23.11.2014, 02:47 +0100 schrieb Joakim Sindholt:
> GCC 4.9:
> 
> typedef _Atomic struct
> {
> #if __GCC_ATOMIC_TEST_AND_SET_TRUEVAL == 1
>   _Bool __val;
> #else
>   unsigned char __val;
> #endif
> } atomic_flag;
> 
> Clang 3.6:
> 
> #ifdef __cplusplus
> typedef _Atomic(bool)               atomic_bool;
> #else
> typedef _Atomic(_Bool)              atomic_bool;
> #endif
> 
> typedef struct atomic_flag { atomic_bool _Value; } atomic_flag;

So they fucked it up and have incompatible declarations? Great.

Please, first of all don't look at it for the #ifdef things, this is
secondary, in particular if it never triggers in real life (but I
think it does), see below.

These two declarations are incompatible and having to compilation
units one compiled with gcc and one with clang that are linked
together, leads to UB:

 - the structures should end up to have the same layout, but here one
   has a qualified field and the other not, so not the same types.

 - for struct types to be compatible, the struct tag must be the same

For the discussion about the second case for the type, this is the
question if there are archs that implement TAS operations with other
values than 0 for "unset" and 1 for "set". There seem to be archs out
there that implement TAS with other values, I vaguely remember having
heard about some risk arch (??). Actually this also is the reason why
the standard defines this type in such a weird manner, and why per the
standard it needs a dedicated initialization macro, default
initialization with 0 doesn't do in all cases.

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23  8:49             ` Jens Gustedt
@ 2014-11-23 15:06               ` Rich Felker
  2014-11-23 16:18                 ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Rich Felker @ 2014-11-23 15:06 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 09:49:03AM +0100, Jens Gustedt wrote:
> Hello,
> 
> Am Samstag, den 22.11.2014, 20:43 -0500 schrieb Rich Felker:
> > On Sun, Nov 23, 2014 at 02:31:35AM +0100, Jens Gustedt wrote:
> > > Actually, I think a specially cooked synchronization tool would be
> > > better. Something that combines an atomic pointer (to point to the
> > > object) with a futex living on it for the waiting. This would probably
> > > be a bit more challenging to implement, but here we really have an
> > > interest to have the fast path really fast, just one CAS of the
> > > pointer.
> > 
> > I don't get what you mean. To access an atomic object larger than the
> > hardware supports, you have to hold a lock for the whole interval of
> > reading/writing.
> 
> No, why do you think that? If you implement access to a critical
> resource through a mutex, you only need one mutex and not several
> ones. The association to the whole range of the resource is only
> logical.

I think my statement was just unclear. I don't mean that the lock has
to be associated with the whole interval in memory. I mean the lock
has to be held for the whole interval in time. This makes it clear
that a spinlock is not appropriate even if you don't have the deadlock
issue to worry about from priority scheduling.

> Thinking of it, there might be an unintended loophole in the
> standard, due to a difference in _Atomic as a qualifier and as
> specifier. The qualifier version seems to permit to be applied to a
> struct that itself contains other _Atomic types. This then would not
> work with the table approach. I'll investigate.

Accessing individual members of atomic structs is UB last I checked.
IMO it's rather stupid that the standard allowed atomic structs
anyway, but we're stuck with them.

> > This is O(n) in the size of the object.
> 
> This would be prohibitive, indeed. Luckily we don't need that, so only
> the copy operation is O(n), and not the lock.

O(n) time the lock is held, not O(n) time obtaining locks. Sorry.

> > I don't see
> > where your ideas about pointers and CAS are coming in.

I'm still confused about this.

> > > > > What has all of this to do with VLA? I am lost.
> > > > 
> > > > The operands of __typeof__ and sizeof get evaluated when they have VLA
> > > > type. I think this is the problem.
> > > 
> > > ah, ok
> > > 
> > > No, this isn't a problem, I think. Arrays aren't allowed to be subject
> > > of an _Atomic qualification (arrays are never qualified
> > > themselves). For _Atomic type, the standard explicitly excludes
> > > arrays. So arrays in general and VLA in particular should never be
> > > passed as such into any of these generic functions, only pointers to
> > > atomic objects can.
> > 
> > Is a pointer to a variably modified type considered variably modified?
> 
> yes
> 
> > If so maybe these are affected too...
> 
> no, the pointers that can be passed to the atomic "functions" are
> always pointers to atomic objects, so they can't be arrays (and so
> VLA) themselves, nor can an atomic struct containt a VLA or a pointer
> to VLA.

_Atomic int (*pmat)[n];

Then &pmat is a pointer to a valid atomic type, an atomic pointer to a
VLA type.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23  9:43               ` Jens Gustedt
@ 2014-11-23 15:21                 ` Rich Felker
  2014-11-23 16:29                   ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Rich Felker @ 2014-11-23 15:21 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 10:43:34AM +0100, Jens Gustedt wrote:
> Hello,
> 
> Am Sonntag, den 23.11.2014, 02:47 +0100 schrieb Joakim Sindholt:
> > GCC 4.9:
> > 
> > typedef _Atomic struct
> > {
> > #if __GCC_ATOMIC_TEST_AND_SET_TRUEVAL == 1
> >   _Bool __val;
> > #else
> >   unsigned char __val;
> > #endif
> > } atomic_flag;
> > 
> > Clang 3.6:
> > 
> > #ifdef __cplusplus
> > typedef _Atomic(bool)               atomic_bool;
> > #else
> > typedef _Atomic(_Bool)              atomic_bool;
> > #endif
> > 
> > typedef struct atomic_flag { atomic_bool _Value; } atomic_flag;
> 
> So they fucked it up and have incompatible declarations? Great.

The differences are minor, and are things that are generally
considered non-issues when linking code from two different
compilers/implementations of the language which share an ABI. The only
differences are the name of the member (__val vs _Value) and whether
its type is atomic-qualified. I agree there's room for different
interpretations, but my view is that when you're linking code produced
by different compilers that have agreed on an ABI, matching the
size/representation/layout is generally sufficient.

> For the discussion about the second case for the type, this is the
> question if there are archs that implement TAS operations with other
> values than 0 for "unset" and 1 for "set". There seem to be archs out
> there that implement TAS with other values, I vaguely remember having
> heard about some risk arch (??). Actually this also is the reason why
> the standard defines this type in such a weird manner, and why per the
> standard it needs a dedicated initialization macro, default
> initialization with 0 doesn't do in all cases.

These are not archs we can support with musl, so they wouldn't be
relevant. And they're not archs that could support POSIX without a
kernel stop-the-world approach for implementing CAS, or syscalls for
every synchronization action.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 15:06               ` Rich Felker
@ 2014-11-23 16:18                 ` Jens Gustedt
  2014-11-23 16:37                   ` Rich Felker
  0 siblings, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23 16:18 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]

Am Sonntag, den 23.11.2014, 10:06 -0500 schrieb Rich Felker:
> > > I don't see
> > > where your ideas about pointers and CAS are coming in.
> 
> I'm still confused about this.

I was thinking of storing the address of the object within the lock
structure such that we can detect a collision. But perhaps that's not
such a good idea anyhow.

> > > > > > What has all of this to do with VLA? I am lost.
> > > > > 
> > > > > The operands of __typeof__ and sizeof get evaluated when they have VLA
> > > > > type. I think this is the problem.
> > > > 
> > > > ah, ok
> > > > 
> > > > No, this isn't a problem, I think. Arrays aren't allowed to be subject
> > > > of an _Atomic qualification (arrays are never qualified
> > > > themselves). For _Atomic type, the standard explicitly excludes
> > > > arrays. So arrays in general and VLA in particular should never be
> > > > passed as such into any of these generic functions, only pointers to
> > > > atomic objects can.
> > > 
> > > Is a pointer to a variably modified type considered variably modified?
> > 
> > yes
> > 
> > > If so maybe these are affected too...
> > 
> > no, the pointers that can be passed to the atomic "functions" are
> > always pointers to atomic objects, so they can't be arrays (and so
> > VLA) themselves, nor can an atomic struct containt a VLA or a pointer
> > to VLA.
> 
> _Atomic int (*pmat)[n];
> 
> Then &pmat is a pointer to a valid atomic type, an atomic pointer to a
> VLA type.

I don't see that. Ain't that a pointer to a VLA of base type _Atomic
int?

The beast that you describe would be

int (*_Atomic pmat)[n];

and effectively this would be an atomic pointer to VM type. But
concerning size, atomicity etc this is just a pointer type like any
other. (The size information doesn't change, once it is initialized.)
And concerning the atomic functions this is not different to

int (*_Atomic pmat)[];

Could you describe a bit more, where you think that there is a problem
with such a thing?

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 15:21                 ` Rich Felker
@ 2014-11-23 16:29                   ` Jens Gustedt
  2014-11-23 16:38                     ` Rich Felker
  0 siblings, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23 16:29 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

Am Sonntag, den 23.11.2014, 10:21 -0500 schrieb Rich Felker:
> On Sun, Nov 23, 2014 at 10:43:34AM +0100, Jens Gustedt wrote:
> > Hello,
> > 
> > Am Sonntag, den 23.11.2014, 02:47 +0100 schrieb Joakim Sindholt:
> > > GCC 4.9:
> > > 
> > > typedef _Atomic struct
> > > {
> > > #if __GCC_ATOMIC_TEST_AND_SET_TRUEVAL == 1
> > >   _Bool __val;
> > > #else
> > >   unsigned char __val;
> > > #endif
> > > } atomic_flag;
> > > 
> > > Clang 3.6:
> > > 
> > > #ifdef __cplusplus
> > > typedef _Atomic(bool)               atomic_bool;
> > > #else
> > > typedef _Atomic(_Bool)              atomic_bool;
> > > #endif
> > > 
> > > typedef struct atomic_flag { atomic_bool _Value; } atomic_flag;
> > 
> > So they fucked it up and have incompatible declarations? Great.
> 
> The differences are minor, and are things that are generally
> considered non-issues when linking code from two different
> compilers/implementations of the language which share an ABI. The only
> differences are the name of the member (__val vs _Value) and whether
> its type is atomic-qualified. I agree there's room for different
> interpretations, but my view is that when you're linking code produced
> by different compilers that have agreed on an ABI, matching the
> size/representation/layout is generally sufficient.

I wouldn't exactly call that "have agreed on an ABI", but be it. Since
this behavior is undefined by the standard, they'd have to document
that as implementation defined behavior.

> > For the discussion about the second case for the type, this is the
> > question if there are archs that implement TAS operations with other
> > values than 0 for "unset" and 1 for "set". There seem to be archs out
> > there that implement TAS with other values, I vaguely remember having
> > heard about some risk arch (??). Actually this also is the reason why
> > the standard defines this type in such a weird manner, and why per the
> > standard it needs a dedicated initialization macro, default
> > initialization with 0 doesn't do in all cases.
> 
> These are not archs we can support with musl, so they wouldn't be
> relevant. And they're not archs that could support POSIX without a
> kernel stop-the-world approach for implementing CAS, or syscalls for
> every synchronization action.

Could you be more specific? Is it that you know that all the arch in
question and conclude about their behavior from you knowledge about
them?

Without more specific information I don't see any reason, that an arch
that has such specialized super-fast TAS with weird values couldn't
have a CAS that behaves "normal". But then I also don't see any reason
for such a TAS design in any case ...

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 16:18                 ` Jens Gustedt
@ 2014-11-23 16:37                   ` Rich Felker
  2014-11-23 18:01                     ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Rich Felker @ 2014-11-23 16:37 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 05:18:17PM +0100, Jens Gustedt wrote:
> > > > > > > What has all of this to do with VLA? I am lost.
> > > > > > 
> > > > > > The operands of __typeof__ and sizeof get evaluated when they have VLA
> > > > > > type. I think this is the problem.
> > > > > 
> > > > > ah, ok
> > > > > 
> > > > > No, this isn't a problem, I think. Arrays aren't allowed to be subject
> > > > > of an _Atomic qualification (arrays are never qualified
> > > > > themselves). For _Atomic type, the standard explicitly excludes
> > > > > arrays. So arrays in general and VLA in particular should never be
> > > > > passed as such into any of these generic functions, only pointers to
> > > > > atomic objects can.
> > > > 
> > > > Is a pointer to a variably modified type considered variably modified?
> > > 
> > > yes
> > > 
> > > > If so maybe these are affected too...
> > > 
> > > no, the pointers that can be passed to the atomic "functions" are
> > > always pointers to atomic objects, so they can't be arrays (and so
> > > VLA) themselves, nor can an atomic struct containt a VLA or a pointer
> > > to VLA.
> > 
> > _Atomic int (*pmat)[n];
> > 
> > Then &pmat is a pointer to a valid atomic type, an atomic pointer to a
> > VLA type.
> 
> I don't see that. Ain't that a pointer to a VLA of base type _Atomic
> int?
> 
> The beast that you describe would be
> 
> int (*_Atomic pmat)[n];

Yes, I messed it up. Sorry.

> and effectively this would be an atomic pointer to VM type. But
> concerning size, atomicity etc this is just a pointer type like any
> other. (The size information doesn't change, once it is initialized.)
> And concerning the atomic functions this is not different to
> 
> int (*_Atomic pmat)[];
> 
> Could you describe a bit more, where you think that there is a problem
> with such a thing?

But it's a variably-modified type, so sizeof and typeof evaluate the
expression when their operand is of this type. Thus, all you have to
do is make an array of objects of this type, and then refer to
arr[i++]. This will be an expression with variably-modified type, and
the evaluation will have unwanted side effects.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 16:29                   ` Jens Gustedt
@ 2014-11-23 16:38                     ` Rich Felker
  2014-11-23 17:05                       ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Rich Felker @ 2014-11-23 16:38 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 05:29:03PM +0100, Jens Gustedt wrote:
> > > For the discussion about the second case for the type, this is the
> > > question if there are archs that implement TAS operations with other
> > > values than 0 for "unset" and 1 for "set". There seem to be archs out
> > > there that implement TAS with other values, I vaguely remember having
> > > heard about some risk arch (??). Actually this also is the reason why
> > > the standard defines this type in such a weird manner, and why per the
> > > standard it needs a dedicated initialization macro, default
> > > initialization with 0 doesn't do in all cases.
> > 
> > These are not archs we can support with musl, so they wouldn't be
> > relevant. And they're not archs that could support POSIX without a
> > kernel stop-the-world approach for implementing CAS, or syscalls for
> > every synchronization action.
> 
> Could you be more specific? Is it that you know that all the arch in
> question and conclude about their behavior from you knowledge about
> them?
> 
> Without more specific information I don't see any reason, that an arch
> that has such specialized super-fast TAS with weird values couldn't
> have a CAS that behaves "normal". But then I also don't see any reason
> for such a TAS design in any case ...

If you have a CAS, you don't use the broken TAS to implement TAS. You
just use the CAS.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 16:38                     ` Rich Felker
@ 2014-11-23 17:05                       ` Jens Gustedt
  2014-11-23 17:29                         ` stephen Turner
  2014-11-23 19:38                         ` Rich Felker
  0 siblings, 2 replies; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23 17:05 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

Am Sonntag, den 23.11.2014, 11:38 -0500 schrieb Rich Felker:
> On Sun, Nov 23, 2014 at 05:29:03PM +0100, Jens Gustedt wrote:
> > > > For the discussion about the second case for the type, this is the
> > > > question if there are archs that implement TAS operations with other
> > > > values than 0 for "unset" and 1 for "set". There seem to be archs out
> > > > there that implement TAS with other values, I vaguely remember having
> > > > heard about some risk arch (??). Actually this also is the reason why
> > > > the standard defines this type in such a weird manner, and why per the
> > > > standard it needs a dedicated initialization macro, default
> > > > initialization with 0 doesn't do in all cases.
> > > 
> > > These are not archs we can support with musl, so they wouldn't be
> > > relevant. And they're not archs that could support POSIX without a
> > > kernel stop-the-world approach for implementing CAS, or syscalls for
> > > every synchronization action.
> > 
> > Could you be more specific? Is it that you know that all the arch in
> > question and conclude about their behavior from you knowledge about
> > them?
> > 
> > Without more specific information I don't see any reason, that an arch
> > that has such specialized super-fast TAS with weird values couldn't
> > have a CAS that behaves "normal". But then I also don't see any reason
> > for such a TAS design in any case ...
> 
> If you have a CAS, you don't use the broken TAS to implement TAS. You
> just use the CAS.

We wouldn't do that, if we'd have a choice, sure. But, if gcc decides
to use the super-fast TAS that has other values, you'd give up binary
compatibility?

Jens


-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 17:05                       ` Jens Gustedt
@ 2014-11-23 17:29                         ` stephen Turner
  2014-11-23 19:38                         ` Rich Felker
  1 sibling, 0 replies; 23+ messages in thread
From: stephen Turner @ 2014-11-23 17:29 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

It sounds like the ideal path would be to rebuild a compiler to posix
standards? If I understand your conversation at all your saying that
niether the clang or gcc compilers are ideal for the standard. Perhaps tcc
or gcc etc, has a version which could lend itself both in licensing and
core functionality to be a good starting point for the standard?

[-- Attachment #2: Type: text/html, Size: 369 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 16:37                   ` Rich Felker
@ 2014-11-23 18:01                     ` Jens Gustedt
  2014-11-23 19:39                       ` Rich Felker
  0 siblings, 1 reply; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23 18:01 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

Am Sonntag, den 23.11.2014, 11:37 -0500 schrieb Rich Felker:
> But it's a variably-modified type, so sizeof and typeof evaluate the
> expression when their operand is of this type.  Thus, all you have
> to do is make an array of objects of this type, and then refer to
> arr[i++]. This will be an expression with variably-modified type,

yes, but not a VLA

> and the evaluation will have unwanted side effects.

No, I don't think it is evaluated, 6.5.3.4 says:

  If the type of the operand is a variable length array type, the
  operand is evaluated;

observe that it says VLA, and not VM type.

  otherwise, the operand is not evaluated and the result is an integer
  constant.

There are no exception for other VM types. To deduce the size for our
case here, an evaluation isn't necessary.

(Hoping that the __typeof__ stuff follows the same rules.)

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 17:05                       ` Jens Gustedt
  2014-11-23 17:29                         ` stephen Turner
@ 2014-11-23 19:38                         ` Rich Felker
  1 sibling, 0 replies; 23+ messages in thread
From: Rich Felker @ 2014-11-23 19:38 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 06:05:07PM +0100, Jens Gustedt wrote:
> Am Sonntag, den 23.11.2014, 11:38 -0500 schrieb Rich Felker:
> > On Sun, Nov 23, 2014 at 05:29:03PM +0100, Jens Gustedt wrote:
> > > > > For the discussion about the second case for the type, this is the
> > > > > question if there are archs that implement TAS operations with other
> > > > > values than 0 for "unset" and 1 for "set". There seem to be archs out
> > > > > there that implement TAS with other values, I vaguely remember having
> > > > > heard about some risk arch (??). Actually this also is the reason why
> > > > > the standard defines this type in such a weird manner, and why per the
> > > > > standard it needs a dedicated initialization macro, default
> > > > > initialization with 0 doesn't do in all cases.
> > > > 
> > > > These are not archs we can support with musl, so they wouldn't be
> > > > relevant. And they're not archs that could support POSIX without a
> > > > kernel stop-the-world approach for implementing CAS, or syscalls for
> > > > every synchronization action.
> > > 
> > > Could you be more specific? Is it that you know that all the arch in
> > > question and conclude about their behavior from you knowledge about
> > > them?
> > > 
> > > Without more specific information I don't see any reason, that an arch
> > > that has such specialized super-fast TAS with weird values couldn't
> > > have a CAS that behaves "normal". But then I also don't see any reason
> > > for such a TAS design in any case ...
> > 
> > If you have a CAS, you don't use the broken TAS to implement TAS. You
> > just use the CAS.
> 
> We wouldn't do that, if we'd have a choice, sure. But, if gcc decides
> to use the super-fast TAS that has other values, you'd give up binary
> compatibility?

This is a hypothetical situation that doesn't exist, and since it
doesn't exist now and there's no logical reason for it to be
introduced in the future on new archs, it's not worth adding
complexity over.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 18:01                     ` Jens Gustedt
@ 2014-11-23 19:39                       ` Rich Felker
  2014-11-23 23:30                         ` Jens Gustedt
  0 siblings, 1 reply; 23+ messages in thread
From: Rich Felker @ 2014-11-23 19:39 UTC (permalink / raw)
  To: musl

On Sun, Nov 23, 2014 at 07:01:24PM +0100, Jens Gustedt wrote:
> Am Sonntag, den 23.11.2014, 11:37 -0500 schrieb Rich Felker:
> > But it's a variably-modified type, so sizeof and typeof evaluate the
> > expression when their operand is of this type.  Thus, all you have
> > to do is make an array of objects of this type, and then refer to
> > arr[i++]. This will be an expression with variably-modified type,
> 
> yes, but not a VLA
> 
> > and the evaluation will have unwanted side effects.
> 
> No, I don't think it is evaluated, 6.5.3.4 says:
> 
>   If the type of the operand is a variable length array type, the
>   operand is evaluated;
> 
> observe that it says VLA, and not VM type.
> 
>   otherwise, the operand is not evaluated and the result is an integer
>   constant.
> 
> There are no exception for other VM types. To deduce the size for our
> case here, an evaluation isn't necessary.
> 
> (Hoping that the __typeof__ stuff follows the same rules.)

OK, thanks for clarifying this. In that case I think we're safe as
long as __typeof__ follows the same rules.

Rich


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1
  2014-11-23 19:39                       ` Rich Felker
@ 2014-11-23 23:30                         ` Jens Gustedt
  0 siblings, 0 replies; 23+ messages in thread
From: Jens Gustedt @ 2014-11-23 23:30 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2563 bytes --]

Am Sonntag, den 23.11.2014, 14:39 -0500 schrieb Rich Felker:
> On Sun, Nov 23, 2014 at 07:01:24PM +0100, Jens Gustedt wrote:
> > No, I don't think it is evaluated, 6.5.3.4 says:
> > 
> >   If the type of the operand is a variable length array type, the
> >   operand is evaluated;
> > 
> > observe that it says VLA, and not VM type.
> > 
> >   otherwise, the operand is not evaluated and the result is an integer
> >   constant.
> > 
> > There are no exception for other VM types. To deduce the size for our
> > case here, an evaluation isn't necessary.
> > 
> > (Hoping that the __typeof__ stuff follows the same rules.)
> 
> OK, thanks for clarifying this. In that case I think we're safe as
> long as __typeof__ follows the same rules.

Bad news, it seems that typeof does in fact evaluate VM types in typeof:

https://gcc.gnu.org/ml/gcc-patches/2013-11/msg01378.html

I am not much convinced that it is necessary to have that rule, if it
is not a VLA. For any value expression, the necessary type information
should be available without evaluation of the expression. (Type
expressions are different, but we don't need that, here.) But anyhow,
we can't change that.

So the only way to get away with it to support all allowed atomic
types would be to use the __auto_type extension that they introduce in
that patch. That would only allow us to provide support for recent
gcc.

For older gcc, say from the point where they have the __sync builtins,
we could still

 - support all types with sizes that fit to the natively supported
   instruction set, usually 1, 2, 4 and 8, by juggling with sizeof
   expressions.
 - Only allow the atomic type specifier version "_Atomic(typename)"
   and not the qualifier version "_Atomic typename"
 - Only have the atomic generic macros to use these types, no
   operators allowed.
 - Don't support atomic struct

One way to do this would be

#define _Atomic(T) __typeof__(__typeof__(T) volatile[1])

e.g the result for int would be equivalent to

typedef int volatile atomic_int[1];

This would still have it binary compatible with future version that
really have _Atomic as a keyword. On the other hand, it would not
allow to assign such a variable.

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-11-23 23:30 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-09 12:53 [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1 Joakim Sindholt
2014-11-09 17:11 ` Jens Gustedt
2014-11-22 20:52   ` Joakim Sindholt
2014-11-22 23:09     ` Jens Gustedt
2014-11-22 23:30       ` Rich Felker
2014-11-23  1:31         ` Jens Gustedt
2014-11-23  1:43           ` Rich Felker
2014-11-23  1:47             ` Joakim Sindholt
2014-11-23  2:42               ` Rich Felker
2014-11-23  9:43               ` Jens Gustedt
2014-11-23 15:21                 ` Rich Felker
2014-11-23 16:29                   ` Jens Gustedt
2014-11-23 16:38                     ` Rich Felker
2014-11-23 17:05                       ` Jens Gustedt
2014-11-23 17:29                         ` stephen Turner
2014-11-23 19:38                         ` Rich Felker
2014-11-23  8:49             ` Jens Gustedt
2014-11-23 15:06               ` Rich Felker
2014-11-23 16:18                 ` Jens Gustedt
2014-11-23 16:37                   ` Rich Felker
2014-11-23 18:01                     ` Jens Gustedt
2014-11-23 19:39                       ` Rich Felker
2014-11-23 23:30                         ` Jens Gustedt

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).