[RFC v6 5/6] eal: add atomic bit operations

[Mattias Rönnblom]

On 2024-05-02 07:57, Mattias Rönnblom wrote:
> Add atomic bit test/set/clear/assign/flip and
> test-and-set/clear/assign/flip functions.
> 
> All atomic bit functions allow (and indeed, require) the caller to
> specify a memory order.
> 
> RFC v6:
>   * Have rte_bit_atomic_test() accept const-marked bitsets.
> 
> RFC v4:
>   * Add atomic bit flip.
>   * Mark macro-generated private functions experimental.
> 
> RFC v3:
>   * Work around lack of C++ support for _Generic (Tyler Retzlaff).
> 
> RFC v2:
>   o Add rte_bit_atomic_test_and_assign() (for consistency).
>   o Fix bugs in rte_bit_atomic_test_and_[set|clear]().
>   o Use <rte_stdatomics.h> to support MSVC.
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom at ericsson.com>
> Acked-by: Morten Brørup <mb at smartsharesystems.com>
> Acked-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> ---
>   lib/eal/include/rte_bitops.h | 428 +++++++++++++++++++++++++++++++++++
>   1 file changed, 428 insertions(+)
> 
> diff --git a/lib/eal/include/rte_bitops.h b/lib/eal/include/rte_bitops.h
> index caec4f36bb..9cde982113 100644
> --- a/lib/eal/include/rte_bitops.h
> +++ b/lib/eal/include/rte_bitops.h
> @@ -21,6 +21,7 @@
>   
>   #include <rte_compat.h>
>   #include <rte_debug.h>
> +#include <rte_stdatomic.h>
>   
>   #ifdef __cplusplus
>   extern "C" {
> @@ -399,6 +400,202 @@ extern "C" {
>   		 uint32_t *: __rte_bit_once_flip32,		\
>   		 uint64_t *: __rte_bit_once_flip64)(addr, nr)
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Test if a particular bit in a word is set with a particular memory
> + * order.
> + *
> + * Test a bit with the resulting memory load ordered as per the
> + * specified memory order.
> + *
> + * @param addr
> + *   A pointer to the word to query.
> + * @param nr
> + *   The index of the bit.
> + * @param memory_order
> + *   The memory order to use. See <rte_stdatomics.h> for details.
> + * @return
> + *   Returns true if the bit is set, and false otherwise.
> + */
> +#define rte_bit_atomic_test(addr, nr, memory_order)			\
> +	_Generic((addr),						\
> +		 uint32_t *: __rte_bit_atomic_test32,			\
> +		 const uint32_t *: __rte_bit_atomic_test32,		\
> +		 uint64_t *: __rte_bit_atomic_test64,			\
> +		 const uint64_t *: __rte_bit_atomic_test64)(addr, nr,	\
> +							    memory_order)

Should __rte_bit_atomic_test32()'s addr parameter be marked volatile, 
and two volatile-marked branches added to the above list? Both the 
C11-style GCC built-ins and the C11-proper atomic functions have 
addresses marked volatile. The Linux kernel and the old __sync GCC 
built-ins on the other hand, doesn't (although I think you still get 
volatile semantics). The only point of "volatile", as far as I can see, 
is to avoid warnings in case the user passed a volatile-marked pointer. 
The drawback is that *you're asking for volatile semantics*, although 
with the current compilers, it seems like that is what you get, 
regardless if you asked for it or not.

Just to be clear: even these functions would accept volatile-marked 
pointers, non-volatile pointers should be accepted as well (and should 
generally be preferred).

Isn't parallel programming in C lovely.

<snip>