From fa4e34667c6f90b4a24d708caac6ac4d8f25fa48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 6 Jun 2022 21:30:41 +0200 Subject: [PATCH] gatomic: Add APIs to perform atomic int / pointer exchanges Atomic APIs provide a way to exchange values only if we compare a value that is equal to the old value, but not to just exchange the value returning the old one. However, compilers provide such built-in functions, so we can use them to expose such functionality to GLib. The only drawback is that when using an old version of gcc not providing atomic APIs to swap values, we need to re-implement it with an implementation that may not be fully atomic, but that is safe enough. However this codepath should really not be used currently as gcc introduced __atomic_exchange_n() at version 4.7.4, so 8 years ago. --- docs/reference/glib/glib-sections.txt | 2 + glib/gatomic.c | 94 +++++++++++++++++++++++++++ glib/gatomic.h | 62 ++++++++++++++++++ glib/tests/atomic.c | 30 +++++++++ glib/tests/cxx.cpp | 35 ++++++++++ meson.build | 12 ++++ 6 files changed, 235 insertions(+) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 399dfd466..c1abf1aee 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -1273,6 +1273,7 @@ g_atomic_int_set g_atomic_int_inc g_atomic_int_dec_and_test g_atomic_int_compare_and_exchange +g_atomic_int_exchange g_atomic_int_add g_atomic_int_and g_atomic_int_or @@ -1282,6 +1283,7 @@ g_atomic_int_xor g_atomic_pointer_get g_atomic_pointer_set g_atomic_pointer_compare_and_exchange +g_atomic_pointer_exchange g_atomic_pointer_add g_atomic_pointer_and g_atomic_pointer_or diff --git a/glib/gatomic.c b/glib/gatomic.c index b317601b5..4771d449d 100644 --- a/glib/gatomic.c +++ b/glib/gatomic.c @@ -218,6 +218,31 @@ gboolean return g_atomic_int_compare_and_exchange (atomic, oldval, newval); } +/** + * g_atomic_int_exchange: + * @atomic: a pointer to a #gint or #guint + * @newval: the value to replace with + * + * Sets the @atomic to @newval and returns the old value from @atomic. + * + * This exchange is done atomically. + * + * Think of this operation as an atomic version of + * `{ tmp = *atomic; *atomic = val; return tmp; }`. + * + * This call acts as a full compiler and hardware memory barrier. + * + * Returns: the value of @atomic before the exchange, signed + * + * Since: 2.74 + **/ +gint +(g_atomic_int_exchange) (gint *atomic, + gint newval) +{ + return g_atomic_int_exchange (atomic, newval); +} + /** * g_atomic_int_add: * @atomic: a pointer to a #gint or #guint @@ -405,6 +430,31 @@ gboolean oldval, newval); } +/** + * g_atomic_pointer_exchange: + * @atomic: a pointer to a #gint or #guint + * @newval: the value to replace with + * + * Sets the @atomic to @newval and returns the old value from @atomic. + * + * This exchange is done atomically. + * + * Think of this operation as an atomic version of + * `{ tmp = *atomic; *atomic = val; return tmp; }`. + * + * This call acts as a full compiler and hardware memory barrier. + * + * Returns: the value of @atomic before the exchange, signed + * + * Since: 2.74 + **/ +gpointer +(g_atomic_pointer_exchange) (void *atomic, + gpointer newval) +{ + return g_atomic_pointer_exchange ((gpointer *) atomic, newval); +} + /** * g_atomic_pointer_add: * @atomic: (not nullable): a pointer to a #gpointer-sized value @@ -609,6 +659,13 @@ gboolean return InterlockedCompareExchange (atomic, newval, oldval) == oldval; } +gint +(g_atomic_int_exchange) (gint *atomic, + gint newval) +{ + return InterlockedExchange (atomic, newval); +} + gint (g_atomic_int_add) (volatile gint *atomic, gint val) @@ -665,6 +722,13 @@ gboolean return InterlockedCompareExchangePointer (atomic, newval, oldval) == oldval; } +gpointer +(g_atomic_pointer_exchange) (void *atomic, + gpointer newval) +{ + return InterlockedExchangePointer (atomic, newval); +} + gssize (g_atomic_pointer_add) (volatile void *atomic, gssize val) @@ -787,6 +851,21 @@ gboolean return success; } +gint +(g_atomic_int_exchange) (gint *atomic, + gint newval) +{ + gint *ptr = atomic; + gint oldval; + + pthread_mutex_lock (&g_atomic_lock); + oldval = *ptr; + *ptr = newval; + pthread_mutex_unlock (&g_atomic_lock); + + return oldval; +} + gint (g_atomic_int_add) (volatile gint *atomic, gint val) @@ -886,6 +965,21 @@ gboolean return success; } +gpointer +(g_atomic_pointer_exchange) (void *atomic, + gpointer newval) +{ + gpointer *ptr = atomic; + gpointer oldval; + + pthread_mutex_lock (&g_atomic_lock); + oldval = *ptr; + *ptr = newval; + pthread_mutex_unlock (&g_atomic_lock); + + return oldval; +} + gssize (g_atomic_pointer_add) (volatile void *atomic, gssize val) diff --git a/glib/gatomic.h b/glib/gatomic.h index 086aae368..bbdcbd596 100644 --- a/glib/gatomic.h +++ b/glib/gatomic.h @@ -44,6 +44,9 @@ GLIB_AVAILABLE_IN_ALL gboolean g_atomic_int_compare_and_exchange (volatile gint *atomic, gint oldval, gint newval); +GLIB_AVAILABLE_IN_2_74 +gint g_atomic_int_exchange (gint *atomic, + gint newval); GLIB_AVAILABLE_IN_ALL gint g_atomic_int_add (volatile gint *atomic, gint val); @@ -66,6 +69,9 @@ GLIB_AVAILABLE_IN_ALL gboolean g_atomic_pointer_compare_and_exchange (volatile void *atomic, gpointer oldval, gpointer newval); +GLIB_AVAILABLE_IN_2_74 +gpointer g_atomic_pointer_exchange (void *atomic, + gpointer newval); GLIB_AVAILABLE_IN_ALL gssize g_atomic_pointer_add (volatile void *atomic, gssize val); @@ -173,6 +179,12 @@ G_END_DECLS __atomic_compare_exchange_n ((atomic), (void *) (&(gaicae_oldval)), (newval), FALSE, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ? TRUE : FALSE; \ })) #endif /* defined(glib_typeof) */ +#define g_atomic_int_exchange(atomic, newval) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (newval) : 1); \ + (gint) __atomic_exchange_n ((atomic), (newval), __ATOMIC_SEQ_CST); \ + })) #define g_atomic_int_add(atomic, val) \ (G_GNUC_EXTENSION ({ \ G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ @@ -225,6 +237,12 @@ G_END_DECLS __atomic_compare_exchange_n ((atomic), (void *) (&(gapcae_oldval)), (newval), FALSE, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ? TRUE : FALSE; \ })) #endif /* defined(glib_typeof) */ +#define g_atomic_pointer_exchange(atomic, newval) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + (gpointer) __atomic_exchange_n ((atomic), (newval), __ATOMIC_SEQ_CST); \ + })) #define g_atomic_pointer_add(atomic, val) \ (G_GNUC_EXTENSION ({ \ G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ @@ -355,6 +373,26 @@ G_END_DECLS (void) (0 ? *(atomic) ^ (newval) ^ (oldval) : 1); \ __sync_bool_compare_and_swap ((atomic), (oldval), (newval)) ? TRUE : FALSE; \ })) +#if defined(__GCC_HAVE_SYNC_SWAP) +#define g_atomic_int_exchange(atomic, newval) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (newval) : 1); \ + (gint) __sync_swap ((atomic), (newval)); \ + })) +#else /* defined(__GCC_HAVE_SYNC_SWAP) */ + #define g_atomic_int_exchange(atomic, newval) \ + (G_GNUC_EXTENSION ({ \ + gint oldval; \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (newval) : 1); \ + do \ + { \ + oldval = *atomic; \ + } while (!__sync_bool_compare_and_swap (atomic, oldval, newval)); \ + oldval; \ + })) +#endif /* defined(__GCC_HAVE_SYNC_SWAP) */ #define g_atomic_int_add(atomic, val) \ (G_GNUC_EXTENSION ({ \ G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ @@ -386,6 +424,26 @@ G_END_DECLS (void) (0 ? (gpointer) *(atomic) : NULL); \ __sync_bool_compare_and_swap ((atomic), (oldval), (newval)) ? TRUE : FALSE; \ })) +#if defined(__GCC_HAVE_SYNC_SWAP) +#define g_atomic_pointer_exchange(atomic, newval) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + (gpointer) __sync_swap ((atomic), (newval)); \ + })) +#else +#define g_atomic_pointer_exchange(atomic, newval) \ + (G_GNUC_EXTENSION ({ \ + gpointer oldval; \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + do \ + { \ + oldval = (gpointer) *atomic; \ + } while (!__sync_bool_compare_and_swap (atomic, oldval, newval)); \ + oldval; \ + })) +#endif /* defined(__GCC_HAVE_SYNC_SWAP) */ #define g_atomic_pointer_add(atomic, val) \ (G_GNUC_EXTENSION ({ \ G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ @@ -425,6 +483,8 @@ G_END_DECLS (g_atomic_int_set ((gint *) (atomic), (gint) (newval))) #define g_atomic_int_compare_and_exchange(atomic, oldval, newval) \ (g_atomic_int_compare_and_exchange ((gint *) (atomic), (oldval), (newval))) +#define g_atomic_int_exchange(atomic, newval) \ + (g_atomic_int_exchange ((gint *) (atomic), (newval))) #define g_atomic_int_add(atomic, val) \ (g_atomic_int_add ((gint *) (atomic), (val))) #define g_atomic_int_and(atomic, val) \ @@ -458,6 +518,8 @@ G_END_DECLS #define g_atomic_pointer_compare_and_exchange(atomic, oldval, newval) \ (g_atomic_pointer_compare_and_exchange ((atomic), (gpointer) (oldval), (gpointer) (newval))) +#define g_atomic_pointer_exchange(atomic, newval) \ + (g_atomic_pointer_exchange ((atomic), (gpointer) (newval))) #define g_atomic_pointer_add(atomic, val) \ (g_atomic_pointer_add ((atomic), (gssize) (val))) #define g_atomic_pointer_and(atomic, val) \ diff --git a/glib/tests/atomic.c b/glib/tests/atomic.c index 8bc100f4e..201514bfb 100644 --- a/glib/tests/atomic.c +++ b/glib/tests/atomic.c @@ -56,6 +56,9 @@ test_types (void) u2 = g_atomic_int_xor (&u, 4); g_assert_cmpint (u2, ==, 12); g_assert_cmpint (u, ==, 8); + u2 = g_atomic_int_exchange (&u, 55); + g_assert_cmpint (u2, ==, 8); + g_assert_cmpint (u, ==, 55); g_atomic_int_set (&s, 5); s2 = g_atomic_int_get (&s); @@ -79,6 +82,9 @@ test_types (void) s2 = (gint) g_atomic_int_xor (&s, 4); g_assert_cmpint (s2, ==, 12); g_assert_cmpint (s, ==, 8); + s2 = g_atomic_int_exchange (&s, 55); + g_assert_cmpint (s2, ==, 8); + g_assert_cmpint (s, ==, 55); g_atomic_pointer_set (&vp, 0); vp2 = g_atomic_pointer_get (&vp); @@ -89,10 +95,14 @@ test_types (void) res = g_atomic_pointer_compare_and_exchange (&vp, NULL, NULL); g_assert_true (res); g_assert_true (vp == 0); + g_assert_null (g_atomic_pointer_exchange (&vp, &s)); + g_assert_true (vp == &s); g_atomic_pointer_set (&vp_str, NULL); res = g_atomic_pointer_compare_and_exchange (&vp_str, NULL, str); g_assert_true (res); + g_assert_cmpstr (g_atomic_pointer_exchange (&vp_str, NULL), ==, str); + g_assert_null (vp_str); /* Note that atomic variables should almost certainly not be marked as * `volatile` — see http://isvolatileusefulwiththreads.in/c/. This test exists @@ -130,6 +140,10 @@ test_types (void) gs2 = g_atomic_pointer_xor (&gs, 4); g_assert_cmpuint (gs2, ==, 12); g_assert_cmpuint (gs, ==, 8); + vp2 = g_atomic_pointer_exchange ((gpointer*) &gs, NULL); + gs2 = (gsize) vp2; + g_assert_cmpuint (gs2, ==, 8); + g_assert_null ((gpointer) gs); g_assert_cmpint (g_atomic_int_get (csp), ==, s); g_assert_true (g_atomic_pointer_get ((const gint **) cspp) == csp); @@ -138,6 +152,7 @@ test_types (void) #undef g_atomic_int_set #undef g_atomic_int_get #undef g_atomic_int_compare_and_exchange +#undef g_atomic_int_exchange #undef g_atomic_int_add #undef g_atomic_int_inc #undef g_atomic_int_and @@ -147,6 +162,7 @@ test_types (void) #undef g_atomic_pointer_set #undef g_atomic_pointer_get #undef g_atomic_pointer_compare_and_exchange +#undef g_atomic_pointer_exchange #undef g_atomic_pointer_add #undef g_atomic_pointer_and #undef g_atomic_pointer_or @@ -173,6 +189,9 @@ test_types (void) g_assert_cmpint (u, ==, 12); u2 = g_atomic_int_xor (&u, 4); g_assert_cmpint (u2, ==, 12); + u2 = g_atomic_int_exchange (&u, 55); + g_assert_cmpint (u2, ==, 8); + g_assert_cmpint (u, ==, 55); g_atomic_int_set (&s, 5); s2 = g_atomic_int_get (&s); @@ -201,6 +220,9 @@ G_GNUC_BEGIN_IGNORE_DEPRECATIONS G_GNUC_END_IGNORE_DEPRECATIONS g_assert_cmpint (s2, ==, 8); g_assert_cmpint (s, ==, 9); + s2 = g_atomic_int_exchange (&s, 55); + g_assert_cmpint (s2, ==, 9); + g_assert_cmpint (s, ==, 55); g_atomic_pointer_set (&vp, 0); vp2 = g_atomic_pointer_get (&vp); @@ -211,6 +233,8 @@ G_GNUC_END_IGNORE_DEPRECATIONS res = g_atomic_pointer_compare_and_exchange (&vp, NULL, NULL); g_assert_true (res); g_assert_true (vp == 0); + g_assert_null (g_atomic_pointer_exchange (&vp, &s)); + g_assert_true (vp == &s); g_atomic_pointer_set (&vp_str, NULL); res = g_atomic_pointer_compare_and_exchange (&vp_str, NULL, (char *) str); @@ -222,6 +246,8 @@ G_GNUC_END_IGNORE_DEPRECATIONS g_atomic_pointer_set (&vp_str_vol, NULL); res = g_atomic_pointer_compare_and_exchange (&vp_str_vol, NULL, (char *) str); g_assert_true (res); + g_assert_cmpstr (g_atomic_pointer_exchange (&vp_str, NULL), ==, str); + g_assert_null (vp_str); g_atomic_pointer_set (&ip, 0); ip2 = g_atomic_pointer_get (&ip); @@ -249,6 +275,10 @@ G_GNUC_END_IGNORE_DEPRECATIONS gs2 = g_atomic_pointer_xor (&gs, 4); g_assert_cmpuint (gs2, ==, 12); g_assert_cmpuint (gs, ==, 8); + vp2 = g_atomic_pointer_exchange ((gpointer*) &gs, NULL); + gs2 = (gsize) vp2; + g_assert_cmpuint (gs2, ==, 8); + g_assert_null ((gpointer) gs); g_assert_cmpint (g_atomic_int_get (csp), ==, s); g_assert_true (g_atomic_pointer_get (cspp) == csp); diff --git a/glib/tests/cxx.cpp b/glib/tests/cxx.cpp index 6426d43a7..c074e18f3 100644 --- a/glib/tests/cxx.cpp +++ b/glib/tests/cxx.cpp @@ -87,6 +87,39 @@ test_atomic_int_compare_and_exchange (void) #endif } +static void +test_atomic_pointer_exchange (void) +{ +#if __cplusplus >= 201103L + const gchar *str1 = "str1"; + const gchar *str2 = "str2"; + const gchar *atomic_string = str1; + + g_test_message ("Test that g_atomic_pointer_exchange() with a " + "non-void* pointer doesn’t have any compiler warnings in C++ mode"); + + g_assert_true (g_atomic_pointer_exchange (&atomic_string, str2) == str1); + g_assert_true (atomic_string == str2); +#else + g_test_skip ("This test requires a C++11 compiler"); +#endif +} + +static void +test_atomic_int_exchange (void) +{ +#if __cplusplus >= 201103L + gint atomic_int = 5; + + g_test_message ("Test that g_atomic_int_compare_and_exchange() doesn’t have " + "any compiler warnings in C++ mode"); + + g_assert_cmpint (g_atomic_int_exchange (&atomic_int, 50), ==, 5); +#else + g_test_skip ("This test requires a C++11 compiler"); +#endif +} + int main (int argc, char *argv[]) { @@ -99,6 +132,8 @@ main (int argc, char *argv[]) g_test_add_func ("/C++/typeof", test_typeof); g_test_add_func ("/C++/atomic-pointer-compare-and-exchange", test_atomic_pointer_compare_and_exchange); g_test_add_func ("/C++/atomic-int-compare-and-exchange", test_atomic_int_compare_and_exchange); + g_test_add_func ("/C++/atomic-pointer-exchange", test_atomic_pointer_exchange); + g_test_add_func ("/C++/atomic-int-exchange", test_atomic_int_exchange); return g_test_run (); } diff --git a/meson.build b/meson.build index cecea7271..fd6545cad 100644 --- a/meson.build +++ b/meson.build @@ -1840,6 +1840,18 @@ if cc.get_id() == 'msvc' or cc.get_id() == 'clang-cl' or cc.links(atomictest, na # __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 glib_conf.set('__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4', true) endif + + if cc.get_id() == 'gcc' or cc.get_id() == 'clang' + sync_swap_test = ''' + int main() { + int atomic = 2; + __sync_swap (&atomic, 2); + return 0; + } + ''' + + glib_conf.set('__GCC_HAVE_SYNC_SWAP', cc.links(sync_swap_test, name : 'sync swap')) + endif else have_atomic_lock_free = false if host_machine.cpu_family() == 'x86' and cc.links(atomictest, args : '-march=i486')