RE: C11 atomics adoption blocked

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, 14 August 2023 15.46
> 
> mercredi 9 août 2023, Morten Brørup:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Tuesday, 8 August 2023 22.50
> > >
> > > On Tue, Aug 08, 2023 at 10:22:09PM +0200, Morten Brørup wrote:
> > > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > > Sent: Tuesday, 8 August 2023 21.20
> > > > >
> > > > > On Tue, Aug 08, 2023 at 07:23:41PM +0100, Bruce Richardson
> wrote:
> > > > > > On Tue, Aug 08, 2023 at 10:53:03AM -0700, Tyler Retzlaff
> wrote:
> > > > > > > Hi folks,
> > > > > > >
> > > > > > > Moving this discussion to the dev mailing list for broader
> > > comment.
> > > > > > >
> > > > > > > Unfortunately, we've hit a roadblock with integrating C11
> > > atomics
> > > > > > > for DPDK.  The main issue is that GNU C++ prior to -
> std=c++23
> > > > > explicitly
> > > > > > > cannot be integrated with C11 stdatomic.h.  Basically, you
> can't
> > > > > include
> > > > > > > the header and you can't use `_Atomic' type specifier to
> declare
> > > > > atomic
> > > > > > > types. This is not a problem with LLVM or MSVC as they both
> > > allow
> > > > > > > integration with C11 stdatomic.h, but going forward with C11
> > > atomics
> > > > > > > would break using DPDK in C++ programs when building with
> GNU
> > > g++.
> > > > > > >
> > > > > > > Essentially you cannot compile the following with g++.
> > > > > > >
> > > > > > >   #include <stdatomic.h>
> > > > > > >
> > > > > > >   int main(int argc, char *argv[]) { return 0; }
> > > > > > >
> > > > > > >   In file included from atomic.cpp:1:
> > > > > > >   /usr/lib/gcc/x86_64-pc-cygwin/11/include/stdatomic.h:40:9:
> > > error:
> > > > > > >   ‘_Atomic’ does not name a type
> > > > > > >      40 | typedef _Atomic _Bool atomic_bool;
> > > > > > >
> > > > > > >   ... more errors of same ...
> > > > > > >
> > > > > > > It's also acknowledged as something known and won't fix by
> GNU
> > > g++
> > > > > > > maintainers.
> > > > > > >
> > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60932
> > > > > > >
> > > > > > > Given the timeframe I would like to propose the minimally
> > > invasive,
> > > > > > > lowest risk solution as follows.
> > > > > > >
> > > > > > > 1.  Adopt stdatomic.h for all Windows targets, leave all
> > > Linux/BSD
> > > > > targets
> > > > > > >     using GCC builtin C++11 memory model atomics.
> > > > > > > 2.  Introduce a macro that allows _Atomic type specifier to
> be
> > > > > applied to
> > > > > > >     function parameter, structure field types and variable
> > > > > declarations.
> > > > > > >
> > > > > > >     * The macro would expand empty for Linux/BSD targets.
> > > > > > >     * The macro would expand to C11 _Atomic keyword for
> Windows
> > > > > targets.
> > > > > > >
> > > > > > > 3.  Introduce basic macro that allows __atomic_xxx  for
> > > normalized
> > > > > use
> > > > > > >     internal to DPDK.
> > > > > > >
> > > > > > >     * The macro would not be defined for Linux/BSD targets.
> > > > > > >     * The macro would expand __atomic_xxx to corresponding
> > > > > stdatomic.h
> > > > > > >       atomic_xxx operations for Windows targets.
> > > > > > >
> > > >
> > > > Regarding naming of these macros (suggested in 2. and 3.), they
> should
> > > probably bear the rte_ prefix instead of overlapping existing names,
> so
> > > applications can also use them directly.
> > > >
> > > > E.g.:
> > > > #define rte_atomic for _Atomic or nothing,
> > > > #define rte_atomic_fetch_add() for atomic_fetch_add() or
> > > __atomic_fetch_add(), and
> > > > #define RTE_MEMORY_ORDER_SEQ_CST for memory_order_seq_cst or
> > > __ATOMIC_SEQ_CST.
> > > >
> > > > Maybe that is what you meant already. I'm not sure of the scope
> and
> > > details of your suggestion here.
> > >
> > > I'm shy to do anything in the rte_ namespace because I don't want to
> > > formalize it as an API.
> > >
> > > I was envisioning the following.
> > >
> > > Internally DPDK code just uses __atomic_fetch_add directly, the
> macros
> > > are provided for Windows targets to expand to __atomic_fetch_add.
> > >
> > > Externally DPDK applications that don't care about being portable
> may
> > > use __atomic_fetch_add (BSD/Linux) or atomic_fetch_add (Windows)
> > > directly.
> > >
> > > Externally DPDK applications that care to be portable may do what is
> > > done Internally and <<use>> the __atomic_fetch_add directly. By
> > > including say rte_stdatomic.h indirectly (Windows) gets the macros
> > > expanded to atomic_fetch_add and for BSD/Linux it's a noop include.
> > >
> > > Basically I'm placing a little ugly into Windows built code and in
> trade
> > > we don't end up with a bunch of rte_ APIs that were strongly
> objected to
> > > previously.
> > >
> > > It's a compromise.
> >
> > OK, we probably need to offer a public header file to wrap the
> atomics, using either names prefixed with rte_ or names similar to the
> gcc builtin atomics.
> >
> > I guess the objections were based on the assumption that we were
> switching to C11 atomics with DPDK 23.11, so the rte_ prefixed atomic
> APIs would be very short lived (DPDK 23.07 to 23.11 only). But with this
> new information about GNU C++ incompatibility, that seems not to be the
> case, so the naming discussion can be reopened.
> >
> > If we don't introduce such a wrapper header, all portable code needs
> to surround the use of atomics with #ifdef USE_STDATOMIC_H.
> >
> > BTW: Can the compilers that understand both builtin atomics and C11
> stdatomics.h handle code with #define __atomic_fetch_add
> atomic_fetch_add and #define __ATOMIC_SEQ_CST memory_order_seq_cst? If
> not, then we need to use rte_ prefixed atomics.
> >
> > And what about C++ atomics... Do we want (or need?) a third variant
> using C++ atomics, e.g. "atomic<int> x;" instead of "_Atomic int x;"? (I
> hope not!) For reference, the "atomic_int" type is "_Atomic int" in C,
> but "std::atomic<int>" in C++.
> >
> > C++23 provides the C11 compatibility macro "_Atomic(T)", which means
> "_Atomic T" in C and "std::atomic<T>" in C++. Perhaps we can somewhat
> rely on this, and update our coding standards to require using e.g.
> "_Atomic(int)" for atomic types, and disallow using "_Atomic int".
> 
> You mean the syntax _Atomic(T) is working well in both C and C++?

This syntax is API compatible across C11 and C++23, so it would work with (C11 and C++23) applications building DPDK from scratch.

But it is only "recommended" ABI compatible for compilers [1], so DPDK in distros cannot rely on.

[1]: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0943r6.html

It would be future-proofing for the benefit of C++23 based applications... I was mainly mentioning it for completeness, now that we are switching to a new standard for atomics.

Realistically, considering that 1. such a coding standard (requiring "_Atomic(T)" instead of "_Atomic T") would only be relevant for a 2023 standard, and 2. that we are now upgrading to a standard from 2011, we would probably have to wait for a very distant future (12 years?) before C++ applications can reap the benefits of such a coding standard.