On Mon, Apr 03, 2023 at 11:11:35PM +0200, Mattias Rönnblom wrote:
> On 2023-02-08 22:43, Tyler Retzlaff wrote:
> >Introduce atomics abstraction that permits optional use of standard C11
> >atomics when meson is provided the new enable_stdatomics=true option.
> >
>
> Terminology nitpicking: I don't think these functions provide any
> abstraction at all. They are just wrappers.
>
> >Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com>
> >---
> > config/meson.build | 11 ++++
> > lib/eal/arm/include/rte_atomic_32.h | 6 ++-
> > lib/eal/arm/include/rte_atomic_64.h | 6 ++-
> > lib/eal/include/generic/rte_atomic.h | 96
> > +++++++++++++++++++++++++++++++++-
> > lib/eal/loongarch/include/rte_atomic.h | 6 ++-
> > lib/eal/ppc/include/rte_atomic.h | 6 ++-
> > lib/eal/riscv/include/rte_atomic.h | 6 ++-
> > lib/eal/x86/include/rte_atomic.h | 8 ++-
> > meson_options.txt | 2 +
> > 9 files changed, 139 insertions(+), 8 deletions(-)
> >
> >diff --git a/config/meson.build b/config/meson.build
> >index 26f3168..25dd628 100644
> >--- a/config/meson.build
> >+++ b/config/meson.build
> >@@ -255,6 +255,17 @@ endif
> > # add -include rte_config to cflags
> > add_project_arguments('-include', 'rte_config.h', language: 'c')
> >+stdc_atomics_enabled = get_option('enable_stdatomics')
> >+dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
> >+
> >+if stdc_atomics_enabled
> >+if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
> >+ add_project_arguments('-std=gnu11', language: 'c')
> >+else
> >+ add_project_arguments('-std=c11', language: 'c')
> >+endif
> >+endif
> >+
> > # enable extra warnings and disable any unwanted warnings
> > # -Wall is added by default at warning level 1, and -Wextra
> > # at warning level 2 (DPDK default)
> >diff --git a/lib/eal/arm/include/rte_atomic_32.h
> >b/lib/eal/arm/include/rte_atomic_32.h
> >index c00ab78..7088a12 100644
> >--- a/lib/eal/arm/include/rte_atomic_32.h
> >+++ b/lib/eal/arm/include/rte_atomic_32.h
> >@@ -34,9 +34,13 @@
> > #define rte_io_rmb() rte_rmb()
> > static __rte_always_inline void
> >-rte_atomic_thread_fence(int memorder)
> >+rte_atomic_thread_fence(rte_memory_order memorder)
> > {
> >+#ifdef RTE_STDC_ATOMICS
> >+ atomic_thread_fence(memorder);
> >+#else
> > __atomic_thread_fence(memorder);
> >+#endif
> > }
> > #ifdef __cplusplus
> >diff --git a/lib/eal/arm/include/rte_atomic_64.h
> >b/lib/eal/arm/include/rte_atomic_64.h
> >index 6047911..7f02c57 100644
> >--- a/lib/eal/arm/include/rte_atomic_64.h
> >+++ b/lib/eal/arm/include/rte_atomic_64.h
> >@@ -38,9 +38,13 @@
> > #define rte_io_rmb() rte_rmb()
> > static __rte_always_inline void
> >-rte_atomic_thread_fence(int memorder)
> >+rte_atomic_thread_fence(rte_memory_order memorder)
> > {
> >+#ifdef RTE_STDC_ATOMICS
> >+ atomic_thread_fence(memorder);
> >+#else
> > __atomic_thread_fence(memorder);
> >+#endif
> > }
> > /*------------------------ 128 bit atomic operations
> > -------------------------*/
> >diff --git a/lib/eal/include/generic/rte_atomic.h
> >b/lib/eal/include/generic/rte_atomic.h
> >index f5c49a9..392d928 100644
> >--- a/lib/eal/include/generic/rte_atomic.h
> >+++ b/lib/eal/include/generic/rte_atomic.h
> >@@ -110,6 +110,100 @@
> > #endif /* __DOXYGEN__ */
> >+#ifdef RTE_STDC_ATOMICS
> >+
> >+#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L ||
> >defined(__STDC_NO_ATOMICS__)
> >+#error compiler does not support C11 standard atomics
> >+#else
> >+#include <stdatomic.h>
> >+#endif
> >+
> >+#define __rte_atomic _Atomic
> >+
> >+typedef int rte_memory_order;
> >+
> >+#define rte_memory_order_relaxed memory_order_relaxed
> >+#define rte_memory_order_consume memory_order_consume
> >+#define rte_memory_order_acquire memory_order_acquire
> >+#define rte_memory_order_release memory_order_release
> >+#define rte_memory_order_acq_rel memory_order_acq_rel
> >+#define rte_memory_order_seq_cst memory_order_seq_cst
> >+
>
> Would this be better of as an enum, rather than a typedef? If
> typedef, it should have the "_t" postfix. Also, the #define should
> be all-caps.
>
> >+#define rte_atomic_store_explicit(obj, desired, order) \
> >+ atomic_store_explicit(obj, desired, order)
> >+
>
> Drop "explicit" from all the names. It's just noise. Also, the
> memory orders have very long names.
>
> We haven't even move all DPDK code over from the old API, to using
> GCC C11 built-ins, and now we are switching to a new API?
This series has been marked obsolete/abandoned. As per technical board
DPDK will move straight to standard C atomics without transitioning with
these macros.
Thanks
