[FFmpeg-devel,2/3] avutil/atomic: Add avpriv_atomic_int64_cas()

Submitted by Michael Niedermayer on Nov. 21, 2016, 3:18 p.m.

Details

Message ID 20161121151858.703-2-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Nov. 21, 2016, 3:18 p.m.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavutil/atomic.c     | 20 ++++++++++++++++++++
 libavutil/atomic.h     | 10 ++++++++++
 libavutil/atomic_gcc.h | 12 ++++++++++++
 3 files changed, 42 insertions(+)

Comments

James Almer Nov. 21, 2016, 3:49 p.m.
On 11/21/2016 12:18 PM, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavutil/atomic.c     | 20 ++++++++++++++++++++
>  libavutil/atomic.h     | 10 ++++++++++
>  libavutil/atomic_gcc.h | 12 ++++++++++++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/libavutil/atomic.c b/libavutil/atomic.c
> index 64cff25..27561ad 100644
> --- a/libavutil/atomic.c
> +++ b/libavutil/atomic.c
> @@ -70,6 +70,17 @@ void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
>      return ret;
>  }
>  
> +int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval)

There's a bunch of libav commits in queue to replace the entire custom atomic
API and use instead the standard C11 API.
The C11 functions are generic and are defined for all atomic types regardless
of size, so this commit and its new private-but-exported symbol can be avoided.

Since said commits are a bit far away in the queue, we could cherry pick them
now and noop them later.
They are 13f5d2b (C11 stdatomics.h configure check), 4e928ef, c275586, bb81ed4,
f9a6a80, eb34d40 (fallback implementations for non-C11 compilers), followed by
a bunch of commits replacing the current usage of the custom API with the C11
one.
A last commit removes the current API altogether, but i don't think we can do
that until a major bump takes place.


> +{
> +    int64_t ret;
> +    pthread_mutex_lock(&atomic_lock);
> +    ret = *ptr;
> +    if (ret == oldval)
> +        *ptr = newval;
> +    pthread_mutex_unlock(&atomic_lock);
> +    return ret;
> +}
> +
>  #elif !HAVE_THREADS
>  
>  int avpriv_atomic_int_get(volatile int *ptr)
> @@ -97,6 +108,15 @@ void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
>      return *ptr;
>  }
>  
> +int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval)
> +{
> +    if (*ptr == oldval) {
> +        *ptr = newval;
> +        return oldval;
> +    }
> +    return *ptr;
> +}
> +
>  #else /* HAVE_THREADS */
>  
>  /* This should never trigger, unless a new threading implementation
> diff --git a/libavutil/atomic.h b/libavutil/atomic.h
> index 15906d2..cabc4f0 100644
> --- a/libavutil/atomic.h
> +++ b/libavutil/atomic.h
> @@ -74,6 +74,16 @@ int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc);
>   */
>  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval);
>  
> +/**
> + * Atomic int64_t compare and swap.
> + *
> + * @param ptr pointer to the int64_t to operate on
> + * @param oldval do the swap if the current value of *ptr equals to oldval
> + * @param newval value to replace *ptr with
> + * @return the value of *ptr before comparison
> + */
> +int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval);
> +
>  #endif /* HAVE_ATOMICS_NATIVE */
>  
>  #endif /* AVUTIL_ATOMIC_H */
> diff --git a/libavutil/atomic_gcc.h b/libavutil/atomic_gcc.h
> index 5f9fc49..c7e0dba 100644
> --- a/libavutil/atomic_gcc.h
> +++ b/libavutil/atomic_gcc.h
> @@ -75,4 +75,16 @@ static inline void *atomic_ptr_cas_gcc(void * volatile *ptr,
>  #endif
>  }
>  
> +#define avpriv_atomic_int64_cas atomic_int64_cas_gcc
> +static inline int64_t atomic_int64_cas_gcc(int64_t volatile *ptr,
> +                                           int64_t oldval, int64_t newval)
> +{
> +#if HAVE_SYNC_VAL_COMPARE_AND_SWAP
> +    return __sync_val_compare_and_swap(ptr, oldval, newval);
> +#else
> +    __atomic_compare_exchange_n(ptr, &oldval, newval, 0, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
> +    return oldval;
> +#endif
> +}
> +
>  #endif /* AVUTIL_ATOMIC_GCC_H */
>
Carl Eugen Hoyos Nov. 22, 2016, 1:20 a.m.
2016-11-21 16:49 GMT+01:00 James Almer <jamrial@gmail.com>:

>> +int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval)
>
> There's a bunch of libav commits in queue to replace the entire custom atomic
> API and use instead the standard C11 API.

What does that mean for pre-C11 compilers?

Carl Eugen
James Almer Nov. 22, 2016, 1:30 a.m.
On 11/21/2016 10:20 PM, Carl Eugen Hoyos wrote:
> 2016-11-21 16:49 GMT+01:00 James Almer <jamrial@gmail.com>:
> 
>>> +int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval)
>>
>> There's a bunch of libav commits in queue to replace the entire custom atomic
>> API and use instead the standard C11 API.
> 
> What does that mean for pre-C11 compilers?
> 
> Carl Eugen

As i said in a following paragraph, the set includes fallback compat
implementations for non C11 compilers. The result is similar to how
threading is handled on non-pthreads targets.

Patch hide | download patch | download mbox

diff --git a/libavutil/atomic.c b/libavutil/atomic.c
index 64cff25..27561ad 100644
--- a/libavutil/atomic.c
+++ b/libavutil/atomic.c
@@ -70,6 +70,17 @@  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
     return ret;
 }
 
+int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval)
+{
+    int64_t ret;
+    pthread_mutex_lock(&atomic_lock);
+    ret = *ptr;
+    if (ret == oldval)
+        *ptr = newval;
+    pthread_mutex_unlock(&atomic_lock);
+    return ret;
+}
+
 #elif !HAVE_THREADS
 
 int avpriv_atomic_int_get(volatile int *ptr)
@@ -97,6 +108,15 @@  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
     return *ptr;
 }
 
+int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval)
+{
+    if (*ptr == oldval) {
+        *ptr = newval;
+        return oldval;
+    }
+    return *ptr;
+}
+
 #else /* HAVE_THREADS */
 
 /* This should never trigger, unless a new threading implementation
diff --git a/libavutil/atomic.h b/libavutil/atomic.h
index 15906d2..cabc4f0 100644
--- a/libavutil/atomic.h
+++ b/libavutil/atomic.h
@@ -74,6 +74,16 @@  int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc);
  */
 void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval);
 
+/**
+ * Atomic int64_t compare and swap.
+ *
+ * @param ptr pointer to the int64_t to operate on
+ * @param oldval do the swap if the current value of *ptr equals to oldval
+ * @param newval value to replace *ptr with
+ * @return the value of *ptr before comparison
+ */
+int64_t avpriv_atomic_int64_cas(int64_t volatile *ptr, int64_t oldval, int64_t newval);
+
 #endif /* HAVE_ATOMICS_NATIVE */
 
 #endif /* AVUTIL_ATOMIC_H */
diff --git a/libavutil/atomic_gcc.h b/libavutil/atomic_gcc.h
index 5f9fc49..c7e0dba 100644
--- a/libavutil/atomic_gcc.h
+++ b/libavutil/atomic_gcc.h
@@ -75,4 +75,16 @@  static inline void *atomic_ptr_cas_gcc(void * volatile *ptr,
 #endif
 }
 
+#define avpriv_atomic_int64_cas atomic_int64_cas_gcc
+static inline int64_t atomic_int64_cas_gcc(int64_t volatile *ptr,
+                                           int64_t oldval, int64_t newval)
+{
+#if HAVE_SYNC_VAL_COMPARE_AND_SWAP
+    return __sync_val_compare_and_swap(ptr, oldval, newval);
+#else
+    __atomic_compare_exchange_n(ptr, &oldval, newval, 0, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
+    return oldval;
+#endif
+}
+
 #endif /* AVUTIL_ATOMIC_GCC_H */