[FFmpeg-devel] [PATCH] compat/atomics/win32: improve similarity to stdatomic.h

[Frank Plowman <post@frankplowman.com>]

Some preliminary info:
* The win32 atomic compatibility header is based on VLC's (c91e72ed52). This patch makes the header more in line with what it was before they got rid of this way of doing things: https://code.videolan.org/videolan/vlc/-/blob/ce150f3849cebe97bc7fc028674d3c7f8c73f64d/include/vlc_atomic.h
* The Windows API does not support atomic operations on 8-bit types and only has functions for atomic operations on 16-bit types on Windows Desktop.
* There is nowhere in the FFmpeg codebase which currently uses 8- or 16-bit atomic types

This patch:
* Makes atomic types the same size as their non-atomic counterparts. Previously, all atomic types were intptr_t to get around the lack of 8- and 16-bit atomic operations. This could lead to overreads. Now, each atomic type is the same size as its non-atomic counterpart, in line with the C11 specification.
* Dispatches Windows API functions based on operand size to avoid overwrites. Now there are a variety of sizes of atomic types, the correct Windows API function must be selected based on the object's sizes. Feedback is appreciated on how best to handle the failure case in the atomic_helper macro.
* Removes 8- and 16-bit types not supported by all versions of winnt.h. None of these were being used anywhere in FFmpeg. As these cannot be operated on atomically, they have been removed. Alternatives to this recommended.

(PS: This is my first submitted patch, please be nice although I'm sure you will)

Signed-off-by: Frank Plowman <post@frankplowman.com>

---
 compat/atomics/win32/stdatomic.h | 104 +++++++++++--------------------
 1 file changed, 36 insertions(+), 68 deletions(-)

diff --git a/compat/atomics/win32/stdatomic.h b/compat/atomics/win32/stdatomic.h
index 28a627bfd3..a0475fda7d 100644
--- a/compat/atomics/win32/stdatomic.h
+++ b/compat/atomics/win32/stdatomic.h
@@ -43,42 +43,26 @@ do {                            \
 
 #define atomic_is_lock_free(obj) 0
 
-typedef intptr_t atomic_flag;
-typedef intptr_t atomic_bool;
-typedef intptr_t atomic_char;
-typedef intptr_t atomic_schar;
-typedef intptr_t atomic_uchar;
-typedef intptr_t atomic_short;
-typedef intptr_t atomic_ushort;
-typedef intptr_t atomic_int;
-typedef intptr_t atomic_uint;
-typedef intptr_t atomic_long;
-typedef intptr_t atomic_ulong;
-typedef intptr_t atomic_llong;
-typedef intptr_t atomic_ullong;
-typedef intptr_t atomic_wchar_t;
-typedef intptr_t atomic_int_least8_t;
-typedef intptr_t atomic_uint_least8_t;
-typedef intptr_t atomic_int_least16_t;
-typedef intptr_t atomic_uint_least16_t;
-typedef intptr_t atomic_int_least32_t;
-typedef intptr_t atomic_uint_least32_t;
-typedef intptr_t atomic_int_least64_t;
-typedef intptr_t atomic_uint_least64_t;
-typedef intptr_t atomic_int_fast8_t;
-typedef intptr_t atomic_uint_fast8_t;
-typedef intptr_t atomic_int_fast16_t;
-typedef intptr_t atomic_uint_fast16_t;
-typedef intptr_t atomic_int_fast32_t;
-typedef intptr_t atomic_uint_fast32_t;
-typedef intptr_t atomic_int_fast64_t;
-typedef intptr_t atomic_uint_fast64_t;
-typedef intptr_t atomic_intptr_t;
-typedef intptr_t atomic_uintptr_t;
-typedef intptr_t atomic_size_t;
-typedef intptr_t atomic_ptrdiff_t;
-typedef intptr_t atomic_intmax_t;
-typedef intptr_t atomic_uintmax_t;
+typedef          int       atomic_int;
+typedef unsigned int       atomic_uint;
+typedef          long      atomic_long;
+typedef unsigned long      atomic_ulong;
+typedef          long long atomic_llong;
+typedef unsigned long long atomic_ullong;
+typedef      int_least32_t atomic_int_least32_t;
+typedef     uint_least32_t atomic_uint_least32_t;
+typedef      int_least64_t atomic_int_least64_t;
+typedef     uint_least64_t atomic_uint_least64_t;
+typedef       int_fast32_t atomic_int_fast32_t;
+typedef      uint_fast32_t atomic_uint_fast32_t;
+typedef       int_fast64_t atomic_int_fast64_t;
+typedef      uint_fast64_t atomic_uint_fast64_t;
+typedef           intptr_t atomic_intptr_t;
+typedef          uintptr_t atomic_uintptr_t;
+typedef             size_t atomic_size_t;
+typedef          ptrdiff_t atomic_ptrdiff_t;
+typedef           intmax_t atomic_intmax_t;
+typedef          uintmax_t atomic_uintmax_t;
 
 #define atomic_store(object, desired)   \
 do {                                    \
@@ -95,20 +79,21 @@ do {                                    \
 #define atomic_load_explicit(object, order) \
     atomic_load(object)
 
+#define atomic_helper(operation, object, ...)                  \
+    (sizeof(*object) == 4 ?                                    \
+        operation((volatile LONG *) object, __VA_ARGS__)       \
+    : sizeof(*object) == 8 ?                                   \
+        operation##64((volatile LONG64 *) object, __VA_ARGS__) \
+    : (abort(), 0))
+
 #define atomic_exchange(object, desired) \
-    InterlockedExchangePointer((PVOID volatile *)object, (PVOID)desired)
+    atomic_helper(InterlockedExchange, object, desired)
 
 #define atomic_exchange_explicit(object, desired, order) \
     atomic_exchange(object, desired)
 
-static inline int atomic_compare_exchange_strong(intptr_t *object, intptr_t *expected,
-                                                 intptr_t desired)
-{
-    intptr_t old = *expected;
-    *expected = (intptr_t)InterlockedCompareExchangePointer(
-        (PVOID *)object, (PVOID)desired, (PVOID)old);
-    return *expected == old;
-}
+#define atomic_compare_exchange_strong(object, expected, desired) \
+    atomic_helper(InterlockedCompareExchange, object, desired, *expected)
 
 #define atomic_compare_exchange_strong_explicit(object, expected, desired, success, failure) \
     atomic_compare_exchange_strong(object, expected, desired)
@@ -119,37 +104,20 @@ static inline int atomic_compare_exchange_strong(intptr_t *object, intptr_t *exp
 #define atomic_compare_exchange_weak_explicit(object, expected, desired, success, failure) \
     atomic_compare_exchange_weak(object, expected, desired)
 
-#ifdef _WIN64
-#define atomic_fetch_add(object, operand) \
-    InterlockedExchangeAdd64(object, operand)
-
-#define atomic_fetch_sub(object, operand) \
-    InterlockedExchangeAdd64(object, -(operand))
-
-#define atomic_fetch_or(object, operand) \
-    InterlockedOr64(object, operand)
-
-#define atomic_fetch_xor(object, operand) \
-    InterlockedXor64(object, operand)
-
-#define atomic_fetch_and(object, operand) \
-    InterlockedAnd64(object, operand)
-#else
 #define atomic_fetch_add(object, operand) \
-    InterlockedExchangeAdd(object, operand)
+    atomic_helper(InterlockedExchangeAdd, object, operand)
 
-#define atomic_fetch_sub(object, operand) \
-    InterlockedExchangeAdd(object, -(operand))
+#define atomic_fetch_sub(object, operand)    \
+    atomic_fetch_add(object, -(operand))
 
 #define atomic_fetch_or(object, operand) \
-    InterlockedOr(object, operand)
+    atomic_helper(InterlockedOr, object, operand)
 
 #define atomic_fetch_xor(object, operand) \
-    InterlockedXor(object, operand)
+    atomic_helper(InterlockedXor, object, operand)
 
 #define atomic_fetch_and(object, operand) \
-    InterlockedAnd(object, operand)
-#endif /* _WIN64 */
+    atomic_helper(InterlockedAnd, object, operand)
 
 #define atomic_fetch_add_explicit(object, operand, order) \
     atomic_fetch_add(object, operand)
-- 
2.40.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".