Re: [PATCH 1/3] lib/eal: add rte_atomic128_cmp_exchange compatible with MSVC

[Andre Muezerie <andremue@linux.microsoft.com>]

On Fri, Jan 24, 2025 at 03:27:06PM +0100, David Marchand wrote:
> On Tue, Dec 10, 2024 at 5:33 PM Andre Muezerie
> <andremue@linux.microsoft.com> wrote:
> >
> > MSVC does not support inline assembly, which is used by the
> > implementation of rte_atomic128_cmp_exchange and is needed
> > by lib/stack.
> >
> > Error printed by MSVC:
> >
> > stack_rte_stack_lf.c.obj : error LNK2019:
> >     unresolved external symbol rte_atomic128_cmp_exchange referenced
> >     in function __rte_stack_lf_push_elems
> >
> > Fix is to provide an implementation for rte_atomic128_cmp_exchange
> > which uses an intrinsic function, which is used when compiling with
> > MSVC. For other compilers the existing implementation continues to
> > be used.
> >
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  lib/eal/x86/include/rte_atomic.h    |  4 ++--
> >  lib/eal/x86/include/rte_atomic_64.h | 18 ++++++++++++++++++
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
> > index c72c47c83e..e8e0e4c33c 100644
> > --- a/lib/eal/x86/include/rte_atomic.h
> > +++ b/lib/eal/x86/include/rte_atomic.h
> > @@ -288,12 +288,12 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
> >
> >  #endif
> >
> > +#endif /* RTE_TOOLCHAIN_MSVC */
> > +
> >  #ifdef RTE_ARCH_I686
> >  #include "rte_atomic_32.h"
> >  #else
> >  #include "rte_atomic_64.h"
> >  #endif
> >
> > -#endif
> > -
> 
> This partially reverts 27da6a123414 ("eal: hide legacy atomics API for MSVC").
> It would be better to implement an equivalent to
> rte_atomic128_cmp_exchange in the DPDK "new" stdatomic API
> (rte_stdatomic.h).
> 
> 
> -- 
> David Marchand

Thanks for calling that out. After looking at the past commits I got a better understanding
of the reasons the atomic-related code is laid out the way it is, and I agree that we should
attempt to follow the same guidelines.

For that reason I changed the approach taken in the v2 I sent out today.

Let me know your thoughts.
--
Andre Muezerie