On 2024-05-04 01:30, Tyler Retzlaff wrote:
On Fri, May 03, 2024 at 08:41:09AM +0200, Mattias Rönnblom wrote:
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.ronnb...@ericsson.com>
Acked-by: Morten Brørup <m...@smartsharesystems.com>
Acked-by: Tyler Retzlaff <roret...@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?
off-topic comment relating to the generic type selection list above, i was
reading C17 DR481 recently and i think we may want to avoid providing
qualified and unauqlified types in the list.
DR481: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2396.htm#dr_481
"the controlling expression of a generic selection shall have type
compatibile with at most one of the types named in its generic
association list."
Const and unqualified pointers are not compatible. Without the "const
uint32_t *" element in the association list, passing const-qualified
pointers to rte_bit_test() will cause a compiler error.
So, if you want to support both passing const-qualified and unqualified
pointers (which you do, obviously), then you have no other option than
to treat them separately.
GCC, clang and ICC all seem to agree on this. The standard also is
pretty clear on this, from what I can tell. "No two generic associations
in the same generic selection shall specify compatible types." (6.5.1.1,
note *compatible*). "For two pointer types to be compatible, both shall
be identically qualified and both shall be pointers to compatible
types." (6.7.6.1)
"the type of the controlling expression is the type of the expression as
if it had undergone an lvalue conversion"
"lvalue conversion drops type qualifiers"
so the unqualified type of the controlling expression is only matched
selection list which i guess that means the qualified entries in the
list are never selected.
i suppose the implication here is we couldn't then provide 2 inline
functions one for volatile qualified and for not volatile qualified.
as for a single function where the parameter is or isn't volatile
qualified. if we're always forwarding to an intrinsic i've always
assumed (perhaps incorrectly) that the intrinsic itself did what was
semantically correct even without qualification.
as you note i believe there is a convenience element in providing the
volatile qualified version since it means the function like macro /
inline function will accept both volatile qualified and unqualified
whereas if we did not qualify the parameter it would require the
caller/user to strip the volatile qualification if present with casts.
i imagine in most cases we are just forwarding, in which case it seems
not horrible to provide the qualified version.
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.
it's super!
<snip>
