> From: Mattias Rönnblom [mailto:hof...@lysator.liu.se] > Sent: Friday, 26 January 2024 09.07 > > On 2024-01-25 23:10, Morten Brørup wrote: > >> From: Mattias Rönnblom [mailto:hof...@lysator.liu.se] > >> Sent: Thursday, 25 January 2024 19.54 > >> > >> Why do rte_stdatomic.h functions have the suffix "_explicit"? > >> Especially > >> since there aren't any wrappers for the implicit variants. > >> > >> More to type, more to read. > > > > They have the "_explicit" suffix to make their names similar to those > in stdatomic.h. > > > > OK, so to avoid a situation where someone accidentally misinterpret > rte_atomic_fetch_add(&counter, 1, rte_memory_order_relaxed); > as what, exactly? > > If you have a wrapper, why not take the opportunity and reap some > benefits from this and fix/extend the standard API, making it a better > fit for your application. Like removing the redundant "_explicit", > "fetch"-less add/sub, maybe and a function for single-writer atomic add > (non-atomic read + non-atomic add + atomic store), etc. > > Prohibiting implicit operations was done already, so it's already now > not a straight-off copy of the standard API. > > > You might consider their existence somewhat temporary until C11 > stdatomics can be fully phased in, so there's another argument for > similar names. (This probably does not happen as long as compilers > generate slower code for C11 stdatomics than with their atomic built- > ins.) > > > > To me, it seems reasonable a wrapper API should stay as long as it > provide a benefit over the standard API. One could imagine that being a > long time. > > I imagine some DPDK developers being tired of migrating from one > atomics > API to another. <rte_atomic.h> -> GCC built-ins (-> attempted C11 > stdatomics?) -> <rte_stdatomic.h>. Now you are adding a future "-> CXY > atomics" move as well, and with that it also seems natural to add a > change back to a wrapper or complementary API, when CXY didn't turned > out good enough for some particular platform, or when some non- > complaint > compiler comes along.
Yes, more migrations seem to be on the roadmap. We can take the opportunity to change direction now, and decide to keep the <rte_stdatomic.h> API long term. Then it would need more documentation (basically copying function descriptions from <stdatomic.h>), and the functions could have the "_explicit" suffix removed (macros with the suffix could be added for backwards compatibility), and more functions - like the ones you suggested above - could be added. What do people think? 1. Keep considering <rte_stdatomic.h> a temporary wrapper for <stdatomic.h> until compilers reach some undefined level of maturity, or 2. Consider <rte_stdatomic.h> stable, clean it up (remove "_explicit" suffix), add documentation to the macros, and extend it. Living in a DPDK-only environment, I would prefer option 2; but if mixing DPDK code with non-DPDK code (that uses <stdatomic.h>) it might be weird. > > I suggested fixing the original <rte_atomic.h> API, or at least have a > wrapper API, already at the point DPDK moved to direct GCC built-in > calls. Then we wouldn't have had this atomics API ping-pong. The decision back then might have been too hasty, and based on incomplete assumptions. Please shout louder next time you think a mistake is in the making. > > >> > >> When was this API introduced? Shouldn't it say "experimental" > >> somewhere? > > > > They were introduced as part of the migration to C11. > > I suppose they were not marked experimental because they replaced > something we didn't want anymore (the compiler built-ins for atomics, > e.g. __atomic_load_n()). I don't recall if we discussed experimental > marking or not. In hindsight, they should probably have been marked "experimental".
