[PATCH 0/3] enable build of lib/stack when using MSVC

[PATCH 0/3] enable build of lib/stack when using MSVC

[Andre Muezerie]

MSVC does not support inline assembly, which is used by the
implementation of rte_atomic128_cmp_exchange and is needed
by lib/stack.

An implementation for rte_atomic128_cmp_exchange compatible with MSVC
using an intrinsic function is added. For other compilers the
existing implementation continues to be used.

A basic test is added to provide coverage for this special
rte_atomic128_cmp_exchange implementation for MSVC. This same
test can be run when using other compilers as well, in which case
the old implementation for rte_atomic128_cmp_exchange is used.

Andre Muezerie (3):
  lib/eal: add rte_atomic128_cmp_exchange compatible with MSVC
  app/test: add basic test for rte_atomic128_cmp_exchange
  lib/stack: enable build with MSVC

 app/test/test_atomic.c              | 59 +++++++++++++++++++++++++++++
 lib/eal/x86/include/rte_atomic.h    |  4 +-
 lib/eal/x86/include/rte_atomic_64.h | 18 +++++++++
 lib/stack/meson.build               |  6 ---
 4 files changed, 79 insertions(+), 8 deletions(-)

--
2.47.0.vfs.0.3

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

[Andre Muezerie]

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
-
 #endif /* _RTE_ATOMIC_X86_H_ */
diff --git a/lib/eal/x86/include/rte_atomic_64.h b/lib/eal/x86/include/rte_atomic_64.h
index 0a7a2131e0..26c87a2da6 100644
--- a/lib/eal/x86/include/rte_atomic_64.h
+++ b/lib/eal/x86/include/rte_atomic_64.h
@@ -182,6 +182,23 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 
 /*------------------------ 128 bit atomic operations -------------------------*/
 
+#ifdef RTE_TOOLCHAIN_MSVC
+static inline int
+rte_atomic128_cmp_exchange(rte_int128_t *dst,
+			   rte_int128_t *exp,
+			   const rte_int128_t *src,
+			   unsigned int weak,
+			   int success,
+			   int failure)
+{
+	return (int)_InterlockedCompareExchange128(
+		(int64_t volatile *) dst,
+		src->val[1], /* exchange high */
+		src->val[0], /* exchange low */
+		(int64_t *) exp /* comparand result */
+	);
+}
+#else
 static inline int
 rte_atomic128_cmp_exchange(rte_int128_t *dst,
 			   rte_int128_t *exp,
@@ -212,5 +229,6 @@ rte_atomic128_cmp_exchange(rte_int128_t *dst,
 
 	return res;
 }
+#endif /* RTE_TOOLCHAIN_MSVC */
 
 #endif /* _RTE_ATOMIC_X86_64_H_ */
-- 
2.47.0.vfs.0.3

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

[David Marchand]

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

[PATCH 2/3] app/test: add basic test for rte_atomic128_cmp_exchange

[Andre Muezerie]

A basic test for rte_atomic128_cmp_exchange that can also be
compiled with MSVC and run on Windows is being added.

This is relevant as rte_atomic128_cmp_exchange uses a different
implementation when compiled with MSVC and the existing tests for
this function are not compatible with MSVC.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 app/test/test_atomic.c | 59 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/app/test/test_atomic.c b/app/test/test_atomic.c
index db07159e81..d3d9de0a41 100644
--- a/app/test/test_atomic.c
+++ b/app/test/test_atomic.c
@@ -20,6 +20,7 @@
 
 #include "test.h"
 
+#ifndef RTE_TOOLCHAIN_MSVC
 /*
  * Atomic Variables
  * ================
@@ -441,9 +442,41 @@ test_atomic_exchange(__rte_unused void *arg)
 
 	return 0;
 }
+#endif /* RTE_TOOLCHAIN_MSVC */
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_ARM64)
+static rte_int128_t big_int;
+
+/*
+ * This function atomically performs:
+ *     big_int.val[1] += big_int.val[0]
+ *     big_int.val[0] += 1
+ */
+static void
+test_atomic_128_bit_compare_and_swap_basic_test(void)
+{
+	rte_int128_t comparand = big_int;
+
+	rte_int128_t src;
+	src.val[0] = big_int.val[0] + 1;
+	src.val[1] = big_int.val[0] + big_int.val[1];
+
+	do {
+		; /* nothing */
+	} while (rte_atomic128_cmp_exchange(&big_int,
+										&comparand,
+										&src,
+										1,
+										0,
+										0
+										));
+}
+#endif
+
 static int
 test_atomic(void)
 {
+#ifndef RTE_TOOLCHAIN_MSVC
 	rte_atomic16_init(&a16);
 	rte_atomic32_init(&a32);
 	rte_atomic64_init(&a64);
@@ -628,6 +661,32 @@ test_atomic(void)
 		printf("Atomic exchange test failed\n");
 		return -1;
 	}
+#endif /* RTE_TOOLCHAIN_MSVC */
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_ARM64)
+	/*
+	 * This is a basic test case for rte_atomic128_cmp_exchange.
+	 * On MSVC this test provides the confirmation that
+	 * rte_atomic128_cmp_exchange passes the parameters correctly
+	 * to the underlying intrinsic function responsible for the
+	 * operation.
+	 *
+	 * The test atomically performs:
+	 *     big_int.val[1] += big_int.val[0]
+	 *     big_int.val[0] += 1
+	 */
+	printf("128-bit compare and swap basic test\n");
+
+	big_int.val[1] = 23; /* should become 34 */
+	big_int.val[0] = 11; /* should become 12 */
+
+	test_atomic_128_bit_compare_and_swap_basic_test();
+
+	if (big_int.val[1] != 34 || big_int.val[0] != 12) {
+		printf("128-bit compare and swap basic test failed\n");
+		return -1;
+	}
+#endif
 
 	return 0;
 }
-- 
2.47.0.vfs.0.3

[PATCH 3/3] lib/stack: enable build with MSVC

[Andre Muezerie]

Now that the issues preventing the code needed to build lib/stack
have been addressed, it can be enabled so that it also gets built
when using the MSVC compiler.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 lib/stack/meson.build | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/lib/stack/meson.build b/lib/stack/meson.build
index 7631a14784..18177a742f 100644
--- a/lib/stack/meson.build
+++ b/lib/stack/meson.build
@@ -1,12 +1,6 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2019 Intel Corporation
 
-if is_ms_compiler
-    build = false
-    reason = 'not supported building with Visual Studio Toolset'
-    subdir_done()
-endif
-
 sources = files('rte_stack.c', 'rte_stack_std.c', 'rte_stack_lf.c')
 headers = files('rte_stack.h')
 # subheaders, not for direct inclusion by apps
-- 
2.47.0.vfs.0.3