From 2eb37622418a5c9f31a9d728a99bc621d3157ab0 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Sat, 21 Sep 2019 17:34:37 +0200 Subject: [PATCH 1/3] gatomic: Use new __atomic_*() intrinsics for all atomic operations Previously we used the old `__sync_fetch_*()` intrinsics for some of the atomic operations, such as `g_atomic_int_compare_and_exchange()`. When available, use the new `__atomic_*()` intrinsics for those instead. As with the rest of our use of `__atomic_*()` intrinsics, we use the `__ATOMIC_SEQ_CST` memory model. If people want to use a less restrictive memory model to get better performance in certain situations, they can use the compiler intrinsics directly themselves. `g_atomic_*()` aim to be as fast as possible while remaining general purpose. Tested using: ``` meson test --repeat 1000 atomic atomic-test ``` Signed-off-by: Philip Withnall Fixes: #1750 --- glib/gatomic.h | 90 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 88 insertions(+), 2 deletions(-) diff --git a/glib/gatomic.h b/glib/gatomic.h index 61a99adf2..712553e69 100644 --- a/glib/gatomic.h +++ b/glib/gatomic.h @@ -118,6 +118,92 @@ G_END_DECLS __atomic_store ((gpointer *)(atomic), &gaps_temp, __ATOMIC_SEQ_CST); \ })) +#define g_atomic_int_inc(atomic) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ *(atomic) : 1); \ + (void) __atomic_fetch_add ((atomic), 1, __ATOMIC_SEQ_CST); \ + })) +#define g_atomic_int_dec_and_test(atomic) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ *(atomic) : 1); \ + __atomic_fetch_sub ((atomic), 1, __ATOMIC_SEQ_CST) == 1; \ + })) +#define g_atomic_int_compare_and_exchange(atomic, oldval, newval) \ + (G_GNUC_EXTENSION ({ \ + gint gaicae_oldval = (oldval); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (newval) ^ (oldval) : 1); \ + __atomic_compare_exchange_n ((atomic), &gaicae_oldval, (newval), FALSE, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ? TRUE : FALSE; \ + })) +#define g_atomic_int_add(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (val) : 1); \ + (gint) __atomic_fetch_add ((atomic), (val), __ATOMIC_SEQ_CST); \ + })) +#define g_atomic_int_and(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (val) : 1); \ + (guint) __atomic_fetch_and ((atomic), (val), __ATOMIC_SEQ_CST); \ + })) +#define g_atomic_int_or(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (val) : 1); \ + (guint) __atomic_fetch_or ((atomic), (val), __ATOMIC_SEQ_CST); \ + })) +#define g_atomic_int_xor(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ + (void) (0 ? *(atomic) ^ (val) : 1); \ + (guint) __atomic_fetch_xor ((atomic), (val), __ATOMIC_SEQ_CST); \ + })) + +#define g_atomic_pointer_compare_and_exchange(atomic, oldval, newval) \ + (G_GNUC_EXTENSION ({ \ + __typeof__ ((oldval)) gapcae_oldval = (oldval); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + __atomic_compare_exchange_n ((atomic), &gapcae_oldval, (newval), FALSE, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ? TRUE : FALSE; \ + })) +#define g_atomic_pointer_add(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + (void) (0 ? (val) ^ (val) : 1); \ + (gssize) __atomic_fetch_add ((atomic), (val), __ATOMIC_SEQ_CST); \ + })) +#define g_atomic_pointer_and(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + volatile gsize *gapa_atomic = (volatile gsize *) (atomic); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gsize)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + (void) (0 ? (val) ^ (val) : 1); \ + (gsize) __atomic_fetch_and (gapa_atomic, (val), __ATOMIC_SEQ_CST); \ + })) +#define g_atomic_pointer_or(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + volatile gsize *gapo_atomic = (volatile gsize *) (atomic); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gsize)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + (void) (0 ? (val) ^ (val) : 1); \ + (gsize) __atomic_fetch_or (gapo_atomic, (val), __ATOMIC_SEQ_CST); \ + })) +#define g_atomic_pointer_xor(atomic, val) \ + (G_GNUC_EXTENSION ({ \ + volatile gsize *gapx_atomic = (volatile gsize *) (atomic); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gpointer)); \ + G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gsize)); \ + (void) (0 ? (gpointer) *(atomic) : NULL); \ + (void) (0 ? (val) ^ (val) : 1); \ + (gsize) __atomic_fetch_xor (gapx_atomic, (val), __ATOMIC_SEQ_CST); \ + })) + #else /* defined(__ATOMIC_SEQ_CST) */ /* We want to achieve __ATOMIC_SEQ_CST semantics here. See @@ -184,8 +270,6 @@ G_END_DECLS *(atomic) = (__typeof__ (*(atomic))) (gsize) (newval); \ })) -#endif /* !defined(__ATOMIC_SEQ_CST) */ - #define g_atomic_int_inc(atomic) \ (G_GNUC_EXTENSION ({ \ G_STATIC_ASSERT (sizeof *(atomic) == sizeof (gint)); \ @@ -264,6 +348,8 @@ G_END_DECLS (gsize) __sync_fetch_and_xor ((atomic), (val)); \ })) +#endif /* !defined(__ATOMIC_SEQ_CST) */ + #else /* defined(G_ATOMIC_LOCK_FREE) && defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4) */ #define g_atomic_int_get(atomic) \ From 5dbaa18d6117a301e7b08da69af2c90ab9024657 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 8 Oct 2019 12:07:58 +0100 Subject: [PATCH 2/3] tests: Test g_atomic_pointer_compare_and_exchange() returning false There were tests for the situation where it does the exchange and returns true, but no tests for the situation where it returns false. Signed-off-by: Philip Withnall --- glib/tests/atomic.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/glib/tests/atomic.c b/glib/tests/atomic.c index 22cb0ee77..b806c0fa5 100644 --- a/glib/tests/atomic.c +++ b/glib/tests/atomic.c @@ -80,6 +80,9 @@ test_types (void) g_atomic_pointer_set (&vp, 0); vp2 = g_atomic_pointer_get (&vp); g_assert_true (vp2 == 0); + res = g_atomic_pointer_compare_and_exchange (&vp, &s, &s); + g_assert_false (res); + g_assert_true (vp == 0); res = g_atomic_pointer_compare_and_exchange (&vp, 0, 0); g_assert_true (res); g_assert_true (vp == 0); @@ -185,6 +188,9 @@ G_GNUC_END_IGNORE_DEPRECATIONS g_atomic_pointer_set (&vp, 0); vp2 = g_atomic_pointer_get (&vp); g_assert_true (vp2 == 0); + res = g_atomic_pointer_compare_and_exchange (&vp, &s, &s); + g_assert_false (res); + g_assert_true (vp == 0); res = g_atomic_pointer_compare_and_exchange (&vp, 0, 0); g_assert_true (res); g_assert_true (vp == 0); From 30dd30d05a6e20589a52a6e32d9bf8610548be95 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 8 Oct 2019 12:08:50 +0100 Subject: [PATCH 3/3] tests: Remove redundant old atomic test The test in `glib/tests/atomic.c` does everything this test did, and more. Signed-off-by: Philip Withnall Helps: #1434 --- tests/atomic-test.c | 63 --------------------------------------------- tests/meson.build | 1 - 2 files changed, 64 deletions(-) delete mode 100644 tests/atomic-test.c diff --git a/tests/atomic-test.c b/tests/atomic-test.c deleted file mode 100644 index 3dd4b1990..000000000 --- a/tests/atomic-test.c +++ /dev/null @@ -1,63 +0,0 @@ -#undef G_DISABLE_ASSERT -#undef G_LOG_DOMAIN - -#include - -/* Obviously we can't test that the operations are atomic, but we can - * at least test, that they do, what they ought to do */ - -int -main (int argc, - char *argv[]) -{ - gint i; - gint atomic = -5; - gpointer atomic_pointer = NULL; - gpointer biggest_pointer = (gpointer)((gsize)atomic_pointer - 1); - - for (i = 0; i < 15; i++) - g_atomic_int_inc (&atomic); - g_assert (atomic == 10); - for (i = 0; i < 9; i++) - g_assert (!g_atomic_int_dec_and_test (&atomic)); - g_assert (g_atomic_int_dec_and_test (&atomic)); - g_assert (atomic == 0); - - g_assert (g_atomic_int_add (&atomic, 5) == 0); - g_assert (atomic == 5); - - g_assert (g_atomic_int_add (&atomic, -10) == 5); - g_assert (atomic == -5); - - g_atomic_int_add (&atomic, 20); - g_assert (atomic == 15); - - g_atomic_int_add (&atomic, -35); - g_assert (atomic == -20); - - g_assert (atomic == g_atomic_int_get (&atomic)); - - g_assert (g_atomic_int_compare_and_exchange (&atomic, -20, 20)); - g_assert (atomic == 20); - - g_assert (!g_atomic_int_compare_and_exchange (&atomic, 42, 12)); - g_assert (atomic == 20); - - g_assert (g_atomic_int_compare_and_exchange (&atomic, 20, G_MAXINT)); - g_assert (atomic == G_MAXINT); - - g_assert (g_atomic_int_compare_and_exchange (&atomic, G_MAXINT, G_MININT)); - g_assert (atomic == G_MININT); - - g_assert (g_atomic_pointer_compare_and_exchange (&atomic_pointer, - NULL, biggest_pointer)); - g_assert (atomic_pointer == biggest_pointer); - - g_assert (atomic_pointer == g_atomic_pointer_get (&atomic_pointer)); - - g_assert (g_atomic_pointer_compare_and_exchange (&atomic_pointer, - biggest_pointer, NULL)); - g_assert (atomic_pointer == NULL); - - return 0; -} diff --git a/tests/meson.build b/tests/meson.build index e4ea22692..6741f8f52 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -19,7 +19,6 @@ tests = { 'testglib' : {'tap' : true}, 'testgdate' : {}, 'datetime' : {}, - 'atomic-test' : {}, 'bit-test' : {}, 'child-test' : {}, 'completion-test' : {},