msvc integration changes

PATCH Superseded Tyler Retzlaff 8 participants 240 messages Apr 3, 2023 – Sep 25, 2023 Arm/Amlogic Meson SoC support Page 2 of 2

lore

Share & tools

lore

Tyler Retzlaff

author

2023-05-02 03:17 UTC #164 lore raw

PATCH v8 10/14 eal: expand most macros to empty when using MSVC

In reply to Tyler Retzlaff

For now expand a lot of common rte macros empty. The catch here is we need to test that most of the macros do what they should but at the same time they are blocking work needed to bootstrap of the unit tests.

Later we will return and provide (where possible) expansions that work correctly for msvc and where not possible provide some alternate macros to achieve the same outcome.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_branch_prediction.h |  8 +++++
 lib/eal/include/rte_common.h            | 54 +++++++++++++++++++++++++++++++++
 lib/eal/include/rte_compat.h            | 20 ++++++++++++
 3 files changed, 82 insertions(+)

lib/eal/include/rte_branch_prediction.h +8 -0

index 0256a9d..1eff9f6 100644
@@ -25,7 +25,11 @@
  *
  */
 #ifndef likely
+#ifndef RTE_TOOLCHAIN_MSVC
 #define likely(x)	__builtin_expect(!!(x), 1)
+#else
+#define likely(x)	(!!(x))
+#endif
 #endif /* likely */
 
 /**

@@ -39,7 +43,11 @@
  *
  */
 #ifndef unlikely
+#ifndef RTE_TOOLCHAIN_MSVC
 #define unlikely(x)	__builtin_expect(!!(x), 0)
+#else
+#define unlikely(x)	(!!(x))
+#endif
 #endif /* unlikely */
 
 #ifdef __cplusplus

lib/eal/include/rte_common.h +54 -0

147 lines not on this page · raw message

lib/eal/include/rte_compat.h +20 -0

index fc9fbaa..6a4b5ee 100644
@@ -12,14 +12,22 @@
 
 #ifndef ALLOW_EXPERIMENTAL_API
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_experimental \
 __attribute__((deprecated("Symbol is not yet part of stable ABI"), \
 section(".text.experimental")))
+#else
+#define __rte_experimental
+#endif
 
 #else
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_experimental \
 __attribute__((section(".text.experimental")))
+#else
+#define __rte_experimental
+#endif
 
 #endif

@@ -30,23 +38,35 @@
 
 #if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_internal \
 __attribute__((error("Symbol is not public ABI"), \
 section(".text.internal")))
+#else
+#define __rte_internal
+#endif
 
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_internal \
 _Pragma("GCC diagnostic push") \
 _Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
 section(".text.internal"))) \
 _Pragma("GCC diagnostic pop")
+#else
+#define __rte_internal
+#endif
 
 #else
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #define __rte_internal \
 __attribute__((section(".text.internal")))
+#else
+#define __rte_internal
+#endif
 
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-05-02 03:17 UTC #165 lore raw

PATCH v8 11/14 eal: exclude exposure of rte atomic APIs for MSVC builds

In reply to Tyler Retzlaff

It's discouraged to use rte_atomics APIs instead standard APIs should be used from C11. Since MSVC is a new toolchain/platform combination block visibility of the rte_atomic APIs from day 1.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_atomic.h | 7 +++++++
 lib/eal/x86/include/rte_atomic.h     | 4 ++++
 2 files changed, 11 insertions(+)

lib/eal/include/generic/rte_atomic.h +7 -0

index e973184..1964697 100644
@@ -131,6 +131,8 @@
 
 /*------------------------- 16 bit atomic operations -------------------------*/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Atomic compare and set.
  *

@@ -1038,8 +1040,11 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 }
 #endif
 
+#endif
+
 /*------------------------ 128 bit atomic operations -------------------------*/
 
+
 /**
  * 128-bit integer structure.
  */

@@ -1049,8 +1054,10 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 	union {
 		uint64_t val[2];
 #ifdef RTE_ARCH_64
+#ifndef RTE_TOOLCHAIN_MSVC
 		__extension__ __int128 int128;
 #endif
+#endif
 	};
 } __rte_aligned(16) rte_int128_t;

lib/eal/x86/include/rte_atomic.h +4 -0

index 569d3ab..adf3309 100644
@@ -83,6 +83,8 @@
 
 #define rte_io_rmb() rte_compiler_barrier()
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Synchronization fence between threads based on the specified memory order.
  *

@@ -279,6 +281,8 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 #include "rte_atomic_64.h"
 #endif
 
+#endif
+
 #ifdef __cplusplus
 }
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-05-02 03:16 UTC #166 lore raw

PATCH v8 12/14 telemetry: avoid expanding versioned symbol macros on MSVC

In reply to Tyler Retzlaff

Windows does not support versioned symbols. Fortunately Windows also doesn't have an exported stable ABI.

Export rte_tel_data_add_array_int -> rte_tel_data_add_array_int_24 and rte_tel_data_add_dict_int -> rte_tel_data_add_dict_int_v24 functions.

Windows does have a way to achieve similar versioning for symbols but it is not a simple #define so it will be done as a work package later.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/telemetry/telemetry_data.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

lib/telemetry/telemetry_data.c +16 -0

index 2bac2de..284c16e 100644
@@ -82,8 +82,16 @@
 /* mark the v23 function as the older version, and v24 as the default version */
 VERSION_SYMBOL(rte_tel_data_add_array_int, _v23, 23);
 BIND_DEFAULT_SYMBOL(rte_tel_data_add_array_int, _v24, 24);
+#ifndef RTE_TOOLCHAIN_MSVC
 MAP_STATIC_SYMBOL(int rte_tel_data_add_array_int(struct rte_tel_data *d,
 		int64_t x), rte_tel_data_add_array_int_v24);
+#else
+int
+rte_tel_data_add_array_int(struct rte_tel_data *d, int64_t x)
+{
+	return rte_tel_data_add_array_int_v24(d, x);
+}
+#endif
 
 int
 rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)

@@ -220,8 +228,16 @@
 /* mark the v23 function as the older version, and v24 as the default version */
 VERSION_SYMBOL(rte_tel_data_add_dict_int, _v23, 23);
 BIND_DEFAULT_SYMBOL(rte_tel_data_add_dict_int, _v24, 24);
+#ifndef RTE_TOOLCHAIN_MSVC
 MAP_STATIC_SYMBOL(int rte_tel_data_add_dict_int(struct rte_tel_data *d,
 		const char *name, int64_t val), rte_tel_data_add_dict_int_v24);
+#else
+int
+rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int64_t val)
+{
+	return rte_tel_data_add_dict_int_v24(d, name, val);
+}
+#endif
 
 int
 rte_tel_data_add_dict_uint(struct rte_tel_data *d,

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-05-02 03:16 UTC #167 lore raw

PATCH v8 13/14 eal: always define MSVC as little endian

In reply to Tyler Retzlaff

The MSVC compiler does not target big endian platforms so define little endian always.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_byteorder.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/generic/rte_byteorder.h +2 -0

index 1e32a1b..375f118 100644
@@ -45,6 +45,8 @@
 #define RTE_BYTE_ORDER RTE_BIG_ENDIAN
 #elif defined __LITTLE_ENDIAN__
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#elif defined RTE_TOOLCHAIN_MSVC
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
 #if !defined(RTE_BYTE_ORDER)
 #error Unknown endianness.

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-05-02 03:17 UTC #168 lore raw

PATCH v8 14/14 eal: do not define typeof macro when building with MSVC

In reply to Tyler Retzlaff

When building with MSVC do not assume typeof is a macro and don't define a typeof macro that conflicts with C23 typeof keyword.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_common.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/rte_common.h +2 -0

index 0c55a23..ce66287 100644
@@ -24,9 +24,11 @@
 /* OS specific include */
 #include <rte_os.h>
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifndef typeof
 #define typeof __typeof__
 #endif
+#endif
 
 #ifndef __cplusplus
 #ifndef asm

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:49 UTC #169 lore raw

v9 cover msvc integration changes

In reply to Tyler Retzlaff

In accordance with draft plan
http://mails.dpdk.org/archives/web/2023-February/002023.html
introduces conditionally compiled code to enable building with MSVC that _does not_ require C99/C11 meaning it can be integrated now.

This series covers minimal changes for item #2 in draft plan for EAL dependencies kvargs, telemetry and consumed EAL public headers.

Note if any patch in the series requires in-depth discussion I'll detach it from this series for separate submission & more focused discussion so it doesn't block the entire series.

Note other "common" intrinsics were investigated for viability but were not adopted either because they were not available on older versions of gcc or because they generate different code for msvc vs gcc.

v9:
* invert macro conditional evaluation and use to be #ifdef RTE_TOOLCHAIN_MSVC instead of #ifndef RTE_TOOLCHAIN_MSVC wherever it doesn't make evaluation more complex than it already is

v8:
* use waitpkg intrinsics if __WAITPKG__ is defined by the compiler

v7:
* remove patch typedef of rte_cpuflags_t as an int. for now we will just allow warning to be emitted to avoid adding conditional compilation

v6:
* convert / expand likely/unlikely macros to _Bool type
* expand __extension__ empty it's cleaner to do this for now instead of modifying the whole tree
* provide msvc container_of not based on gcc expression statement extension
* add patch that stops defining typeof (C23 keyword) when building with msvc

v5:
* remove accidental line removal in barrier patch
* update prefetch patch to use intrinsics for all toolchains
* remove x86 specific changes for byte swap patch since msvc intrinsics are processor agnostic always use intrinsics from generic include
* expand __rte_packed empty for msvc packing will be addressed in a separate series

v4:
* update rdtsc patch to use gcc intrinsics
* update rtm patch to use gcc intrinsics
* drop patch disable json print formatting, we will utilize series removing VLAs from Bruce
* added patch using prefetch intrinsics for msvc
* added patch using byte swap intrinsics for msvc
* added patch hiding typdefs for msvc using gcc vector extension
* added patch defining MSVC as little endian always

v3:
* v3 does not group together conditional blocks when experimented with it didn't reduce conditionals enough to make it worth while. once msvc tests are at a running point i suggest a narrow targeted discussion about code organization without blocking this series
* v3 does not attempt to refactor to use intrinsics for non-msvc compilers. again this should be done as a separate follow-up series later if desired
* fix expansion of likely and unlikely macros
* remove unnecessary define for rte_smp_{r,w}mb it is sufficient for these to be compiler barriers on x86
* add a new patch to use __cpuid and __cpuidex intrinsics when building with msvc
* add a new patch to use _umonitor, _umwait and _tpause intrinsics when building with msvc

v2:
* use _mm_{l,s,m}fence intrinsics for rte_smp_{r,w,}mb macros are intended to be memory barriers not compiler barriers on x86_64

Tyler Retzlaff (14):
eal: use rdtsc intrinsic
eal: use rtm and xtest intrinsics
eal: use barrier intrinsics
eal: use cpuid and cpuidex intrinsics
eal: use umonitor umwait and tpause intrinsics
eal: use prefetch intrinsics
eal: use byte swap intrinsics
eal: hide GCC extension based alignment markers
eal: hide typedefs based on GCC vector extensions
eal: expand most macros to empty when using MSVC
eal: exclude exposure of rte atomic APIs for MSVC builds
telemetry: avoid expanding versioned symbol macros on MSVC
eal: always define MSVC as little endian
eal: do not define typeof macro when building with MSVC

 config/x86/meson.build                  |  6 ++++
 lib/eal/include/generic/rte_atomic.h    | 11 ++++++
 lib/eal/include/generic/rte_byteorder.h | 13 +++++++
 lib/eal/include/generic/rte_vect.h      |  4 +++
 lib/eal/include/rte_branch_prediction.h |  8 +++++
 lib/eal/include/rte_common.h            | 60 +++++++++++++++++++++++++++++++++
 lib/eal/include/rte_compat.h            | 20 +++++++++++
 lib/eal/x86/include/rte_atomic.h        |  8 +++++
 lib/eal/x86/include/rte_byteorder.h     |  4 +++
 lib/eal/x86/include/rte_cycles.h        | 14 ++++----
 lib/eal/x86/include/rte_prefetch.h      | 25 +++++++++++---
 lib/eal/x86/include/rte_rtm.h           | 18 +++-------
 lib/eal/x86/rte_cpuflags.c              |  6 +++-
 lib/eal/x86/rte_cpuid.h                 |  7 ++++
 lib/eal/x86/rte_cycles.c                | 36 ++++++++++++++++++++
 lib/eal/x86/rte_hypervisor.c            |  4 +++
 lib/eal/x86/rte_power_intrinsics.c      | 12 +++++++
 lib/telemetry/telemetry_data.c          | 16 +++++++++
 18 files changed, 248 insertions(+), 24 deletions(-)

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:49 UTC #170 lore raw

PATCH v9 1/14 eal: use rdtsc intrinsic

Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/x86/include/rte_cycles.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

lib/eal/x86/include/rte_cycles.h +8 -6

index a461a4d..ca0fb10 100644
@@ -6,6 +6,12 @@
 #ifndef _RTE_CYCLES_X86_64_H_
 #define _RTE_CYCLES_X86_64_H_
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#include <intrin.h>
+#else
+#include <x86intrin.h>
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif

@@ -23,6 +29,7 @@
 static inline uint64_t
 rte_rdtsc(void)
 {
+#ifdef RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT
 	union {
 		uint64_t tsc_64;
 		RTE_STD_C11

@@ -32,7 +39,6 @@
 		};
 	} tsc;
 
-#ifdef RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT
 	if (unlikely(rte_cycles_vmware_tsc_map)) {
 		/* ecx = 0x10000 corresponds to the physical TSC for VMware */
 		asm volatile("rdpmc" :

@@ -42,11 +48,7 @@
 		return tsc.tsc_64;
 	}
 #endif
-
-	asm volatile("rdtsc" :
-		     "=a" (tsc.lo_32),
-		     "=d" (tsc.hi_32));
-	return tsc.tsc_64;
+	return __rdtsc();
 }
 
 static inline uint64_t

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:49 UTC #171 lore raw

PATCH v9 2/14 eal: use rtm and xtest intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64. Convert code to use _xend, _xabort and _xtest intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 config/x86/meson.build        |  6 ++++++
 lib/eal/x86/include/rte_rtm.h | 18 +++++-------------
 2 files changed, 11 insertions(+), 13 deletions(-)

config/x86/meson.build +6 -0

index 54345c4..4c0b06c 100644
@@ -30,6 +30,12 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
     machine_args += '-msse4'
 endif
 
+# enable restricted transactional memory intrinsics
+# https://gcc.gnu.org/onlinedocs/gcc/x86-transactional-memory-intrinsics.html
+if cc.get_id() != 'msvc'
+    machine_args += '-mrtm'
+endif
+
 base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
 foreach f:base_flags
     compile_time_cpuflags += ['RTE_CPUFLAG_' + f]

lib/eal/x86/include/rte_rtm.h +5 ~~-13~~

index 36bf498..b84e58e 100644
@@ -5,6 +5,7 @@
 #ifndef _RTE_RTM_H_
 #define _RTE_RTM_H_ 1
 
+#include <immintrin.h>
 
 /* Official RTM intrinsics interface matching gcc/icc, but works
    on older gcc compatible compilers and binutils. */

@@ -28,31 +29,22 @@
 static __rte_always_inline
 unsigned int rte_xbegin(void)
 {
-	unsigned int ret = RTE_XBEGIN_STARTED;
-
-	asm volatile(".byte 0xc7,0xf8 ; .long 0" : "+a" (ret) :: "memory");
-	return ret;
+	return _xbegin();
 }
 
 static __rte_always_inline
 void rte_xend(void)
 {
-	 asm volatile(".byte 0x0f,0x01,0xd5" ::: "memory");
+	_xend();
 }
 
 /* not an inline function to workaround a clang bug with -O0 */
-#define rte_xabort(status) do { \
-	asm volatile(".byte 0xc6,0xf8,%P0" :: "i" (status) : "memory"); \
-} while (0)
+#define rte_xabort(status) _xabort(status)
 
 static __rte_always_inline
 int rte_xtest(void)
 {
-	unsigned char out;
-
-	asm volatile(".byte 0x0f,0x01,0xd6 ; setnz %0" :
-		"=r" (out) :: "memory");
-	return out;
+	return _xtest();
 }
 
 #ifdef __cplusplus

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:49 UTC #172 lore raw

PATCH v9 3/14 eal: use barrier intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead expand rte_compiler_barrier as _ReadWriteBarrier and for rte_smp_mb _m_mfence intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_atomic.h | 4 ++++
 lib/eal/x86/include/rte_atomic.h     | 4 ++++
 2 files changed, 8 insertions(+)

lib/eal/include/generic/rte_atomic.h +4 -0

index 58df843..6a6c394 100644
@@ -116,9 +116,13 @@
  * Guarantees that operation reordering does not occur at compile time
  * for operations directly before and after the barrier.
  */
+#ifdef RTE_TOOLCHAIN_MSVC
+#define rte_compiler_barrier() _ReadWriteBarrier()
+#else
 #define	rte_compiler_barrier() do {		\
 	asm volatile ("" : : : "memory");	\
 } while(0)
+#endif
 
 /**
  * Synchronization fence between threads based on the specified memory order.

lib/eal/x86/include/rte_atomic.h +4 -0

index f2ee1a9..7aba1c3 100644
@@ -66,11 +66,15 @@
 static __rte_always_inline void
 rte_smp_mb(void)
 {
+#ifdef RTE_TOOLCHAIN_MSVC
+	_mm_mfence();
+#else
 #ifdef RTE_ARCH_I686
 	asm volatile("lock addl $0, -128(%%esp); " ::: "memory");
 #else
 	asm volatile("lock addl $0, -128(%%rsp); " ::: "memory");
 #endif
+#endif
 }
 
 #define rte_io_mb() rte_mb()

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:49 UTC #173 lore raw

PATCH v9 4/14 eal: use cpuid and cpuidex intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use __cpuid and __cpuidex intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/x86/rte_cpuflags.c   |  6 +++++-
 lib/eal/x86/rte_cpuid.h      |  7 +++++++
 lib/eal/x86/rte_cycles.c     | 36 ++++++++++++++++++++++++++++++++++++
 lib/eal/x86/rte_hypervisor.c |  4 ++++
 4 files changed, 52 insertions(+), 1 deletion(-)

lib/eal/x86/rte_cpuflags.c +5 -1

index d6b5182..8a30f79 100644
@@ -165,9 +165,13 @@ struct feature_entry {
 	if (maxleaf < feat->leaf)
 		return 0;
 
-	 __cpuid_count(feat->leaf, feat->subleaf,
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuidex(regs, feat->leaf, feat->subleaf);
+#else
+	__cpuid_count(feat->leaf, feat->subleaf,
 			 regs[RTE_REG_EAX], regs[RTE_REG_EBX],
 			 regs[RTE_REG_ECX], regs[RTE_REG_EDX]);
+#endif
 
 	/* check if the feature is enabled */
 	return (regs[feat->reg] >> feat->bit) & 1;

lib/eal/x86/rte_cpuid.h +7 -0

index b773ad9..c6abaad 100644
@@ -5,7 +5,9 @@
 #ifndef RTE_CPUID_H
 #define RTE_CPUID_H
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #include <cpuid.h>
+#endif
 
 enum cpu_register_t {
 	RTE_REG_EAX = 0,

@@ -16,4 +18,9 @@ enum cpu_register_t {
 
 typedef uint32_t cpuid_registers_t[4];
 
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+__get_cpuid_max(unsigned int e, unsigned int *s);
+#endif
+
 #endif /* RTE_CPUID_H */

lib/eal/x86/rte_cycles.c +36 -0

index 0e695ca..69ed59b 100644
@@ -4,7 +4,11 @@
 
 #include <fcntl.h>
 #include <unistd.h>
+#ifdef RTE_TOOLCHAIN_MSVC
+#define bit_AVX (1 << 28)
+#else
 #include <cpuid.h>
+#endif
 
 
 #include "eal_private.h"

@@ -82,9 +86,25 @@
 	return 0;
 }
 
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+__get_cpuid_max(unsigned int e, unsigned int *s)
+{
+	uint32_t cpuinfo[4];
+
+	__cpuid(cpuinfo, e);
+	if (s)
+		*s = cpuinfo[1];
+	return cpuinfo[0];
+}
+#endif
+
 uint64_t
 get_tsc_freq_arch(void)
 {
+#ifdef RTE_TOOLCHAIN_MSVC
+	int cpuinfo[4];
+#endif
 	uint64_t tsc_hz = 0;
 	uint32_t a, b, c, d, maxleaf;
 	uint8_t mult, model;

@@ -97,14 +117,30 @@
 	maxleaf = __get_cpuid_max(0, NULL);
 
 	if (maxleaf >= 0x15) {
+#ifdef RTE_TOOLCHAIN_MSVC
+		__cpuid(cpuinfo, 0x15);
+		a = cpuinfo[0];
+		b = cpuinfo[1];
+		c = cpuinfo[2];
+		d = cpuinfo[3];
+#else
 		__cpuid(0x15, a, b, c, d);
+#endif
 
 		/* EBX : TSC/Crystal ratio, ECX : Crystal Hz */
 		if (b && c)
 			return c * (b / a);
 	}
 
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuid(cpuinfo, 0x1);
+	a = cpuinfo[0];
+	b = cpuinfo[1];
+	c = cpuinfo[2];
+	d = cpuinfo[3];
+#else
 	__cpuid(0x1, a, b, c, d);
+#endif
 	model = rte_cpu_get_model(a);
 
 	if (check_model_wsm_nhm(model))

lib/eal/x86/rte_hypervisor.c +4 -0

index c38cfc0..04fe767 100644
@@ -23,9 +23,13 @@ enum rte_hypervisor
 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_HYPERVISOR))
 		return RTE_HYPERVISOR_NONE;
 
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuid(regs, HYPERVISOR_INFO_LEAF);
+#else
 	__cpuid(HYPERVISOR_INFO_LEAF,
 			regs[RTE_REG_EAX], regs[RTE_REG_EBX],
 			regs[RTE_REG_ECX], regs[RTE_REG_EDX]);
+#endif
 	for (reg = 1; reg < 4; reg++)
 		memcpy(name + (reg - 1) * 4, &regs[reg], 4);
 	name[12] = '\0';

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #174 lore raw

PATCH v9 5/14 eal: use umonitor umwait and tpause intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _umonitor, _umwait and _tpause intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

lib/eal/x86/rte_power_intrinsics.c +12 -0

index f749da9..4066d13 100644
@@ -109,9 +109,13 @@
 	 */
 
 	/* set address for UMONITOR */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umonitor(pmc->addr);
+#else
 	asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
 			:
 			: "D"(pmc->addr));
+#endif
 
 	/* now that we've put this address into monitor, we can unlock */
 	rte_spinlock_unlock(&s->lock);

@@ -123,10 +127,14 @@
 		goto end;
 
 	/* execute UMWAIT */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umwait(tsc_l, tsc_h);
+#else
 	asm volatile(".byte 0xf2, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */
 			  "a"(tsc_l), "d"(tsc_h));
+#endif
 
 end:
 	/* erase sleep address */

@@ -153,10 +161,14 @@
 		return -ENOTSUP;
 
 	/* execute TPAUSE */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_tpause(tsc_l, tsc_h);
+#else
 	asm volatile(".byte 0x66, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */
 			"a"(tsc_l), "d"(tsc_h));
+#endif
 
 	return 0;
 }

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:49 UTC #175 lore raw

PATCH v9 6/14 eal: use prefetch intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _mm_prefetch and _mm_cldemote intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

---
 lib/eal/x86/include/rte_prefetch.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

lib/eal/x86/include/rte_prefetch.h +21 -4

index 7fd01c4..7a6988e 100644
@@ -9,30 +9,38 @@
 extern "C" {
 #endif
 
+#include <emmintrin.h>
+
 #include <rte_compat.h>
 #include <rte_common.h>
 #include "generic/rte_prefetch.h"
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-qual"
+
 static inline void rte_prefetch0(const volatile void *p)
 {
-	asm volatile ("prefetcht0 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T0);
 }
 
 static inline void rte_prefetch1(const volatile void *p)
 {
-	asm volatile ("prefetcht1 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T1);
 }
 
 static inline void rte_prefetch2(const volatile void *p)
 {
-	asm volatile ("prefetcht2 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T2);
 }
 
 static inline void rte_prefetch_non_temporal(const volatile void *p)
 {
-	asm volatile ("prefetchnta %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_NTA);
 }
 
+#pragma GCC diagnostic pop
+
+#ifdef RTE_TOOLCHAIN_MSVC
 /*
  * We use raw byte codes for now as only the newest compiler
  * versions support this instruction natively.

@@ -41,8 +49,17 @@ static inline void rte_prefetch_non_temporal(const volatile void *p)
 static inline void
 rte_cldemote(const volatile void *p)
 {
+	_mm_cldemote(p);
+}
+#else
+__rte_experimental
+static inline void
+rte_cldemote(const volatile void *p)
+{
 	asm volatile(".byte 0x0f, 0x1c, 0x06" :: "S" (p));
 }
+#endif
+
 
 #ifdef __cplusplus
 }

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #176 lore raw

PATCH v9 7/14 eal: use byte swap intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead expand use _byteswap_u{ushort,ulong,uint64} intrinsics instead.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_byteorder.h | 11 +++++++++++
 lib/eal/x86/include/rte_byteorder.h     |  4 ++++
 2 files changed, 15 insertions(+)

lib/eal/include/generic/rte_byteorder.h +11 -0

index a67e1d7..1e32a1b 100644
@@ -234,6 +234,7 @@
 #endif /* __DOXYGEN__ */
 
 #ifdef RTE_FORCE_INTRINSICS
+#ifndef RTE_TOOLCHAIN_MSVC
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
 #define rte_bswap16(x) __builtin_bswap16(x)
 #endif

@@ -241,6 +242,16 @@
 #define rte_bswap32(x) __builtin_bswap32(x)
 
 #define rte_bswap64(x) __builtin_bswap64(x)
+#else
+/*
+ * note: we may want to #pragma intrsinsic(_byteswap_u{short,long,uint64})
+ */
+#define rte_bswap16(x) _byteswap_ushort(x)
+
+#define rte_bswap32(x) _byteswap_ulong(x)
+
+#define rte_bswap64(x) _byteswap_uint64(x)
+#endif
 
 #endif

lib/eal/x86/include/rte_byteorder.h +4 -0

index a2dfecc..3d6e58e 100644
@@ -18,6 +18,7 @@
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
 
+#ifndef RTE_TOOLCHAIN_MSVC
 /*
  * An architecture-optimized byte swap for a 16-bit value.
  *

@@ -69,6 +70,7 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 				   rte_arch_bswap16(x)))
 #endif
 #endif
+#endif
 
 #define rte_cpu_to_le_16(x) (x)
 #define rte_cpu_to_le_32(x) (x)

@@ -86,11 +88,13 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 #define rte_be_to_cpu_32(x) rte_bswap32(x)
 #define rte_be_to_cpu_64(x) rte_bswap64(x)
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifdef RTE_ARCH_I686
 #include "rte_byteorder_32.h"
 #else
 #include "rte_byteorder_64.h"
 #endif
+#endif
 
 #ifdef __cplusplus
 }

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:49 UTC #177 lore raw

PATCH v9 8/14 eal: hide GCC extension based alignment markers

In reply to Tyler Retzlaff

When compiling with MSVC don't expose typedefs used as alignment markers.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_common.h | 4 ++++
 1 file changed, 4 insertions(+)

lib/eal/include/rte_common.h +4 -0

index 15765b4..2f464e3 100644
@@ -460,6 +460,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
 
 /*********** Structure alignment markers ********/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /** Generic marker for any place in a structure. */
 __extension__ typedef void    *RTE_MARKER[0];
 /** Marker for 1B alignment in a structure. */

@@ -471,6 +473,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
 /** Marker for 8B alignment in a structure. */
 __extension__ typedef uint64_t RTE_MARKER64[0];
 
+#endif
+
 /**
  * Combines 32b inputs most significant set bits into the least
  * significant bits to construct a value with the same MSBs as x

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #178 lore raw

PATCH v9 9/14 eal: hide typedefs based on GCC vector extensions

In reply to Tyler Retzlaff

When compiling with MSVC don't expose typedefs based on GCC vector extensions.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_vect.h | 4 ++++
 1 file changed, 4 insertions(+)

lib/eal/include/generic/rte_vect.h +4 -0

index 3fec2bf..777510c 100644
@@ -17,6 +17,8 @@
 
 #include <rte_compat.h>
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /* Unsigned vector types */
 
 /**

@@ -186,6 +188,8 @@
  */
 typedef int64_t rte_v256s64_t __attribute__((vector_size(32), aligned(32)));
 
+#endif
+
 /**
  * The max SIMD bitwidth value to limit vector path selection.
  */

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #179 lore raw

PATCH v9 10/14 eal: expand most macros to empty when using MSVC

In reply to Tyler Retzlaff

Later we will return and provide (where possible) expansions that work correctly for msvc and where not possible provide some alternate macros to achieve the same outcome.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_branch_prediction.h |  8 +++++
 lib/eal/include/rte_common.h            | 54 +++++++++++++++++++++++++++++++++
 lib/eal/include/rte_compat.h            | 20 ++++++++++++
 3 files changed, 82 insertions(+)

lib/eal/include/rte_branch_prediction.h +8 -0

index 414cd92..c0356ca 100644
@@ -24,7 +24,11 @@
  *      do_stuff();
  */
 #ifndef likely
+#ifdef RTE_TOOLCHAIN_MSVC
+#define likely(x)	(!!(x))
+#else
 #define likely(x)	__builtin_expect(!!(x), 1)
+#endif
 #endif /* likely */
 
 /**

@@ -37,7 +41,11 @@
  *      do_stuff();
  */
 #ifndef unlikely
+#ifdef RTE_TOOLCHAIN_MSVC
+#define unlikely(x)	(!!(x))
+#else
 #define unlikely(x)	__builtin_expect(!!(x), 0)
+#endif
 #endif /* unlikely */
 
 #ifdef __cplusplus

lib/eal/include/rte_common.h +54 -0

147 lines not on this page · raw message

lib/eal/include/rte_compat.h +20 -0

index fc9fbaa..716bc03 100644
@@ -12,14 +12,22 @@
 
 #ifndef ALLOW_EXPERIMENTAL_API
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_experimental
+#else
 #define __rte_experimental \
 __attribute__((deprecated("Symbol is not yet part of stable ABI"), \
 section(".text.experimental")))
+#endif
 
 #else
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_experimental
+#else
 #define __rte_experimental \
 __attribute__((section(".text.experimental")))
+#endif
 
 #endif

@@ -30,23 +38,35 @@
 
 #if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 __attribute__((error("Symbol is not public ABI"), \
 section(".text.internal")))
+#endif
 
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 _Pragma("GCC diagnostic push") \
 _Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
 section(".text.internal"))) \
 _Pragma("GCC diagnostic pop")
+#endif
 
 #else
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 __attribute__((section(".text.internal")))
+#endif
 
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #180 lore raw

PATCH v9 11/14 eal: exclude exposure of rte atomic APIs for MSVC builds

In reply to Tyler Retzlaff

It's discouraged to use rte_atomics APIs instead standard APIs should be used from C11. Since MSVC is a new toolchain/platform combination block visibility of the rte_atomic APIs from day 1.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_atomic.h | 7 +++++++
 lib/eal/x86/include/rte_atomic.h     | 4 ++++
 2 files changed, 11 insertions(+)

lib/eal/include/generic/rte_atomic.h +7 -0

index 6a6c394..0112208 100644
@@ -131,6 +131,8 @@
 
 /*------------------------- 16 bit atomic operations -------------------------*/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Atomic compare and set.
  *

@@ -1038,8 +1040,11 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 }
 #endif
 
+#endif
+
 /*------------------------ 128 bit atomic operations -------------------------*/
 
+
 /**
  * 128-bit integer structure.
  */

@@ -1049,8 +1054,10 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 	union {
 		uint64_t val[2];
 #ifdef RTE_ARCH_64
+#ifndef RTE_TOOLCHAIN_MSVC
 		__extension__ __int128 int128;
 #endif
+#endif
 	};
 } __rte_aligned(16) rte_int128_t;

lib/eal/x86/include/rte_atomic.h +4 -0

index 7aba1c3..9ba61f8 100644
@@ -83,6 +83,8 @@
 
 #define rte_io_rmb() rte_compiler_barrier()
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Synchronization fence between threads based on the specified memory order.
  *

@@ -279,6 +281,8 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 #include "rte_atomic_64.h"
 #endif
 
+#endif
+
 #ifdef __cplusplus
 }
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #181 lore raw

PATCH v9 12/14 telemetry: avoid expanding versioned symbol macros on MSVC

In reply to Tyler Retzlaff

Windows does not support versioned symbols. Fortunately Windows also doesn't have an exported stable ABI.

Export rte_tel_data_add_array_int -> rte_tel_data_add_array_int_24 and rte_tel_data_add_dict_int -> rte_tel_data_add_dict_int_v24 functions.

Windows does have a way to achieve similar versioning for symbols but it is not a simple #define so it will be done as a work package later.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/telemetry/telemetry_data.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

lib/telemetry/telemetry_data.c +16 -0

index 0c7187b..523287b 100644
@@ -83,8 +83,16 @@
 /* mark the v23 function as the older version, and v24 as the default version */
 VERSION_SYMBOL(rte_tel_data_add_array_int, _v23, 23);
 BIND_DEFAULT_SYMBOL(rte_tel_data_add_array_int, _v24, 24);
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+rte_tel_data_add_array_int(struct rte_tel_data *d, int64_t x)
+{
+	return rte_tel_data_add_array_int_v24(d, x);
+}
+#else
 MAP_STATIC_SYMBOL(int rte_tel_data_add_array_int(struct rte_tel_data *d,
 		int64_t x), rte_tel_data_add_array_int_v24);
+#endif
 
 int
 rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)

@@ -218,8 +226,16 @@
 /* mark the v23 function as the older version, and v24 as the default version */
 VERSION_SYMBOL(rte_tel_data_add_dict_int, _v23, 23);
 BIND_DEFAULT_SYMBOL(rte_tel_data_add_dict_int, _v24, 24);
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int64_t val)
+{
+	return rte_tel_data_add_dict_int_v24(d, name, val);
+}
+#else
 MAP_STATIC_SYMBOL(int rte_tel_data_add_dict_int(struct rte_tel_data *d,
 		const char *name, int64_t val), rte_tel_data_add_dict_int_v24);
+#endif
 
 int
 rte_tel_data_add_dict_uint(struct rte_tel_data *d,

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #182 lore raw

PATCH v9 13/14 eal: always define MSVC as little endian

In reply to Tyler Retzlaff

The MSVC compiler does not target big endian platforms so define little endian always.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_byteorder.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/generic/rte_byteorder.h +2 -0

index 1e32a1b..375f118 100644
@@ -45,6 +45,8 @@
 #define RTE_BYTE_ORDER RTE_BIG_ENDIAN
 #elif defined __LITTLE_ENDIAN__
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#elif defined RTE_TOOLCHAIN_MSVC
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
 #if !defined(RTE_BYTE_ORDER)
 #error Unknown endianness.

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-07-11 16:50 UTC #183 lore raw

PATCH v9 14/14 eal: do not define typeof macro when building with MSVC

In reply to Tyler Retzlaff

When building with MSVC do not assume typeof is a macro and don't define a typeof macro that conflicts with C23 typeof keyword.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_common.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/rte_common.h +2 -0

index b087532..cf7d8d8 100644
@@ -24,9 +24,11 @@
 /* OS specific include */
 #include <rte_os.h>
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifndef typeof
 #define typeof __typeof__
 #endif
+#endif
 
 #ifndef __cplusplus
 #ifndef asm

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:35 UTC #184 lore raw

v10 cover msvc integration changes

In reply to Tyler Retzlaff

This series covers minimal changes for item #2 in draft plan for EAL dependencies kvargs, telemetry and consumed EAL public headers.

Note if any patch in the series requires in-depth discussion I'll detach it from this series for separate submission & more focused discussion so it doesn't block the entire series.

v10:
* rebase series post 23.07 release. no changes except
telemetry: avoid expanding versioned symbol macros on MSVC has been dropped from the series as 23.11 has removed the versioned symbols

v9:
* invert macro conditional evaluation and use to be #ifdef RTE_TOOLCHAIN_MSVC instead of #ifndef RTE_TOOLCHAIN_MSVC wherever it doesn't make evaluation more complex than it already is

v8:
* use waitpkg intrinsics if __WAITPKG__ is defined by the compiler

v7:
* remove patch typedef of rte_cpuflags_t as an int. for now we will just allow warning to be emitted to avoid adding conditional compilation

v2:
* use _mm_{l,s,m}fence intrinsics for rte_smp_{r,w,}mb macros are intended to be memory barriers not compiler barriers on x86_64

Tyler Retzlaff (13):
eal: use rdtsc intrinsic
eal: use rtm and xtest intrinsics
eal: use barrier intrinsics
eal: use cpuid and cpuidex intrinsics
eal: use umonitor umwait and tpause intrinsics
eal: use prefetch intrinsics
eal: use byte swap intrinsics
eal: hide GCC extension based alignment markers
eal: hide typedefs based on GCC vector extensions
eal: expand most macros to empty when using MSVC
eal: exclude exposure of rte atomic APIs for MSVC builds
eal: always define MSVC as little endian
eal: do not define typeof macro when building with MSVC

 config/x86/meson.build                  |  6 ++++
 lib/eal/include/generic/rte_atomic.h    | 11 ++++++
 lib/eal/include/generic/rte_byteorder.h | 13 +++++++
 lib/eal/include/generic/rte_vect.h      |  4 +++
 lib/eal/include/rte_branch_prediction.h |  8 +++++
 lib/eal/include/rte_common.h            | 60 +++++++++++++++++++++++++++++++++
 lib/eal/include/rte_compat.h            | 20 +++++++++++
 lib/eal/x86/include/rte_atomic.h        |  8 +++++
 lib/eal/x86/include/rte_byteorder.h     |  4 +++
 lib/eal/x86/include/rte_cycles.h        | 14 ++++----
 lib/eal/x86/include/rte_prefetch.h      | 25 +++++++++++---
 lib/eal/x86/include/rte_rtm.h           | 18 +++-------
 lib/eal/x86/rte_cpuflags.c              |  6 +++-
 lib/eal/x86/rte_cpuid.h                 |  7 ++++
 lib/eal/x86/rte_cycles.c                | 36 ++++++++++++++++++++
 lib/eal/x86/rte_hypervisor.c            |  4 +++
 lib/eal/x86/rte_power_intrinsics.c      | 12 +++++++
 17 files changed, 232 insertions(+), 24 deletions(-)

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:35 UTC #185 lore raw

PATCH v10 1/13 eal: use rdtsc intrinsic

Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/x86/include/rte_cycles.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

lib/eal/x86/include/rte_cycles.h +8 -6

index a461a4d..ca0fb10 100644
@@ -6,6 +6,12 @@
 #ifndef _RTE_CYCLES_X86_64_H_
 #define _RTE_CYCLES_X86_64_H_
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#include <intrin.h>
+#else
+#include <x86intrin.h>
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif

@@ -23,6 +29,7 @@
 static inline uint64_t
 rte_rdtsc(void)
 {
+#ifdef RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT
 	union {
 		uint64_t tsc_64;
 		RTE_STD_C11

@@ -32,7 +39,6 @@
 		};
 	} tsc;
 
-#ifdef RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT
 	if (unlikely(rte_cycles_vmware_tsc_map)) {
 		/* ecx = 0x10000 corresponds to the physical TSC for VMware */
 		asm volatile("rdpmc" :

@@ -42,11 +48,7 @@
 		return tsc.tsc_64;
 	}
 #endif
-
-	asm volatile("rdtsc" :
-		     "=a" (tsc.lo_32),
-		     "=d" (tsc.hi_32));
-	return tsc.tsc_64;
+	return __rdtsc();
 }
 
 static inline uint64_t

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:35 UTC #186 lore raw

PATCH v10 2/13 eal: use rtm and xtest intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64. Convert code to use _xend, _xabort and _xtest intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 config/x86/meson.build        |  6 ++++++
 lib/eal/x86/include/rte_rtm.h | 18 +++++-------------
 2 files changed, 11 insertions(+), 13 deletions(-)

config/x86/meson.build +6 -0

index 54345c4..4c0b06c 100644
@@ -30,6 +30,12 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
     machine_args += '-msse4'
 endif
 
+# enable restricted transactional memory intrinsics
+# https://gcc.gnu.org/onlinedocs/gcc/x86-transactional-memory-intrinsics.html
+if cc.get_id() != 'msvc'
+    machine_args += '-mrtm'
+endif
+
 base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
 foreach f:base_flags
     compile_time_cpuflags += ['RTE_CPUFLAG_' + f]

lib/eal/x86/include/rte_rtm.h +5 ~~-13~~

index 36bf498..b84e58e 100644
@@ -5,6 +5,7 @@
 #ifndef _RTE_RTM_H_
 #define _RTE_RTM_H_ 1
 
+#include <immintrin.h>
 
 /* Official RTM intrinsics interface matching gcc/icc, but works
    on older gcc compatible compilers and binutils. */

@@ -28,31 +29,22 @@
 static __rte_always_inline
 unsigned int rte_xbegin(void)
 {
-	unsigned int ret = RTE_XBEGIN_STARTED;
-
-	asm volatile(".byte 0xc7,0xf8 ; .long 0" : "+a" (ret) :: "memory");
-	return ret;
+	return _xbegin();
 }
 
 static __rte_always_inline
 void rte_xend(void)
 {
-	 asm volatile(".byte 0x0f,0x01,0xd5" ::: "memory");
+	_xend();
 }
 
 /* not an inline function to workaround a clang bug with -O0 */
-#define rte_xabort(status) do { \
-	asm volatile(".byte 0xc6,0xf8,%P0" :: "i" (status) : "memory"); \
-} while (0)
+#define rte_xabort(status) _xabort(status)
 
 static __rte_always_inline
 int rte_xtest(void)
 {
-	unsigned char out;
-
-	asm volatile(".byte 0x0f,0x01,0xd6 ; setnz %0" :
-		"=r" (out) :: "memory");
-	return out;
+	return _xtest();
 }
 
 #ifdef __cplusplus

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:35 UTC #187 lore raw

PATCH v10 3/13 eal: use barrier intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead expand rte_compiler_barrier as _ReadWriteBarrier and for rte_smp_mb _m_mfence intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_atomic.h | 4 ++++
 lib/eal/x86/include/rte_atomic.h     | 4 ++++
 2 files changed, 8 insertions(+)

lib/eal/include/generic/rte_atomic.h +4 -0

index aef44e2..2d36eb0 100644
@@ -146,9 +146,13 @@
  * Guarantees that operation reordering does not occur at compile time
  * for operations directly before and after the barrier.
  */
+#ifdef RTE_TOOLCHAIN_MSVC
+#define rte_compiler_barrier() _ReadWriteBarrier()
+#else
 #define	rte_compiler_barrier() do {		\
 	asm volatile ("" : : : "memory");	\
 } while(0)
+#endif
 
 /**
  * Synchronization fence between threads based on the specified memory order.

lib/eal/x86/include/rte_atomic.h +4 -0

index f2ee1a9..7aba1c3 100644
@@ -66,11 +66,15 @@
 static __rte_always_inline void
 rte_smp_mb(void)
 {
+#ifdef RTE_TOOLCHAIN_MSVC
+	_mm_mfence();
+#else
 #ifdef RTE_ARCH_I686
 	asm volatile("lock addl $0, -128(%%esp); " ::: "memory");
 #else
 	asm volatile("lock addl $0, -128(%%rsp); " ::: "memory");
 #endif
+#endif
 }
 
 #define rte_io_mb() rte_mb()

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:35 UTC #188 lore raw

PATCH v10 4/13 eal: use cpuid and cpuidex intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use __cpuid and __cpuidex intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/x86/rte_cpuflags.c   |  6 +++++-
 lib/eal/x86/rte_cpuid.h      |  7 +++++++
 lib/eal/x86/rte_cycles.c     | 36 ++++++++++++++++++++++++++++++++++++
 lib/eal/x86/rte_hypervisor.c |  4 ++++
 4 files changed, 52 insertions(+), 1 deletion(-)

lib/eal/x86/rte_cpuflags.c +5 -1

index d6b5182..8a30f79 100644
@@ -165,9 +165,13 @@ struct feature_entry {
 	if (maxleaf < feat->leaf)
 		return 0;
 
-	 __cpuid_count(feat->leaf, feat->subleaf,
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuidex(regs, feat->leaf, feat->subleaf);
+#else
+	__cpuid_count(feat->leaf, feat->subleaf,
 			 regs[RTE_REG_EAX], regs[RTE_REG_EBX],
 			 regs[RTE_REG_ECX], regs[RTE_REG_EDX]);
+#endif
 
 	/* check if the feature is enabled */
 	return (regs[feat->reg] >> feat->bit) & 1;

lib/eal/x86/rte_cpuid.h +7 -0

index b773ad9..c6abaad 100644
@@ -5,7 +5,9 @@
 #ifndef RTE_CPUID_H
 #define RTE_CPUID_H
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #include <cpuid.h>
+#endif
 
 enum cpu_register_t {
 	RTE_REG_EAX = 0,

@@ -16,4 +18,9 @@ enum cpu_register_t {
 
 typedef uint32_t cpuid_registers_t[4];
 
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+__get_cpuid_max(unsigned int e, unsigned int *s);
+#endif
+
 #endif /* RTE_CPUID_H */

lib/eal/x86/rte_cycles.c +36 -0

index 0e695ca..69ed59b 100644
@@ -4,7 +4,11 @@
 
 #include <fcntl.h>
 #include <unistd.h>
+#ifdef RTE_TOOLCHAIN_MSVC
+#define bit_AVX (1 << 28)
+#else
 #include <cpuid.h>
+#endif
 
 
 #include "eal_private.h"

@@ -82,9 +86,25 @@
 	return 0;
 }
 
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+__get_cpuid_max(unsigned int e, unsigned int *s)
+{
+	uint32_t cpuinfo[4];
+
+	__cpuid(cpuinfo, e);
+	if (s)
+		*s = cpuinfo[1];
+	return cpuinfo[0];
+}
+#endif
+
 uint64_t
 get_tsc_freq_arch(void)
 {
+#ifdef RTE_TOOLCHAIN_MSVC
+	int cpuinfo[4];
+#endif
 	uint64_t tsc_hz = 0;
 	uint32_t a, b, c, d, maxleaf;
 	uint8_t mult, model;

@@ -97,14 +117,30 @@
 	maxleaf = __get_cpuid_max(0, NULL);
 
 	if (maxleaf >= 0x15) {
+#ifdef RTE_TOOLCHAIN_MSVC
+		__cpuid(cpuinfo, 0x15);
+		a = cpuinfo[0];
+		b = cpuinfo[1];
+		c = cpuinfo[2];
+		d = cpuinfo[3];
+#else
 		__cpuid(0x15, a, b, c, d);
+#endif
 
 		/* EBX : TSC/Crystal ratio, ECX : Crystal Hz */
 		if (b && c)
 			return c * (b / a);
 	}
 
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuid(cpuinfo, 0x1);
+	a = cpuinfo[0];
+	b = cpuinfo[1];
+	c = cpuinfo[2];
+	d = cpuinfo[3];
+#else
 	__cpuid(0x1, a, b, c, d);
+#endif
 	model = rte_cpu_get_model(a);
 
 	if (check_model_wsm_nhm(model))

lib/eal/x86/rte_hypervisor.c +4 -0

index c38cfc0..04fe767 100644
@@ -23,9 +23,13 @@ enum rte_hypervisor
 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_HYPERVISOR))
 		return RTE_HYPERVISOR_NONE;
 
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuid(regs, HYPERVISOR_INFO_LEAF);
+#else
 	__cpuid(HYPERVISOR_INFO_LEAF,
 			regs[RTE_REG_EAX], regs[RTE_REG_EBX],
 			regs[RTE_REG_ECX], regs[RTE_REG_EDX]);
+#endif
 	for (reg = 1; reg < 4; reg++)
 		memcpy(name + (reg - 1) * 4, &regs[reg], 4);
 	name[12] = '\0';

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #189 lore raw

PATCH v10 5/13 eal: use umonitor umwait and tpause intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _umonitor, _umwait and _tpause intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

lib/eal/x86/rte_power_intrinsics.c +12 -0

index f749da9..4066d13 100644
@@ -109,9 +109,13 @@
 	 */
 
 	/* set address for UMONITOR */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umonitor(pmc->addr);
+#else
 	asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
 			:
 			: "D"(pmc->addr));
+#endif
 
 	/* now that we've put this address into monitor, we can unlock */
 	rte_spinlock_unlock(&s->lock);

@@ -123,10 +127,14 @@
 		goto end;
 
 	/* execute UMWAIT */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umwait(tsc_l, tsc_h);
+#else
 	asm volatile(".byte 0xf2, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */
 			  "a"(tsc_l), "d"(tsc_h));
+#endif
 
 end:
 	/* erase sleep address */

@@ -153,10 +161,14 @@
 		return -ENOTSUP;
 
 	/* execute TPAUSE */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_tpause(tsc_l, tsc_h);
+#else
 	asm volatile(".byte 0x66, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */
 			"a"(tsc_l), "d"(tsc_h));
+#endif
 
 	return 0;
 }

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:35 UTC #190 lore raw

PATCH v10 6/13 eal: use prefetch intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _mm_prefetch and _mm_cldemote intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

---
 lib/eal/x86/include/rte_prefetch.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

lib/eal/x86/include/rte_prefetch.h +21 -4

index 7fd01c4..7a6988e 100644
@@ -9,30 +9,38 @@
 extern "C" {
 #endif
 
+#include <emmintrin.h>
+
 #include <rte_compat.h>
 #include <rte_common.h>
 #include "generic/rte_prefetch.h"
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-qual"
+
 static inline void rte_prefetch0(const volatile void *p)
 {
-	asm volatile ("prefetcht0 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T0);
 }
 
 static inline void rte_prefetch1(const volatile void *p)
 {
-	asm volatile ("prefetcht1 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T1);
 }
 
 static inline void rte_prefetch2(const volatile void *p)
 {
-	asm volatile ("prefetcht2 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T2);
 }
 
 static inline void rte_prefetch_non_temporal(const volatile void *p)
 {
-	asm volatile ("prefetchnta %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_NTA);
 }
 
+#pragma GCC diagnostic pop
+
+#ifdef RTE_TOOLCHAIN_MSVC
 /*
  * We use raw byte codes for now as only the newest compiler
  * versions support this instruction natively.

@@ -41,8 +49,17 @@ static inline void rte_prefetch_non_temporal(const volatile void *p)
 static inline void
 rte_cldemote(const volatile void *p)
 {
+	_mm_cldemote(p);
+}
+#else
+__rte_experimental
+static inline void
+rte_cldemote(const volatile void *p)
+{
 	asm volatile(".byte 0x0f, 0x1c, 0x06" :: "S" (p));
 }
+#endif
+
 
 #ifdef __cplusplus
 }

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #191 lore raw

PATCH v10 7/13 eal: use byte swap intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead expand use _byteswap_u{ushort,ulong,uint64} intrinsics instead.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_byteorder.h | 11 +++++++++++
 lib/eal/x86/include/rte_byteorder.h     |  4 ++++
 2 files changed, 15 insertions(+)

lib/eal/include/generic/rte_byteorder.h +11 -0

index a67e1d7..1e32a1b 100644
@@ -234,6 +234,7 @@
 #endif /* __DOXYGEN__ */
 
 #ifdef RTE_FORCE_INTRINSICS
+#ifndef RTE_TOOLCHAIN_MSVC
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
 #define rte_bswap16(x) __builtin_bswap16(x)
 #endif

@@ -241,6 +242,16 @@
 #define rte_bswap32(x) __builtin_bswap32(x)
 
 #define rte_bswap64(x) __builtin_bswap64(x)
+#else
+/*
+ * note: we may want to #pragma intrsinsic(_byteswap_u{short,long,uint64})
+ */
+#define rte_bswap16(x) _byteswap_ushort(x)
+
+#define rte_bswap32(x) _byteswap_ulong(x)
+
+#define rte_bswap64(x) _byteswap_uint64(x)
+#endif
 
 #endif

lib/eal/x86/include/rte_byteorder.h +4 -0

index a2dfecc..3d6e58e 100644
@@ -18,6 +18,7 @@
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
 
+#ifndef RTE_TOOLCHAIN_MSVC
 /*
  * An architecture-optimized byte swap for a 16-bit value.
  *

@@ -69,6 +70,7 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 				   rte_arch_bswap16(x)))
 #endif
 #endif
+#endif
 
 #define rte_cpu_to_le_16(x) (x)
 #define rte_cpu_to_le_32(x) (x)

@@ -86,11 +88,13 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 #define rte_be_to_cpu_32(x) rte_bswap32(x)
 #define rte_be_to_cpu_64(x) rte_bswap64(x)
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifdef RTE_ARCH_I686
 #include "rte_byteorder_32.h"
 #else
 #include "rte_byteorder_64.h"
 #endif
+#endif
 
 #ifdef __cplusplus
 }

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #192 lore raw

PATCH v10 8/13 eal: hide GCC extension based alignment markers

In reply to Tyler Retzlaff

When compiling with MSVC don't expose typedefs used as alignment markers.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_common.h | 4 ++++
 1 file changed, 4 insertions(+)

lib/eal/include/rte_common.h +4 -0

index 15765b4..2f464e3 100644
@@ -460,6 +460,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
 
 /*********** Structure alignment markers ********/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /** Generic marker for any place in a structure. */
 __extension__ typedef void    *RTE_MARKER[0];
 /** Marker for 1B alignment in a structure. */

@@ -471,6 +473,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
 /** Marker for 8B alignment in a structure. */
 __extension__ typedef uint64_t RTE_MARKER64[0];
 
+#endif
+
 /**
  * Combines 32b inputs most significant set bits into the least
  * significant bits to construct a value with the same MSBs as x

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #193 lore raw

PATCH v10 9/13 eal: hide typedefs based on GCC vector extensions

In reply to Tyler Retzlaff

When compiling with MSVC don't expose typedefs based on GCC vector extensions.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_vect.h | 4 ++++
 1 file changed, 4 insertions(+)

lib/eal/include/generic/rte_vect.h +4 -0

index 3fec2bf..777510c 100644
@@ -17,6 +17,8 @@
 
 #include <rte_compat.h>
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /* Unsigned vector types */
 
 /**

@@ -186,6 +188,8 @@
  */
 typedef int64_t rte_v256s64_t __attribute__((vector_size(32), aligned(32)));
 
+#endif
+
 /**
  * The max SIMD bitwidth value to limit vector path selection.
  */

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #194 lore raw

PATCH v10 10/13 eal: expand most macros to empty when using MSVC

In reply to Tyler Retzlaff

Later we will return and provide (where possible) expansions that work correctly for msvc and where not possible provide some alternate macros to achieve the same outcome.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_branch_prediction.h |  8 +++++
 lib/eal/include/rte_common.h            | 54 +++++++++++++++++++++++++++++++++
 lib/eal/include/rte_compat.h            | 20 ++++++++++++
 3 files changed, 82 insertions(+)

lib/eal/include/rte_branch_prediction.h +8 -0

index 414cd92..c0356ca 100644
@@ -24,7 +24,11 @@
  *      do_stuff();
  */
 #ifndef likely
+#ifdef RTE_TOOLCHAIN_MSVC
+#define likely(x)	(!!(x))
+#else
 #define likely(x)	__builtin_expect(!!(x), 1)
+#endif
 #endif /* likely */
 
 /**

@@ -37,7 +41,11 @@
  *      do_stuff();
  */
 #ifndef unlikely
+#ifdef RTE_TOOLCHAIN_MSVC
+#define unlikely(x)	(!!(x))
+#else
 #define unlikely(x)	__builtin_expect(!!(x), 0)
+#endif
 #endif /* unlikely */
 
 #ifdef __cplusplus

lib/eal/include/rte_common.h +54 -0

147 lines not on this page · raw message

lib/eal/include/rte_compat.h +20 -0

index fc9fbaa..716bc03 100644
@@ -12,14 +12,22 @@
 
 #ifndef ALLOW_EXPERIMENTAL_API
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_experimental
+#else
 #define __rte_experimental \
 __attribute__((deprecated("Symbol is not yet part of stable ABI"), \
 section(".text.experimental")))
+#endif
 
 #else
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_experimental
+#else
 #define __rte_experimental \
 __attribute__((section(".text.experimental")))
+#endif
 
 #endif

@@ -30,23 +38,35 @@
 
 #if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 __attribute__((error("Symbol is not public ABI"), \
 section(".text.internal")))
+#endif
 
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 _Pragma("GCC diagnostic push") \
 _Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
 section(".text.internal"))) \
 _Pragma("GCC diagnostic pop")
+#endif
 
 #else
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 __attribute__((section(".text.internal")))
+#endif
 
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #195 lore raw

PATCH v10 11/13 eal: exclude exposure of rte atomic APIs for MSVC builds

In reply to Tyler Retzlaff

It's discouraged to use rte_atomics APIs instead standard APIs should be used from C11. Since MSVC is a new toolchain/platform combination block visibility of the rte_atomic APIs from day 1.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_atomic.h | 7 +++++++
 lib/eal/x86/include/rte_atomic.h     | 4 ++++
 2 files changed, 11 insertions(+)

lib/eal/include/generic/rte_atomic.h +7 -0

index 2d36eb0..b14e9fe 100644
@@ -161,6 +161,8 @@
 
 /*------------------------- 16 bit atomic operations -------------------------*/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Atomic compare and set.
  *

@@ -1068,8 +1070,11 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 }
 #endif
 
+#endif
+
 /*------------------------ 128 bit atomic operations -------------------------*/
 
+
 /**
  * 128-bit integer structure.
  */

@@ -1079,8 +1084,10 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 	union {
 		uint64_t val[2];
 #ifdef RTE_ARCH_64
+#ifndef RTE_TOOLCHAIN_MSVC
 		__extension__ __int128 int128;
 #endif
+#endif
 	};
 } __rte_aligned(16) rte_int128_t;

lib/eal/x86/include/rte_atomic.h +4 -0

index 7aba1c3..9ba61f8 100644
@@ -83,6 +83,8 @@
 
 #define rte_io_rmb() rte_compiler_barrier()
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Synchronization fence between threads based on the specified memory order.
  *

@@ -279,6 +281,8 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 #include "rte_atomic_64.h"
 #endif
 
+#endif
+
 #ifdef __cplusplus
 }
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #196 lore raw

PATCH v10 12/13 eal: always define MSVC as little endian

In reply to Tyler Retzlaff

The MSVC compiler does not target big endian platforms so define little endian always.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_byteorder.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/generic/rte_byteorder.h +2 -0

index 1e32a1b..375f118 100644
@@ -45,6 +45,8 @@
 #define RTE_BYTE_ORDER RTE_BIG_ENDIAN
 #elif defined __LITTLE_ENDIAN__
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#elif defined RTE_TOOLCHAIN_MSVC
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
 #if !defined(RTE_BYTE_ORDER)
 #error Unknown endianness.

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-02 21:36 UTC #197 lore raw

PATCH v10 13/13 eal: do not define typeof macro when building with MSVC

In reply to Tyler Retzlaff

When building with MSVC do not assume typeof is a macro and don't define a typeof macro that conflicts with C23 typeof keyword.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_common.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/rte_common.h +2 -0

index b087532..cf7d8d8 100644
@@ -24,9 +24,11 @@
 /* OS specific include */
 #include <rte_os.h>
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifndef typeof
 #define typeof __typeof__
 #endif
+#endif
 
 #ifndef __cplusplus
 #ifndef asm

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:21 UTC #198 lore raw

v11 cover msvc integration changes

In reply to Tyler Retzlaff

This series covers minimal changes for item #2 in draft plan for EAL dependencies log, kvargs, telemetry and consumed EAL public headers.

Note if any patch in the series requires in-depth discussion I'll detach it from this series for separate submission & more focused discussion so it doesn't block the entire series.

Note other "common" intrinsics were investigated for viability but were not adopted either because they were not available on older versions of GCC or because they generate different code for msvc vs GCC.

v11:
* rebase for log lib split: introduces 2 patches to convert to portable use of ternary operator
* rebase for log lib split: introduce MSVC versions of RTE_INIT_PRIO and RTE_FINI_PRIO since it is needed by log

v10:
* rebase series post 23.07 release. no changes except
telemetry: avoid expanding versioned symbol macros on MSVC has been dropped from the series as 23.11 has removed the versioned symbols

v9:
* invert macro conditional evaluation and use to be #ifdef RTE_TOOLCHAIN_MSVC instead of #ifndef RTE_TOOLCHAIN_MSVC wherever it doesn't make evaluation more complex than it already is

v8:
* use waitpkg intrinsics if __WAITPKG__ is defined by the compiler

v7:
* remove patch typedef of rte_cpuflags_t as an int. for now we will just allow warning to be emitted to avoid adding conditional compilation

v2:
* use _mm_{l,s,m}fence intrinsics for rte_smp_{r,w,}mb macros are intended to be memory barriers not compiler barriers on x86_64

Tyler Retzlaff (16):
eal: use rdtsc intrinsic
eal: use rtm and xtest intrinsics
eal: use barrier intrinsics
eal: use cpuid and cpuidex intrinsics
eal: use umonitor umwait and tpause intrinsics
eal: use prefetch intrinsics
eal: use byte swap intrinsics
eal: hide GCC extension based alignment markers
eal: hide typedefs based on GCC vector extensions
eal: expand most macros to empty when using MSVC
eal: exclude exposure of rte atomic APIs for MSVC builds
eal: always define MSVC as little endian
eal: do not define typeof macro when building with MSVC
log: use standard ternary operator instead of GCC extension
eal: use standard ternary operator instead of GCC extension
eal: define priority based ctor dtor for MSVC

 config/x86/meson.build                  |  6 ++
 lib/eal/common/eal_common_hexdump.c     |  2 +-
 lib/eal/include/generic/rte_atomic.h    | 11 ++++
 lib/eal/include/generic/rte_byteorder.h | 13 +++++
 lib/eal/include/generic/rte_vect.h      |  4 ++
 lib/eal/include/rte_branch_prediction.h |  8 +++
 lib/eal/include/rte_common.h            | 97 +++++++++++++++++++++++++++++++++
 lib/eal/include/rte_compat.h            | 20 +++++++
 lib/eal/x86/include/rte_atomic.h        |  8 +++
 lib/eal/x86/include/rte_byteorder.h     |  4 ++
 lib/eal/x86/include/rte_cycles.h        | 14 +++--
 lib/eal/x86/include/rte_prefetch.h      | 25 +++++++--
 lib/eal/x86/include/rte_rtm.h           | 18 ++----
 lib/eal/x86/rte_cpuflags.c              |  6 +-
 lib/eal/x86/rte_cpuid.h                 |  7 +++
 lib/eal/x86/rte_cycles.c                | 36 ++++++++++++
 lib/eal/x86/rte_hypervisor.c            |  4 ++
 lib/eal/x86/rte_power_intrinsics.c      | 12 ++++
 lib/log/log.c                           |  2 +-
 19 files changed, 271 insertions(+), 26 deletions(-)

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:21 UTC #199 lore raw

PATCH v11 1/16 eal: use rdtsc intrinsic

Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/x86/include/rte_cycles.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

lib/eal/x86/include/rte_cycles.h +8 -6

index a461a4d..ca0fb10 100644
@@ -6,6 +6,12 @@
 #ifndef _RTE_CYCLES_X86_64_H_
 #define _RTE_CYCLES_X86_64_H_
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#include <intrin.h>
+#else
+#include <x86intrin.h>
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif

@@ -23,6 +29,7 @@
 static inline uint64_t
 rte_rdtsc(void)
 {
+#ifdef RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT
 	union {
 		uint64_t tsc_64;
 		RTE_STD_C11

@@ -32,7 +39,6 @@
 		};
 	} tsc;
 
-#ifdef RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT
 	if (unlikely(rte_cycles_vmware_tsc_map)) {
 		/* ecx = 0x10000 corresponds to the physical TSC for VMware */
 		asm volatile("rdpmc" :

@@ -42,11 +48,7 @@
 		return tsc.tsc_64;
 	}
 #endif
-
-	asm volatile("rdtsc" :
-		     "=a" (tsc.lo_32),
-		     "=d" (tsc.hi_32));
-	return tsc.tsc_64;
+	return __rdtsc();
 }
 
 static inline uint64_t

Signature · 1 line

1.8.3.1

Ali Alnubani

2023-08-26 14:38 UTC #200 lore raw

RE: [PATCH v11 01/16] eal: use rdtsc intrinsic

Quoted 11 lines

-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Friday, August 11, 2023 10:21 PM
To: [email protected]
Cc: Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]; [email protected]; Tyler Retzlaff <[email protected]>
Subject: [PATCH v11 01/16] eal: use rdtsc intrinsic

Quoted 7 lines

Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
Acked-by Morten Brørup <[email protected]>
---

Hello,

This patch is causing a build failure in Windows with Clang 11:

"""
[72/803] Compiling C object drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj
FAILED: drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj clang @drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj.rsp In file included from ../drivers/common/idpf/idpf_common_rxtx_avx512.c:6:
In file included from ..\drivers\common\idpf/idpf_common_device.h:9:
In file included from ..\drivers\common\idpf/base/idpf_prototype.h:9:
In file included from ..\drivers\common\idpf/base/idpf_osdep.h:18:
In file included from ..\lib\eal\include\rte_malloc.h:16:
In file included from ..\lib\eal\include\rte_memory.h:25:
In file included from ..\lib\eal\include\rte_fbarray.h:39:
In file included from ..\lib\eal\x86\include\rte_rwlock.h:13:
In file included from ..\lib\eal\x86\include/rte_spinlock.h:18:
In file included from ..\lib\eal\x86\include/rte_cycles.h:12:
In file included from C:\Tools\LLVM\lib\clang\11.0.0\include\x86intrin.h:24:
C:\Tools\LLVM\lib\clang\11.0.0\include\prfchwintrin.h:50:1: error: conflicting types for '__m_prefetchw' _m_prefetchw(void *__P)
^
..\lib\eal\windows\include\rte_windows.h:28:22: note: expanded from macro '_m_prefetchw'

#define _m_prefetchw __m_prefetchw
                     ^

C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\um\winnt.h:3324:1: note: previous declaration is here _m_prefetchw (
^
..\lib\eal\windows\include\rte_windows.h:28:22: note: expanded from macro '_m_prefetchw'

#define _m_prefetchw __m_prefetchw
                     ^

1 error generated.
[73/803] Generating rte_bus_auxiliary_def with a custom command
[74/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_controlq.c.obj [75/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_idpf_common_rxtx.c.obj [76/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_controlq_setup.c.obj [77/803] Generating rte_bus_vdev.pmd.c with a custom command
[78/803] Compiling C object drivers/a715181@@tmp_rte_bus_pci@sta/bus_pci_pci_params.c.obj [79/803] Compiling C object drivers/a715181@@tmp_rte_bus_pci@sta/bus_pci_pci_common.c.obj [80/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_common.c.obj [81/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_idpf_common_virtchnl.c.obj
ninja: build stopped: subcommand failed.
"""

Cross build with x86_64-w64-mingw32-gcc 12.2.1 on Fedora Linux doesn't reproduce.

Regards,
Ali

Tyler Retzlaff

author

2023-08-29 16:16 UTC #201 lore raw

Re: [PATCH v11 01/16] eal: use rdtsc intrinsic

Quoted 14 lines · Ali Alnubani

Quoted 14 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Friday, August 11, 2023 10:21 PM
To: [email protected]
Cc: Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]; [email protected]; Tyler Retzlaff <[email protected]>
Subject: [PATCH v11 01/16] eal: use rdtsc intrinsic
Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.

Quoted 8 lines

Quoted 4 lines
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
Acked-by Morten Brørup <[email protected]>
---
Hello,
This patch is causing a build failure in Windows with Clang 11:

Hi Ali,

while we don't currently document a minimum clang version required to build the windows port i'm starting to consider establishing policy that in effect says we may bump the required compiler version on any dpdk release (not just long term servicing releases). but before doing that it would be nice to understand if that would cause undue pain on the port users.

So is there a reason you can't use LLVM 16?

Not that this information helps you but this is the background to what is breaking here.

The failure here is due to a mistmatch in cv-qualification of the prototype for __m_prefetchw that comes from clang VS the prototype that comes from the Windows SDK.

The Windows SDK will not be changed to remove the volatile qualification and even if it were removed it would be a breaking API change. The LLVM version of the prototype could be volatile qualified without a breaking change causing no harm.

Thanks

Quoted 45 lines

"""
[72/803] Compiling C object drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj
FAILED: drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj clang @drivers/common/idpf/6e54547@@idpf_common_avx512_lib@sta/idpf_common_rxtx_avx512.c.obj.rsp In file included from ../drivers/common/idpf/idpf_common_rxtx_avx512.c:6:
In file included from ..\drivers\common\idpf/idpf_common_device.h:9:
In file included from ..\drivers\common\idpf/base/idpf_prototype.h:9:
In file included from ..\drivers\common\idpf/base/idpf_osdep.h:18:
In file included from ..\lib\eal\include\rte_malloc.h:16:
In file included from ..\lib\eal\include\rte_memory.h:25:
In file included from ..\lib\eal\include\rte_fbarray.h:39:
In file included from ..\lib\eal\x86\include\rte_rwlock.h:13:
In file included from ..\lib\eal\x86\include/rte_spinlock.h:18:
In file included from ..\lib\eal\x86\include/rte_cycles.h:12:
In file included from C:\Tools\LLVM\lib\clang\11.0.0\include\x86intrin.h:24:
C:\Tools\LLVM\lib\clang\11.0.0\include\prfchwintrin.h:50:1: error: conflicting types for '__m_prefetchw' _m_prefetchw(void *__P)
^
..\lib\eal\windows\include\rte_windows.h:28:22: note: expanded from macro '_m_prefetchw'
#define _m_prefetchw __m_prefetchw
                     ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\um\winnt.h:3324:1: note: previous declaration is here _m_prefetchw (
^
..\lib\eal\windows\include\rte_windows.h:28:22: note: expanded from macro '_m_prefetchw'
#define _m_prefetchw __m_prefetchw
                     ^
1 error generated.
[73/803] Generating rte_bus_auxiliary_def with a custom command
[74/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_controlq.c.obj [75/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_idpf_common_rxtx.c.obj [76/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_controlq_setup.c.obj [77/803] Generating rte_bus_vdev.pmd.c with a custom command
[78/803] Compiling C object drivers/a715181@@tmp_rte_bus_pci@sta/bus_pci_pci_params.c.obj [79/803] Compiling C object drivers/a715181@@tmp_rte_bus_pci@sta/bus_pci_pci_common.c.obj [80/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_base_idpf_common.c.obj [81/803] Compiling C object drivers/a715181@@tmp_rte_common_idpf@sta/common_idpf_idpf_common_virtchnl.c.obj
ninja: build stopped: subcommand failed.
"""
Cross build with x86_64-w64-mingw32-gcc 12.2.1 on Fedora Linux doesn't reproduce.
Regards,
Ali

Ali Alnubani

2023-08-30 13:38 UTC #202 lore raw

RE: [PATCH v11 01/16] eal: use rdtsc intrinsic

Quoted 39 lines

-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Tuesday, August 29, 2023 7:16 PM
To: Ali Alnubani <[email protected]>
Cc: David Marchand <[email protected]>; [email protected]; Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]
Subject: Re: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 6 lines · Ali Alnubani
Quoted 6 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Friday, August 11, 2023 10:21 PM
To: [email protected]
Cc: Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power
<[email protected]>;
Quoted 2 lines
Quoted 2 lines
NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]; [email protected]; Tyler
Retzlaff
Quoted 6 lines
Quoted 6 lines
<[email protected]>
Subject: [PATCH v11 01/16] eal: use rdtsc intrinsic
Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
Acked-by Morten Brørup <[email protected]>
---
Hello,
This patch is causing a build failure in Windows with Clang 11:
Hi Ali,

Quoted 8 lines

while we don't currently document a minimum clang version required to build the windows port i'm starting to consider establishing policy that in effect says we may bump the required compiler version on any dpdk release (not just long term servicing releases). but before doing that it would be nice to understand if that would cause undue pain on the port users.
So is there a reason you can't use LLVM 16?

No specific reason. I reported this because compilation was passing with Clang 11 prior to this patch.

Regards,
Ali

Ali Alnubani

2023-08-30 15:49 UTC #203 lore raw

RE: [PATCH v11 01/16] eal: use rdtsc intrinsic

Quoted 58 lines

-----Original Message-----
From: Ali Alnubani <[email protected]>
Sent: Wednesday, August 30, 2023 4:38 PM
To: Tyler Retzlaff <[email protected]>
Cc: David Marchand <[email protected]>; [email protected]; Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]
Subject: RE: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 11 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Tuesday, August 29, 2023 7:16 PM
To: Ali Alnubani <[email protected]>
Cc: David Marchand <[email protected]>; [email protected]; Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]
Subject: Re: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 6 lines
Quoted 5 lines · Ali Alnubani
Quoted 5 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Friday, August 11, 2023 10:21 PM
To: [email protected]
Cc: Bruce Richardson <[email protected]>; Konstantin
Ananyev
Quoted 29 lines
Quoted 1 line
Quoted 1 line
<[email protected]>; Ciara Power
<[email protected]>;
Quoted 2 lines
Quoted 2 lines
NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]; [email protected]; Tyler
Retzlaff
Quoted 6 lines
Quoted 6 lines
<[email protected]>
Subject: [PATCH v11 01/16] eal: use rdtsc intrinsic
Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
Acked-by Morten Brørup <[email protected]>
---
Hello,
This patch is causing a build failure in Windows with Clang 11:
Hi Ali,
while we don't currently document a minimum clang version required to build the windows port i'm starting to consider establishing policy that in effect says we may bump the required compiler version on any dpdk release (not just long term servicing releases). but before doing that it would be nice to understand if that would cause undue pain on the port users.

Quoted 5 lines

Quoted 1 line
So is there a reason you can't use LLVM 16?
No specific reason. I reported this because compilation was passing with Clang 11 prior to this patch.

Building with clang version 16.0.6 fails for me with:

[..]
[405/803] Linking target lib/rte_eal-24.dll
FAILED: lib/rte_eal-24.dll
"clang" @lib/rte_eal-24.dll.rsp
clang: error: no such file or directory: 'librte_log.lib'
clang: error: no such file or directory: 'librte_kvargs.lib'
[406/803] Compiling C object lib/librte_mempool.a.p/mempool_rte_mempool_ops_default.c.obj [..]

Ali Alnubani

2023-08-30 16:29 UTC #204 lore raw

RE: [PATCH v11 01/16] eal: use rdtsc intrinsic

Quoted 82 lines

-----Original Message-----
From: Ali Alnubani
Sent: Wednesday, August 30, 2023 6:49 PM
To: 'Tyler Retzlaff' <[email protected]>
Cc: 'David Marchand' <[email protected]>; '[email protected]' <[email protected]>; 'Bruce Richardson' <[email protected]>; 'Konstantin Ananyev' <[email protected]>; 'Ciara Power' <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; '[email protected]' <[email protected]>
Subject: RE: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 16 lines
-----Original Message-----
From: Ali Alnubani <[email protected]>
Sent: Wednesday, August 30, 2023 4:38 PM
To: Tyler Retzlaff <[email protected]>
Cc: David Marchand <[email protected]>; [email protected]; Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]
Subject: RE: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 5 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Tuesday, August 29, 2023 7:16 PM
To: Ali Alnubani <[email protected]>
Cc: David Marchand <[email protected]>; [email protected];
Bruce
Quoted 2 lines
Quoted 2 lines
Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power
<[email protected]>;
Quoted 4 lines
Quoted 4 lines
NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]
Subject: Re: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 10 lines
Quoted 6 lines · Ali Alnubani
Quoted 5 lines
Quoted 5 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Friday, August 11, 2023 10:21 PM
To: [email protected]
Cc: Bruce Richardson <[email protected]>; Konstantin
Ananyev
Quoted 3 lines
Quoted 1 line
Quoted 1 line
<[email protected]>; Ciara Power
<[email protected]>;
Quoted 1 line
Quoted 1 line
NBU-Contact-Thomas Monjalon (EXTERNAL)
<[email protected]>;
Quoted 26 lines
Quoted 26 lines
Quoted 1 line
Quoted 1 line
[email protected]; [email protected]; Tyler
Retzlaff
Quoted 6 lines
Quoted 6 lines
<[email protected]>
Subject: [PATCH v11 01/16] eal: use rdtsc intrinsic
Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
Acked-by Morten Brørup <[email protected]>
---
Hello,
This patch is causing a build failure in Windows with Clang 11:
Hi Ali,
while we don't currently document a minimum clang version required to build the windows port i'm starting to consider establishing policy that in effect says we may bump the required compiler version on any dpdk release (not just long term servicing releases). but before doing that it would be nice to understand if that would cause undue pain on the port users.
Quoted 3 lines
Quoted 1 line
So is there a reason you can't use LLVM 16?
No specific reason. I reported this because compilation was passing with
Clang
Quoted 2 lines
11 prior to this patch.
Building with clang version 16.0.6 fails for me with:

Quoted 9 lines

[..]
[405/803] Linking target lib/rte_eal-24.dll
FAILED: lib/rte_eal-24.dll
"clang" @lib/rte_eal-24.dll.rsp
clang: error: no such file or directory: 'librte_log.lib'
clang: error: no such file or directory: 'librte_kvargs.lib' [406/803] Compiling C object lib/librte_mempool.a.p/mempool_rte_mempool_ops_default.c.obj [..]

This build failure was actually caused by upgrading Meson from 0.54.3 to 1.2.1 earlier on the environment. The build passes with Meson 0.54.3 and Clang 16.0.6.

Tyler Retzlaff

author

2023-08-31 23:06 UTC #205 lore raw

Re: [PATCH v11 01/16] eal: use rdtsc intrinsic

Quoted 92 lines · Ali Alnubani

Quoted 82 lines
-----Original Message-----
From: Ali Alnubani
Sent: Wednesday, August 30, 2023 6:49 PM
To: 'Tyler Retzlaff' <[email protected]>
Cc: 'David Marchand' <[email protected]>; '[email protected]' <[email protected]>; 'Bruce Richardson' <[email protected]>; 'Konstantin Ananyev' <[email protected]>; 'Ciara Power' <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; '[email protected]' <[email protected]>
Subject: RE: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 16 lines
-----Original Message-----
From: Ali Alnubani <[email protected]>
Sent: Wednesday, August 30, 2023 4:38 PM
To: Tyler Retzlaff <[email protected]>
Cc: David Marchand <[email protected]>; [email protected]; Bruce Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]
Subject: RE: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 5 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Tuesday, August 29, 2023 7:16 PM
To: Ali Alnubani <[email protected]>
Cc: David Marchand <[email protected]>; [email protected];
Bruce
Quoted 2 lines
Quoted 2 lines
Richardson <[email protected]>; Konstantin Ananyev <[email protected]>; Ciara Power
<[email protected]>;
Quoted 4 lines
Quoted 4 lines
NBU-Contact-Thomas Monjalon (EXTERNAL) <[email protected]>; [email protected]
Subject: Re: [PATCH v11 01/16] eal: use rdtsc intrinsic
Quoted 10 lines
Quoted 6 lines · Ali Alnubani
Quoted 5 lines
Quoted 5 lines
-----Original Message-----
From: Tyler Retzlaff <[email protected]>
Sent: Friday, August 11, 2023 10:21 PM
To: [email protected]
Cc: Bruce Richardson <[email protected]>; Konstantin
Ananyev
Quoted 3 lines
Quoted 1 line
Quoted 1 line
<[email protected]>; Ciara Power
<[email protected]>;
Quoted 1 line
Quoted 1 line
NBU-Contact-Thomas Monjalon (EXTERNAL)
<[email protected]>;
Quoted 26 lines
Quoted 26 lines
Quoted 1 line
Quoted 1 line
[email protected]; [email protected]; Tyler
Retzlaff
Quoted 6 lines
Quoted 6 lines
<[email protected]>
Subject: [PATCH v11 01/16] eal: use rdtsc intrinsic
Inline assembly is not supported for MSVC x64. Convert code to use __rdtsc intrinsic.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
Acked-by Morten Brørup <[email protected]>
---
Hello,
This patch is causing a build failure in Windows with Clang 11:
Hi Ali,
while we don't currently document a minimum clang version required to build the windows port i'm starting to consider establishing policy that in effect says we may bump the required compiler version on any dpdk release (not just long term servicing releases). but before doing that it would be nice to understand if that would cause undue pain on the port users.
Quoted 3 lines
Quoted 1 line
So is there a reason you can't use LLVM 16?
No specific reason. I reported this because compilation was passing with
Clang
Quoted 2 lines
11 prior to this patch.
Building with clang version 16.0.6 fails for me with:
Quoted 9 lines
[..]
[405/803] Linking target lib/rte_eal-24.dll
FAILED: lib/rte_eal-24.dll
"clang" @lib/rte_eal-24.dll.rsp
clang: error: no such file or directory: 'librte_log.lib'
clang: error: no such file or directory: 'librte_kvargs.lib' [406/803] Compiling C object lib/librte_mempool.a.p/mempool_rte_mempool_ops_default.c.obj [..]

Quoted 2 lines

This build failure was actually caused by upgrading Meson from 0.54.3 to 1.2.1 earlier on the environment. The build passes with Meson 0.54.3 and Clang 16.0.6.

yes, the windows build is very sensitive to meson version. i only test with 0.57.0 i have not had time to investigate why newer versions do not work.

Tyler Retzlaff

author

2023-08-11 19:21 UTC #206 lore raw

PATCH v11 2/16 eal: use rtm and xtest intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64. Convert code to use _xend, _xabort and _xtest intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 config/x86/meson.build        |  6 ++++++
 lib/eal/x86/include/rte_rtm.h | 18 +++++-------------
 2 files changed, 11 insertions(+), 13 deletions(-)

config/x86/meson.build +6 -0

index 54345c4..4c0b06c 100644
@@ -30,6 +30,12 @@ if cc.get_define('__SSE4_2__', args: machine_args) == ''
     machine_args += '-msse4'
 endif
 
+# enable restricted transactional memory intrinsics
+# https://gcc.gnu.org/onlinedocs/gcc/x86-transactional-memory-intrinsics.html
+if cc.get_id() != 'msvc'
+    machine_args += '-mrtm'
+endif
+
 base_flags = ['SSE', 'SSE2', 'SSE3','SSSE3', 'SSE4_1', 'SSE4_2']
 foreach f:base_flags
     compile_time_cpuflags += ['RTE_CPUFLAG_' + f]

lib/eal/x86/include/rte_rtm.h +5 ~~-13~~

index 36bf498..b84e58e 100644
@@ -5,6 +5,7 @@
 #ifndef _RTE_RTM_H_
 #define _RTE_RTM_H_ 1
 
+#include <immintrin.h>
 
 /* Official RTM intrinsics interface matching gcc/icc, but works
    on older gcc compatible compilers and binutils. */

@@ -28,31 +29,22 @@
 static __rte_always_inline
 unsigned int rte_xbegin(void)
 {
-	unsigned int ret = RTE_XBEGIN_STARTED;
-
-	asm volatile(".byte 0xc7,0xf8 ; .long 0" : "+a" (ret) :: "memory");
-	return ret;
+	return _xbegin();
 }
 
 static __rte_always_inline
 void rte_xend(void)
 {
-	 asm volatile(".byte 0x0f,0x01,0xd5" ::: "memory");
+	_xend();
 }
 
 /* not an inline function to workaround a clang bug with -O0 */
-#define rte_xabort(status) do { \
-	asm volatile(".byte 0xc6,0xf8,%P0" :: "i" (status) : "memory"); \
-} while (0)
+#define rte_xabort(status) _xabort(status)
 
 static __rte_always_inline
 int rte_xtest(void)
 {
-	unsigned char out;
-
-	asm volatile(".byte 0x0f,0x01,0xd6 ; setnz %0" :
-		"=r" (out) :: "memory");
-	return out;
+	return _xtest();
 }
 
 #ifdef __cplusplus

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:21 UTC #207 lore raw

PATCH v11 3/16 eal: use barrier intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead expand rte_compiler_barrier as _ReadWriteBarrier and for rte_smp_mb _m_mfence intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_atomic.h | 4 ++++
 lib/eal/x86/include/rte_atomic.h     | 4 ++++
 2 files changed, 8 insertions(+)

lib/eal/include/generic/rte_atomic.h +4 -0

index aef44e2..2d36eb0 100644
@@ -146,9 +146,13 @@
  * Guarantees that operation reordering does not occur at compile time
  * for operations directly before and after the barrier.
  */
+#ifdef RTE_TOOLCHAIN_MSVC
+#define rte_compiler_barrier() _ReadWriteBarrier()
+#else
 #define	rte_compiler_barrier() do {		\
 	asm volatile ("" : : : "memory");	\
 } while(0)
+#endif
 
 /**
  * Synchronization fence between threads based on the specified memory order.

lib/eal/x86/include/rte_atomic.h +4 -0

index f2ee1a9..7aba1c3 100644
@@ -66,11 +66,15 @@
 static __rte_always_inline void
 rte_smp_mb(void)
 {
+#ifdef RTE_TOOLCHAIN_MSVC
+	_mm_mfence();
+#else
 #ifdef RTE_ARCH_I686
 	asm volatile("lock addl $0, -128(%%esp); " ::: "memory");
 #else
 	asm volatile("lock addl $0, -128(%%rsp); " ::: "memory");
 #endif
+#endif
 }
 
 #define rte_io_mb() rte_mb()

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:21 UTC #208 lore raw

PATCH v11 4/16 eal: use cpuid and cpuidex intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use __cpuid and __cpuidex intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/x86/rte_cpuflags.c   |  6 +++++-
 lib/eal/x86/rte_cpuid.h      |  7 +++++++
 lib/eal/x86/rte_cycles.c     | 36 ++++++++++++++++++++++++++++++++++++
 lib/eal/x86/rte_hypervisor.c |  4 ++++
 4 files changed, 52 insertions(+), 1 deletion(-)

lib/eal/x86/rte_cpuflags.c +5 -1

index d6b5182..8a30f79 100644
@@ -165,9 +165,13 @@ struct feature_entry {
 	if (maxleaf < feat->leaf)
 		return 0;
 
-	 __cpuid_count(feat->leaf, feat->subleaf,
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuidex(regs, feat->leaf, feat->subleaf);
+#else
+	__cpuid_count(feat->leaf, feat->subleaf,
 			 regs[RTE_REG_EAX], regs[RTE_REG_EBX],
 			 regs[RTE_REG_ECX], regs[RTE_REG_EDX]);
+#endif
 
 	/* check if the feature is enabled */
 	return (regs[feat->reg] >> feat->bit) & 1;

lib/eal/x86/rte_cpuid.h +7 -0

index b773ad9..c6abaad 100644
@@ -5,7 +5,9 @@
 #ifndef RTE_CPUID_H
 #define RTE_CPUID_H
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #include <cpuid.h>
+#endif
 
 enum cpu_register_t {
 	RTE_REG_EAX = 0,

@@ -16,4 +18,9 @@ enum cpu_register_t {
 
 typedef uint32_t cpuid_registers_t[4];
 
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+__get_cpuid_max(unsigned int e, unsigned int *s);
+#endif
+
 #endif /* RTE_CPUID_H */

lib/eal/x86/rte_cycles.c +36 -0

index 0e695ca..69ed59b 100644
@@ -4,7 +4,11 @@
 
 #include <fcntl.h>
 #include <unistd.h>
+#ifdef RTE_TOOLCHAIN_MSVC
+#define bit_AVX (1 << 28)
+#else
 #include <cpuid.h>
+#endif
 
 
 #include "eal_private.h"

@@ -82,9 +86,25 @@
 	return 0;
 }
 
+#ifdef RTE_TOOLCHAIN_MSVC
+int
+__get_cpuid_max(unsigned int e, unsigned int *s)
+{
+	uint32_t cpuinfo[4];
+
+	__cpuid(cpuinfo, e);
+	if (s)
+		*s = cpuinfo[1];
+	return cpuinfo[0];
+}
+#endif
+
 uint64_t
 get_tsc_freq_arch(void)
 {
+#ifdef RTE_TOOLCHAIN_MSVC
+	int cpuinfo[4];
+#endif
 	uint64_t tsc_hz = 0;
 	uint32_t a, b, c, d, maxleaf;
 	uint8_t mult, model;

@@ -97,14 +117,30 @@
 	maxleaf = __get_cpuid_max(0, NULL);
 
 	if (maxleaf >= 0x15) {
+#ifdef RTE_TOOLCHAIN_MSVC
+		__cpuid(cpuinfo, 0x15);
+		a = cpuinfo[0];
+		b = cpuinfo[1];
+		c = cpuinfo[2];
+		d = cpuinfo[3];
+#else
 		__cpuid(0x15, a, b, c, d);
+#endif
 
 		/* EBX : TSC/Crystal ratio, ECX : Crystal Hz */
 		if (b && c)
 			return c * (b / a);
 	}
 
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuid(cpuinfo, 0x1);
+	a = cpuinfo[0];
+	b = cpuinfo[1];
+	c = cpuinfo[2];
+	d = cpuinfo[3];
+#else
 	__cpuid(0x1, a, b, c, d);
+#endif
 	model = rte_cpu_get_model(a);
 
 	if (check_model_wsm_nhm(model))

lib/eal/x86/rte_hypervisor.c +4 -0

index c38cfc0..04fe767 100644
@@ -23,9 +23,13 @@ enum rte_hypervisor
 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_HYPERVISOR))
 		return RTE_HYPERVISOR_NONE;
 
+#ifdef RTE_TOOLCHAIN_MSVC
+	__cpuid(regs, HYPERVISOR_INFO_LEAF);
+#else
 	__cpuid(HYPERVISOR_INFO_LEAF,
 			regs[RTE_REG_EAX], regs[RTE_REG_EBX],
 			regs[RTE_REG_ECX], regs[RTE_REG_EDX]);
+#endif
 	for (reg = 1; reg < 4; reg++)
 		memcpy(name + (reg - 1) * 4, &regs[reg], 4);
 	name[12] = '\0';

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:21 UTC #209 lore raw

PATCH v11 5/16 eal: use umonitor umwait and tpause intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _umonitor, _umwait and _tpause intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

lib/eal/x86/rte_power_intrinsics.c +12 -0

index f749da9..4066d13 100644
@@ -109,9 +109,13 @@
 	 */
 
 	/* set address for UMONITOR */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umonitor(pmc->addr);
+#else
 	asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
 			:
 			: "D"(pmc->addr));
+#endif
 
 	/* now that we've put this address into monitor, we can unlock */
 	rte_spinlock_unlock(&s->lock);

@@ -123,10 +127,14 @@
 		goto end;
 
 	/* execute UMWAIT */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umwait(tsc_l, tsc_h);
+#else
 	asm volatile(".byte 0xf2, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */
 			  "a"(tsc_l), "d"(tsc_h));
+#endif
 
 end:
 	/* erase sleep address */

@@ -153,10 +161,14 @@
 		return -ENOTSUP;
 
 	/* execute TPAUSE */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_tpause(tsc_l, tsc_h);
+#else
 	asm volatile(".byte 0x66, 0x0f, 0xae, 0xf7;"
 			: /* ignore rflags */
 			: "D"(0), /* enter C0.2 */
 			"a"(tsc_l), "d"(tsc_h));
+#endif
 
 	return 0;
 }

Signature · 1 line

1.8.3.1

Bruce Richardson

2023-08-25 14:13 UTC #210 lore raw

Re: [PATCH v11 05/16] eal: use umonitor umwait and tpause intrinsics

Quoted 15 lines · Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _umonitor, _umwait and _tpause intrinsics.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
lib/eal/x86/rte_power_intrinsics.c +0 -0
index f749da9..4066d13 100644

lib/eal/x86/rte_power_intrinsics.c line 114

Quoted 6 lines

lib/eal/x86/rte_power_intrinsics.c +3 -0

@@ -109,9 +109,13 @@
 	 */
 
 	/* set address for UMONITOR */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umonitor(pmc->addr);
+#else

This change is unfortunately giving build errors on system with WAITPKG, since the intrinsics do not take volatile parameters, unlike the inline ASM which works fine with both volatile and non-volatile variables. This is the error I see:

../lib/eal/x86/rte_power_intrinsics.c: In function 'rte_power_monitor':
../lib/eal/x86/rte_power_intrinsics.c:113:22: error: passing argument 1 of '_umonitor' discards 'volatile' qualifier from pointer target type [-Werror=discarded-qualifiers]

  113 |         _umonitor(pmc->addr);
      |                   ~~~^~~~~~

The easy fix for now seems to be just dropping the "|| defined(__WAITPKG__)" part of the #ifdef, and leave the intrinsic for MSVC only.
Any objections?

/Bruce

Morten Brørup

2023-08-25 14:56 UTC #211 lore raw

RE: [PATCH v11 05/16] eal: use umonitor umwait and tpause intrinsics

Quoted 38 lines

From: Bruce Richardson [mailto:[email protected]]
Sent: Friday, 25 August 2023 16.13
Review
Quoted 11 lines · Tyler Retzlaff
Inline assembly is not supported for MSVC x64 instead use _umonitor, _umwait and _tpause intrinsics.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
(quoted diff) +0 -0
b/lib/eal/x86/rte_power_intrinsics.c
lib/eal/x86/rte_power_intrinsics.c line 114
Quoted 10 lines
index f749da9..4066d13 100644
lib/eal/x86/rte_power_intrinsics.c +3 -0
@@ -109,9 +109,13 @@
 	 */
 
 	/* set address for UMONITOR */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umonitor(pmc->addr);
+#else
This change is unfortunately giving build errors on system with WAITPKG, since the intrinsics do not take volatile parameters, unlike the inline ASM
which works fine with both volatile and non-volatile variables. This is the
error I see:
../lib/eal/x86/rte_power_intrinsics.c: In function 'rte_power_monitor':
../lib/eal/x86/rte_power_intrinsics.c:113:22: error: passing argument 1 of '_umonitor' discards 'volatile' qualifier from pointer target type [- Werror=discarded-qualifiers]

Quoted 8 lines

  113 |         _umonitor(pmc->addr);
      |                   ~~~^~~~~~
The easy fix for now seems to be just dropping the "|| defined(__WAITPKG__)" part of the #ifdef, and leave the intrinsic for MSVC
only.
Any objections?

I wonder if omitting the "volatile" qualifier is correct for this parameter. Although the authors of _umonitor() apparently think so, I have seen built-ins with questionable qualifiers before, so I wouldn't trust it to be correct.

Replacing inline assembly with built-ins is generally preferable. So I would prefer if you typecast it away, or temporarily disable that warning.

But I don't object to the quick fix. :-)

-Morten

Bruce Richardson

2023-08-25 15:13 UTC #212 lore raw

Re: [PATCH v11 05/16] eal: use umonitor umwait and tpause intrinsics

Quoted 47 lines · Morten Brørup

Quoted 38 lines
From: Bruce Richardson [mailto:[email protected]]
Sent: Friday, 25 August 2023 16.13
Review
Quoted 11 lines · Tyler Retzlaff
Inline assembly is not supported for MSVC x64 instead use _umonitor, _umwait and _tpause intrinsics.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
(quoted diff) +0 -0
b/lib/eal/x86/rte_power_intrinsics.c
lib/eal/x86/rte_power_intrinsics.c line 114
Quoted 10 lines
index f749da9..4066d13 100644
lib/eal/x86/rte_power_intrinsics.c +3 -0
@@ -109,9 +109,13 @@
 	 */
 
 	/* set address for UMONITOR */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umonitor(pmc->addr);
+#else
This change is unfortunately giving build errors on system with WAITPKG, since the intrinsics do not take volatile parameters, unlike the inline ASM
which works fine with both volatile and non-volatile variables. This is the
error I see:
../lib/eal/x86/rte_power_intrinsics.c: In function 'rte_power_monitor':
../lib/eal/x86/rte_power_intrinsics.c:113:22: error: passing argument 1 of '_umonitor' discards 'volatile' qualifier from pointer target type [- Werror=discarded-qualifiers]
Quoted 8 lines
  113 |         _umonitor(pmc->addr);
      |                   ~~~^~~~~~
The easy fix for now seems to be just dropping the "|| defined(__WAITPKG__)" part of the #ifdef, and leave the intrinsic for MSVC
only.
Any objections?

Quoted 4 lines

I wonder if omitting the "volatile" qualifier is correct for this parameter. Although the authors of _umonitor() apparently think so, I have seen built-ins with questionable qualifiers before, so I wouldn't trust it to be correct.
Replacing inline assembly with built-ins is generally preferable. So I would prefer if you typecast it away, or temporarily disable that warning.

Simple typecasting doesn't work to remove the warning. However, if we typecast via "uintptr_t" it does seem to work.

#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)

-       _umonitor(pmc->addr);
+       uintptr_t addr_val = (uintptr_t)pmc->addr;
+       _umonitor((void *)addr_val);

#else

Seems bit clunky to me. If we want a one-line workaround, we can probably use RTE_PTR_ADD to do the typecasting via uintptr_t for us.

/Bruce

Tyler Retzlaff

author

2023-08-25 15:44 UTC #213 lore raw

Re: [PATCH v11 05/16] eal: use umonitor umwait and tpause intrinsics

Quoted 42 lines · Bruce Richardson

Quoted 41 lines · Morten Brørup
Quoted 41 lines
From: Bruce Richardson [mailto:[email protected]]
Sent: Friday, 25 August 2023 16.13
Review
Quoted 11 lines · Tyler Retzlaff
Inline assembly is not supported for MSVC x64 instead use _umonitor, _umwait and _tpause intrinsics.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
---
 lib/eal/x86/rte_power_intrinsics.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
(quoted diff) +0 -0
b/lib/eal/x86/rte_power_intrinsics.c
lib/eal/x86/rte_power_intrinsics.c line 114
Quoted 10 lines
index f749da9..4066d13 100644
lib/eal/x86/rte_power_intrinsics.c +3 -0
@@ -109,9 +109,13 @@
 	 */
 
 	/* set address for UMONITOR */
+#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
+	_umonitor(pmc->addr);
+#else
This change is unfortunately giving build errors on system with WAITPKG, since the intrinsics do not take volatile parameters, unlike the inline ASM
which works fine with both volatile and non-volatile variables. This is the
error I see:
../lib/eal/x86/rte_power_intrinsics.c: In function 'rte_power_monitor':
../lib/eal/x86/rte_power_intrinsics.c:113:22: error: passing argument 1 of '_umonitor' discards 'volatile' qualifier from pointer target type [- Werror=discarded-qualifiers]
  113 |         _umonitor(pmc->addr);
      |                   ~~~^~~~~~

Quoted 7 lines

Quoted 7 lines
Quoted 5 lines
The easy fix for now seems to be just dropping the "|| defined(__WAITPKG__)" part of the #ifdef, and leave the intrinsic for MSVC
only.
Any objections?
I wonder if omitting the "volatile" qualifier is correct for this parameter. Although the authors of _umonitor() apparently think so, I have seen built-ins with questionable qualifiers before, so I wouldn't trust it to be correct.

omitting the volatile qualifier is okay. often the reason why parameters are qualified is as a convenience so that you may pass a volatile qualified argument (and not force removal of the qualification).

Quoted 5 lines

Quoted 3 lines
Replacing inline assembly with built-ins is generally preferable. So I would prefer if you typecast it away, or temporarily disable that warning.
Simple typecasting doesn't work to remove the warning. However, if we typecast via "uintptr_t" it does seem to work.

yes, this is canonical way to remove the qualification.

Quoted 11 lines

#if defined(RTE_TOOLCHAIN_MSVC) || defined(__WAITPKG__)
(quoted diff) +2 -1
-       _umonitor(pmc->addr);
+       uintptr_t addr_val = (uintptr_t)pmc->addr;
+       _umonitor((void *)addr_val);
 #else
Seems bit clunky to me. If we want a one-line workaround, we can probably use RTE_PTR_ADD to do the typecasting via uintptr_t for us.
/Bruce

Tyler Retzlaff

author

2023-08-11 19:21 UTC #214 lore raw

PATCH v11 6/16 eal: use prefetch intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _mm_prefetch and _mm_cldemote intrinsics.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Bruce Richardson <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Konstantin Ananyev <[email protected]>

---
 lib/eal/x86/include/rte_prefetch.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

lib/eal/x86/include/rte_prefetch.h +21 -4

index 7fd01c4..7a6988e 100644
@@ -9,30 +9,38 @@
 extern "C" {
 #endif
 
+#include <emmintrin.h>
+
 #include <rte_compat.h>
 #include <rte_common.h>
 #include "generic/rte_prefetch.h"
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-qual"
+
 static inline void rte_prefetch0(const volatile void *p)
 {
-	asm volatile ("prefetcht0 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T0);
 }
 
 static inline void rte_prefetch1(const volatile void *p)
 {
-	asm volatile ("prefetcht1 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T1);
 }
 
 static inline void rte_prefetch2(const volatile void *p)
 {
-	asm volatile ("prefetcht2 %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_T2);
 }
 
 static inline void rte_prefetch_non_temporal(const volatile void *p)
 {
-	asm volatile ("prefetchnta %[p]" : : [p] "m" (*(const volatile char *)p));
+	_mm_prefetch((const void *)p, _MM_HINT_NTA);
 }
 
+#pragma GCC diagnostic pop
+
+#ifdef RTE_TOOLCHAIN_MSVC
 /*
  * We use raw byte codes for now as only the newest compiler
  * versions support this instruction natively.

@@ -41,8 +49,17 @@ static inline void rte_prefetch_non_temporal(const volatile void *p)
 static inline void
 rte_cldemote(const volatile void *p)
 {
+	_mm_cldemote(p);
+}
+#else
+__rte_experimental
+static inline void
+rte_cldemote(const volatile void *p)
+{
 	asm volatile(".byte 0x0f, 0x1c, 0x06" :: "S" (p));
 }
+#endif
+
 
 #ifdef __cplusplus
 }

Signature · 1 line

1.8.3.1

David Marchand

2023-08-24 12:06 UTC #215 lore raw

Re: [PATCH v11 06/16] eal: use prefetch intrinsics

Quoted 25 lines · Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead use _mm_prefetch and _mm_cldemote intrinsics.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Bruce Richardson <[email protected]>
Acked-by Morten Brørup <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
---
 lib/eal/x86/include/rte_prefetch.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)
lib/eal/x86/include/rte_prefetch.h +2 -0
index 7fd01c4..7a6988e 100644
@@ -9,30 +9,38 @@
 extern "C" {
 #endif
 
+#include <emmintrin.h>
+
 #include <rte_compat.h>
 #include <rte_common.h>
 #include "generic/rte_prefetch.h"

lib/eal/x86/include/rte_prefetch.h line 25

Quoted 10 lines

lib/eal/x86/include/rte_prefetch.h +4 -1

 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-qual"
+
 static inline void rte_prefetch0(const volatile void *p)
 {
-       asm volatile ("prefetcht0 %[p]" : : [p] "m" (*(const volatile char *)p));
+       _mm_prefetch((const void *)p, _MM_HINT_T0);
 }

Quite surprisingly, this part (exactly) breaks 32bits build in the vhost library:

ccache cc -Ilib/librte_vhost.a.p -Ilib -I../../../git/pub/dpdk.org/main/lib -Ilib/vhost -I../../../git/pub/dpdk.org/main/lib/vhost -I. -I../../../git/pub/dpdk.org/main -Iconfig -I../../../git/pub/dpdk.org/main/config -Ilib/eal/include -I../../../git/pub/dpdk.org/main/lib/eal/include -Ilib/eal/linux/include -I../../../git/pub/dpdk.org/main/lib/eal/linux/include -Ilib/eal/x86/include -I../../../git/pub/dpdk.org/main/lib/eal/x86/include -Ilib/eal/common -I../../../git/pub/dpdk.org/main/lib/eal/common -Ilib/eal -I../../../git/pub/dpdk.org/main/lib/eal -Ilib/kvargs -I../../../git/pub/dpdk.org/main/lib/kvargs -Ilib/log -I../../../git/pub/dpdk.org/main/lib/log -Ilib/metrics -I../../../git/pub/dpdk.org/main/lib/metrics -Ilib/telemetry -I../../../git/pub/dpdk.org/main/lib/telemetry -Ilib/ethdev -I../../../git/pub/dpdk.org/main/lib/ethdev -Ilib/net -I../../../git/pub/dpdk.org/main/lib/net -Ilib/mbuf -I../../../git/pub/dpdk.org/main/lib/mbuf -Ilib/mempool -I../../../git/pub/dpdk.org/main/lib/mempool -Ilib/ring -I../../../git/pub/dpdk.org/main/lib/ring -Ilib/meter -I../../../git/pub/dpdk.org/main/lib/meter -Ilib/cryptodev -I../../../git/pub/dpdk.org/main/lib/cryptodev -Ilib/rcu -I../../../git/pub/dpdk.org/main/lib/rcu -Ilib/hash -I../../../git/pub/dpdk.org/main/lib/hash -Ilib/pci -I../../../git/pub/dpdk.org/main/lib/pci -Ilib/dmadev -I../../../git/pub/dpdk.org/main/lib/dmadev -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c11 -O2 -g -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-zero-length-bounds -Wno-pointer-to-int-cast -D_GNU_SOURCE -m32 -fPIC -march=native -mrtm -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation -DVHOST_GCC_UNROLL_PRAGMA -fno-strict-aliasing -DVHOST_HAS_VDUSE -DRTE_LOG_DEFAULT_LOGTYPE=lib.vhost -MD -MQ lib/librte_vhost.a.p/vhost_virtio_net.c.o -MF lib/librte_vhost.a.p/vhost_virtio_net.c.o.d -o lib/librte_vhost.a.p/vhost_virtio_net.c.o -c ../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c
In function ‘mbuf_to_desc’,
inlined from ‘vhost_enqueue_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1948:6,
inlined from ‘virtio_dev_rx_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1965:6,
inlined from ‘virtio_dev_rx_async_submit_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:2127:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1230:18: error:
‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized]

 1230 |         buf_addr = buf_vec[vec_idx].buf_addr;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~

../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1963:27: note:
‘buf_vec’ declared here
1963 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
In function ‘mbuf_to_desc’,
inlined from ‘vhost_enqueue_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1948:6,
inlined from ‘virtio_dev_rx_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1965:6,
inlined from ‘virtio_dev_rx_async_submit_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:2127:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1231:18: error:
‘buf_vec[0].buf_iova’ may be used uninitialized [-Werror=maybe-uninitialized]

 1231 |         buf_iova = buf_vec[vec_idx].buf_iova;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~

../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1963:27: note:
‘buf_vec’ declared here
1963 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
In function ‘mbuf_to_desc’,
inlined from ‘vhost_enqueue_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1948:6,
inlined from ‘virtio_dev_rx_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1965:6,
inlined from ‘virtio_dev_rx_async_submit_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:2127:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1232:35: error:
‘buf_vec[0].buf_len’ may be used uninitialized [-Werror=maybe-uninitialized]

 1232 |         buf_len = buf_vec[vec_idx].buf_len;
      |                   ~~~~~~~~~~~~~~~~^~~~~~~~

I had a look at the vhost library and I think the compiler thinks size may be 0. Changing the loop on size with a do { } while (size > 0); resolves the warning. I can post a change for this, as we hit a similar issue in the past and the code does not make sense comparing size on the first iteration of this loop.

However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...

Signature · 1 line

David Marchand

David Marchand

2023-08-24 12:25 UTC #216 lore raw

Re: [PATCH v11 06/16] eal: use prefetch intrinsics

Quoted 5 lines · David Marchand

I had a look at the vhost library and I think the compiler thinks size may be 0. Changing the loop on size with a do { } while (size > 0); resolves the warning. I can post a change for this, as we hit a similar issue in the past and the code does not make sense comparing size on the first iteration of this loop.

This sounds like a commit 4226aa9caca9 ("vhost: fix build with GCC 12") we had on LoongArch.

And the 32bits build warning goes away with this:

lib/vhost/virtio_net.c +1 -1

index d7624d18c8..41328b7530 100644
@@ -1906,7 +1906,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
        uint16_t buf_id = 0;
        uint32_t len = 0;
        uint16_t desc_count = 0;
-       uint64_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);
+       uint64_t size = (uint64_t)pkt->pkt_len + sizeof(struct

virtio_net_hdr_mrg_rxbuf);
        uint32_t buffer_len[vq->size];
        uint16_t buffer_buf_id[vq->size];
        uint16_t buffer_desc_count[vq->size];

Quoted 5 lines

However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...

Signature · 1 line

David Marchand

Morten Brørup

2023-08-25 09:23 UTC #217 lore raw

RE: [PATCH v11 06/16] eal: use prefetch intrinsics

Regarding the vhost lib...

Quoted 7 lines

From: David Marchand [mailto:[email protected]]
Sent: Thursday, 24 August 2023 14.26
Quoted 1 line · David Marchand
I had a look at the vhost library and I think the compiler thinks size may
be 0.

Quoted 8 lines

Quoted 1 line
Changing the loop on size with a do { } while (size > 0); resolves the
warning.
Quoted 3 lines
I can post a change for this, as we hit a similar issue in the past and the code does not make sense comparing size on the first iteration of this loop.
This sounds like a commit 4226aa9caca9 ("vhost: fix build with GCC 12") we had on LoongArch.

If we know that size > 0 initially, then "do { } while (size > 0);" is more appropriate than "while (size > 0) { }", not only for the benefit of the compiler/optimizer, but also for the benefit of human reviewers.

Quoted 8 lines

And the 32bits build warning goes away with this:
lib/vhost/virtio_net.c +0 -0
index d7624d18c8..41328b7530 100644

lib/vhost/virtio_net.c vhost_enqueue_async_packed(struct virtio_net *dev, · line 1911

Quoted 7 lines

lib/vhost/virtio_net.c +1 -1

@@ -1906,7 +1906,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
        uint16_t buf_id = 0;
        uint32_t len = 0;
        uint16_t desc_count = 0;
-       uint64_t size = pkt->pkt_len + sizeof(struct
 virtio_net_hdr_mrg_rxbuf);
+       uint64_t size = (uint64_t)pkt->pkt_len + sizeof(struct
 virtio_net_hdr_mrg_rxbuf);

Yes, but performing a 64 bit addition instead of a 32 bit addition has a (very small) cost for 32 bit arch.

And regardless of this cost for 32 bit arch; if we know that size cannot exceed UINT32_MAX, it should be changed to uint32_t.

Quoted 3 lines

lib/vhost/virtio_net.c +0 -0

        uint32_t buffer_len[vq->size];

        uint16_t buffer_buf_id[vq->size];
        uint16_t buffer_desc_count[vq->size];

Morten Brørup

2023-08-24 12:47 UTC #218 lore raw

RE: [PATCH v11 06/16] eal: use prefetch intrinsics

In reply to David Marchand

Quoted 145 lines

From: David Marchand [mailto:[email protected]]
Sent: Thursday, 24 August 2023 14.06
Quoted 8 lines · Tyler Retzlaff
Inline assembly is not supported for MSVC x64 instead use _mm_prefetch and _mm_cldemote intrinsics.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Bruce Richardson <[email protected]>
Acked-by Morten Brørup <[email protected]>
Acked-by Konstantin Ananyev <[email protected]>
Review
Quoted 5 lines
---
 lib/eal/x86/include/rte_prefetch.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)
(quoted diff) +0 -0
b/lib/eal/x86/include/rte_prefetch.h
Quoted 9 lines
index 7fd01c4..7a6988e 100644
lib/eal/x86/include/rte_prefetch.h +2 -0
@@ -9,30 +9,38 @@
 extern "C" {
 #endif
 
+#include <emmintrin.h>
+
lib/eal/x86/include/rte_prefetch.h line 18
Quoted 10 lines
lib/eal/x86/include/rte_prefetch.h +3 -1
 #include <rte_compat.h>
 #include <rte_common.h>
 #include "generic/rte_prefetch.h"
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-qual"
+
 static inline void rte_prefetch0(const volatile void *p)
 {
-       asm volatile ("prefetcht0 %[p]" : : [p] "m" (*(const volatile char
*)p));
lib/eal/x86/include/rte_prefetch.h line 25
Quoted 3 lines
lib/eal/x86/include/rte_prefetch.h +1 -0
+       _mm_prefetch((const void *)p, _MM_HINT_T0);
 }
 
Quite surprisingly, this part (exactly) breaks 32bits build in the vhost library:
ccache cc -Ilib/librte_vhost.a.p -Ilib
(quoted diff) +0 ~~-37~~
-I../../../git/pub/dpdk.org/main/lib -Ilib/vhost
-I../../../git/pub/dpdk.org/main/lib/vhost -I.
-I../../../git/pub/dpdk.org/main -Iconfig
-I../../../git/pub/dpdk.org/main/config -Ilib/eal/include
-I../../../git/pub/dpdk.org/main/lib/eal/include
-Ilib/eal/linux/include
-I../../../git/pub/dpdk.org/main/lib/eal/linux/include
-Ilib/eal/x86/include
-I../../../git/pub/dpdk.org/main/lib/eal/x86/include -Ilib/eal/common
-I../../../git/pub/dpdk.org/main/lib/eal/common -Ilib/eal
-I../../../git/pub/dpdk.org/main/lib/eal -Ilib/kvargs
-I../../../git/pub/dpdk.org/main/lib/kvargs -Ilib/log
-I../../../git/pub/dpdk.org/main/lib/log -Ilib/metrics
-I../../../git/pub/dpdk.org/main/lib/metrics -Ilib/telemetry
-I../../../git/pub/dpdk.org/main/lib/telemetry -Ilib/ethdev
-I../../../git/pub/dpdk.org/main/lib/ethdev -Ilib/net
-I../../../git/pub/dpdk.org/main/lib/net -Ilib/mbuf
-I../../../git/pub/dpdk.org/main/lib/mbuf -Ilib/mempool
-I../../../git/pub/dpdk.org/main/lib/mempool -Ilib/ring
-I../../../git/pub/dpdk.org/main/lib/ring -Ilib/meter
-I../../../git/pub/dpdk.org/main/lib/meter -Ilib/cryptodev
-I../../../git/pub/dpdk.org/main/lib/cryptodev -Ilib/rcu
-I../../../git/pub/dpdk.org/main/lib/rcu -Ilib/hash
-I../../../git/pub/dpdk.org/main/lib/hash -Ilib/pci
-I../../../git/pub/dpdk.org/main/lib/pci -Ilib/dmadev
-I../../../git/pub/dpdk.org/main/lib/dmadev -fdiagnostics-color=always
-D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c11
-O2 -g -include rte_config.h -Wcast-qual -Wdeprecated -Wformat
-Wformat-nonliteral -Wformat-security -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wold-style-definition
-Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef
-Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned
-Wno-missing-field-initializers -Wno-zero-length-bounds
-Wno-pointer-to-int-cast -D_GNU_SOURCE -m32 -fPIC -march=native -mrtm
-DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation
-DVHOST_GCC_UNROLL_PRAGMA -fno-strict-aliasing -DVHOST_HAS_VDUSE
-DRTE_LOG_DEFAULT_LOGTYPE=lib.vhost -MD -MQ
lib/librte_vhost.a.p/vhost_virtio_net.c.o -MF lib/librte_vhost.a.p/vhost_virtio_net.c.o.d -o lib/librte_vhost.a.p/vhost_virtio_net.c.o -c ../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c
In function ‘mbuf_to_desc’,
inlined from ‘vhost_enqueue_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1948:6,
inlined from ‘virtio_dev_rx_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1965:6,
inlined from ‘virtio_dev_rx_async_submit_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:2127:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1230:18: error:
‘buf_vec[0].buf_addr’ may be used uninitialized [-Werror=maybe-uninitialized]
 1230 |         buf_addr = buf_vec[vec_idx].buf_addr;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1963:27: note:
‘buf_vec’ declared here
1963 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
In function ‘mbuf_to_desc’,
inlined from ‘vhost_enqueue_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1948:6,
inlined from ‘virtio_dev_rx_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1965:6,
inlined from ‘virtio_dev_rx_async_submit_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:2127:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1231:18: error:
‘buf_vec[0].buf_iova’ may be used uninitialized [-Werror=maybe-uninitialized]
 1231 |         buf_iova = buf_vec[vec_idx].buf_iova;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1963:27: note:
‘buf_vec’ declared here
1963 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
In function ‘mbuf_to_desc’,
inlined from ‘vhost_enqueue_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1948:6,
inlined from ‘virtio_dev_rx_async_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1965:6,
inlined from ‘virtio_dev_rx_async_submit_packed’ at
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:2127:7:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1232:35: error:
‘buf_vec[0].buf_len’ may be used uninitialized [-Werror=maybe-uninitialized]
 1232 |         buf_len = buf_vec[vec_idx].buf_len;
      |                   ~~~~~~~~~~~~~~~~^~~~~~~~
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c: In function ‘virtio_dev_rx_async_submit_packed’:
../../../git/pub/dpdk.org/main/lib/vhost/virtio_net.c:1963:27: note:
‘buf_vec’ declared here
1963 | struct buf_vector buf_vec[BUF_VECTOR_MAX];
| ^~~~~~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

Quoted 2 lines

I had a look at the vhost library and I think the compiler thinks size may be 0.

In 32 bit architecture, "size" could be zero, ref. line 1909:

uint64_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf);

The values on both sides of the addition are 32 bit unsigned, so the addition will be performed as 32 bit unsigned, and the result could in theory be 0. Casting one of them to 64 bit, e.g. (uint64_t)sizeof(struct virtio_net_hdr_mrg_rxbuf), might fix the warning if the compiler is clever enough to know that adding a non-zero 64 bit value to any 32 bit value cannot give zero as the result.

Side note, off track: Why is "size" uint64_t, wouldn't uint32_t suffice?

Quoted 10 lines

Changing the loop on size with a do { } while (size > 0); resolves the warning.
I can post a change for this, as we hit a similar issue in the past and the code does not make sense comparing size on the first iteration of this loop.
However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...

That is indeed the relevant question here!

Perhaps the compiler somehow ignores the "const" part of the parameter given to the "asm" (in rte_prefetch0()) for 64 bit arch, but not for 32 bit arch?

Quoted 5 lines

--
David Marchand

David Marchand

2023-08-24 14:18 UTC #219 lore raw

Re: [PATCH v11 06/16] eal: use prefetch intrinsics

Quoted 9 lines · Morten Brørup

Quoted 5 lines
From: David Marchand [mailto:[email protected]] However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...
That is indeed the relevant question here!
Perhaps the compiler somehow ignores the "const" part of the parameter given to the "asm" (in rte_prefetch0()) for 64 bit arch, but not for 32 bit arch?

It is possible to reproduce the issue with current DPDK tree (with existing prefetch implementation in asm) and removing all rte_prefetch0 calls from the async rx path:

lib/vhost/virtio_net.c +0 -0

index d7624d18c8..6f941cf27d 100644
@@ -748,8 +748,6 @@ map_one_desc(struct virtio_net *dev, struct

vhost_virtqueue *vq,

                if (unlikely(!desc_addr))
                        return -1;

- rte_prefetch0((void *)(uintptr_t)desc_addr);
-

                buf_vec[vec_id].buf_iova = desc_iova;
                buf_vec[vec_id].buf_addr = desc_addr;
                buf_vec[vec_id].buf_len  = desc_chunck_len;
@@ -1808,8 +1806,6 @@ virtio_dev_rx_async_submit_split(struct
virtio_net *dev, struct vhost_virtqueue
         */
        avail_head = __atomic_load_n(&vq->avail->idx, __ATOMIC_ACQUIRE);

- rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
-

        async_iter_reset(async);

        for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {
@@ -1997,7 +1993,6 @@ virtio_dev_rx_async_packed_batch_enqueue(struct
virtio_net *dev,
        uint16_t i;

        vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
-               rte_prefetch0((void *)(uintptr_t)desc_addrs[i]);
                desc = vhost_iova_to_vva(dev, vq, desc_addrs[i],
&lens[i], VHOST_ACCESS_RW);
                hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)desc;
                lens[i] = pkts[i]->pkt_len +
@@ -2106,8 +2101,6 @@ virtio_dev_rx_async_submit_packed(struct
virtio_net *dev, struct vhost_virtqueue
        uint16_t i;

        do {

- rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
-
if (count - pkt_idx >= PACKED_BATCH_SIZE) {
if (!virtio_dev_rx_async_packed_batch(dev, vq,
&pkts[pkt_idx],

                                        dma_id, vchan_id)) {

If any prefetch is left, the warning disappears.
So the asm usage probably impacts/disables some compiler feature, for this code path.

Signature · 1 line

David Marchand

Morten Brørup

2023-08-24 14:43 UTC #220 lore raw

RE: [PATCH v11 06/16] eal: use prefetch intrinsics

Quoted 64 lines

From: David Marchand [mailto:[email protected]]
Sent: Thursday, 24 August 2023 16.18

Quoted 9 lines · Morten Brørup

Quoted 5 lines
From: David Marchand [mailto:[email protected]] However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...
That is indeed the relevant question here!
Perhaps the compiler somehow ignores the "const" part of the parameter given

to the "asm" (in rte_prefetch0()) for 64 bit arch, but not for 32 bit arch?

It is possible to reproduce the issue with current DPDK tree (with existing prefetch implementation in asm) and removing all rte_prefetch0 calls from the async rx path:

lib/vhost/virtio_net.c +0 -2

index d7624d18c8..6f941cf27d 100644
@@ -748,8 +748,6 @@ map_one_desc(struct virtio_net *dev, struct
 vhost_virtqueue *vq,
                if (unlikely(!desc_addr))
                        return -1;
 
-               rte_prefetch0((void *)(uintptr_t)desc_addr);
-
                buf_vec[vec_id].buf_iova = desc_iova;
                buf_vec[vec_id].buf_addr = desc_addr;

                buf_vec[vec_id].buf_len  = desc_chunck_len;

lib/vhost/virtio_net.c +0 -0

@@ -1808,8 +1806,6 @@ virtio_dev_rx_async_submit_split(struct

virtio_net *dev, struct vhost_virtqueue

         */
        avail_head = __atomic_load_n(&vq->avail->idx, __ATOMIC_ACQUIRE);

lib/vhost/virtio_net.c +0 -3

-       rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
-
        async_iter_reset(async);
 
        for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {

@@ -1997,7 +1993,6 @@ virtio_dev_rx_async_packed_batch_enqueue(struct
 virtio_net *dev,
        uint16_t i;
 
        vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
-               rte_prefetch0((void *)(uintptr_t)desc_addrs[i]);
                desc = vhost_iova_to_vva(dev, vq, desc_addrs[i],
 &lens[i], VHOST_ACCESS_RW);

                hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)desc;
                lens[i] = pkts[i]->pkt_len +

lib/vhost/virtio_net.c +0 -0

@@ -2106,8 +2101,6 @@ virtio_dev_rx_async_submit_packed(struct

virtio_net *dev, struct vhost_virtqueue

        uint16_t i;

        do {

lib/vhost/virtio_net.c +0 -2

-               rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
-
                if (count - pkt_idx >= PACKED_BATCH_SIZE) {
                        if (!virtio_dev_rx_async_packed_batch(dev, vq,

&pkts[pkt_idx],

Quoted 6 lines

                                        dma_id, vchan_id)) {
If any prefetch is left, the warning disappears.
So the asm usage probably impacts/disables some compiler feature, for this code path.

Perhaps any asm usage causes the compiler to assume that any memory might have been modified, which then causes the compiler to not warn about the use of potentially uninitialized variables.

If so, then compiling other code using asm might also not omit warnings about potentially uninitialized variables where it otherwise should have.

Tyler Retzlaff

author

2023-08-24 15:54 UTC #221 lore raw

Re: [PATCH v11 06/16] eal: use prefetch intrinsics

In reply to David Marchand

Quoted 59 lines · David Marchand

Quoted 9 lines · Morten Brørup

Quoted 5 lines
From: David Marchand [mailto:[email protected]] However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...
That is indeed the relevant question here!
Perhaps the compiler somehow ignores the "const" part of the parameter given to the "asm" (in rte_prefetch0()) for 64 bit arch, but not for 32 bit arch?

It is possible to reproduce the issue with current DPDK tree (with existing prefetch implementation in asm) and removing all rte_prefetch0 calls from the async rx path:

lib/vhost/virtio_net.c +0 -2

index d7624d18c8..6f941cf27d 100644
@@ -748,8 +748,6 @@ map_one_desc(struct virtio_net *dev, struct
 vhost_virtqueue *vq,
                if (unlikely(!desc_addr))
                        return -1;
 
-               rte_prefetch0((void *)(uintptr_t)desc_addr);
-
                buf_vec[vec_id].buf_iova = desc_iova;
                buf_vec[vec_id].buf_addr = desc_addr;

                buf_vec[vec_id].buf_len  = desc_chunck_len;

lib/vhost/virtio_net.c +0 -0

@@ -1808,8 +1806,6 @@ virtio_dev_rx_async_submit_split(struct

virtio_net *dev, struct vhost_virtqueue

         */
        avail_head = __atomic_load_n(&vq->avail->idx, __ATOMIC_ACQUIRE);

lib/vhost/virtio_net.c +0 -3

-       rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
-
        async_iter_reset(async);
 
        for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {

@@ -1997,7 +1993,6 @@ virtio_dev_rx_async_packed_batch_enqueue(struct
 virtio_net *dev,
        uint16_t i;
 
        vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
-               rte_prefetch0((void *)(uintptr_t)desc_addrs[i]);
                desc = vhost_iova_to_vva(dev, vq, desc_addrs[i],
 &lens[i], VHOST_ACCESS_RW);

                hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)desc;
                lens[i] = pkts[i]->pkt_len +

lib/vhost/virtio_net.c +0 -0

@@ -2106,8 +2101,6 @@ virtio_dev_rx_async_submit_packed(struct

virtio_net *dev, struct vhost_virtqueue

        uint16_t i;

        do {

lib/vhost/virtio_net.c +0 -2

-               rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
-
                if (count - pkt_idx >= PACKED_BATCH_SIZE) {
                        if (!virtio_dev_rx_async_packed_batch(dev, vq,

&pkts[pkt_idx],

Quoted 6 lines

                                        dma_id, vchan_id)) {
If any prefetch is left, the warning disappears.
So the asm usage probably impacts/disables some compiler feature, for this code path.

So as an aside one of the reasons often cited as to why intrinsics are preferred over inline asm is that inline asm can't be considered when the compiler performs optimization. It could be that when moving to use the intrinsics the scope of what can be optimized around the prefetch0 calls can be used for optimization leading to the observations here.

Quoted 4 lines

Signature · 2 lines
-- 
David Marchand

David Marchand

2023-08-25 08:44 UTC #222 lore raw

Re: [PATCH v11 06/16] eal: use prefetch intrinsics

Quoted 64 lines · Tyler Retzlaff

Quoted 62 lines · David Marchand

Quoted 9 lines · Morten Brørup

Quoted 5 lines
From: David Marchand [mailto:[email protected]] However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...
That is indeed the relevant question here!
Perhaps the compiler somehow ignores the "const" part of the parameter given to the "asm" (in rte_prefetch0()) for 64 bit arch, but not for 32 bit arch?

It is possible to reproduce the issue with current DPDK tree (with existing prefetch implementation in asm) and removing all rte_prefetch0 calls from the async rx path:

lib/vhost/virtio_net.c +0 -2

index d7624d18c8..6f941cf27d 100644
@@ -748,8 +748,6 @@ map_one_desc(struct virtio_net *dev, struct
 vhost_virtqueue *vq,
                if (unlikely(!desc_addr))
                        return -1;
 
-               rte_prefetch0((void *)(uintptr_t)desc_addr);
-
                buf_vec[vec_id].buf_iova = desc_iova;
                buf_vec[vec_id].buf_addr = desc_addr;

                buf_vec[vec_id].buf_len  = desc_chunck_len;

lib/vhost/virtio_net.c +0 -0

@@ -1808,8 +1806,6 @@ virtio_dev_rx_async_submit_split(struct

virtio_net *dev, struct vhost_virtqueue

         */
        avail_head = __atomic_load_n(&vq->avail->idx, __ATOMIC_ACQUIRE);

lib/vhost/virtio_net.c +0 -3

-       rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
-
        async_iter_reset(async);
 
        for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {

@@ -1997,7 +1993,6 @@ virtio_dev_rx_async_packed_batch_enqueue(struct
 virtio_net *dev,
        uint16_t i;
 
        vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
-               rte_prefetch0((void *)(uintptr_t)desc_addrs[i]);
                desc = vhost_iova_to_vva(dev, vq, desc_addrs[i],
 &lens[i], VHOST_ACCESS_RW);

                hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)desc;
                lens[i] = pkts[i]->pkt_len +

lib/vhost/virtio_net.c +0 -0

@@ -2106,8 +2101,6 @@ virtio_dev_rx_async_submit_packed(struct

virtio_net *dev, struct vhost_virtqueue

        uint16_t i;

        do {

lib/vhost/virtio_net.c +0 -2

-               rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
-
                if (count - pkt_idx >= PACKED_BATCH_SIZE) {
                        if (!virtio_dev_rx_async_packed_batch(dev, vq,

&pkts[pkt_idx],

                                        dma_id, vchan_id)) {

Quoted 9 lines

Quoted 3 lines
If any prefetch is left, the warning disappears.
So the asm usage probably impacts/disables some compiler feature, for this code path.
So as an aside one of the reasons often cited as to why intrinsics are preferred over inline asm is that inline asm can't be considered when the compiler performs optimization. It could be that when moving to use the intrinsics the scope of what can be optimized around the prefetch0 calls can be used for optimization leading to the observations here.

Agreed.
While we decide the next steps on the vhost library, and for making progress on this series, I decided to keep the previous asm implementation and put intrinsics under a #ifdef MSVC.

Signature · 1 line

David Marchand

Tyler Retzlaff

author

2023-08-25 15:46 UTC #223 lore raw

Re: [PATCH v11 06/16] eal: use prefetch intrinsics

Quoted 76 lines · David Marchand

Quoted 64 lines · Tyler Retzlaff
Quoted 62 lines · David Marchand
Quoted 9 lines · Morten Brørup
Quoted 5 lines
From: David Marchand [mailto:[email protected]] However, I am a bit puzzled why the prefetch change makes the compiler consider this loop differently.
We have the same constructs everywhere in this library and x86_64 builds are fine...
That is indeed the relevant question here!
Perhaps the compiler somehow ignores the "const" part of the parameter given to the "asm" (in rte_prefetch0()) for 64 bit arch, but not for 32 bit arch?
It is possible to reproduce the issue with current DPDK tree (with existing prefetch implementation in asm) and removing all rte_prefetch0 calls from the async rx path:
lib/vhost/virtio_net.c +0 -2
index d7624d18c8..6f941cf27d 100644
@@ -748,8 +748,6 @@ map_one_desc(struct virtio_net *dev, struct
 vhost_virtqueue *vq,
                if (unlikely(!desc_addr))
                        return -1;
 
-               rte_prefetch0((void *)(uintptr_t)desc_addr);
-
                buf_vec[vec_id].buf_iova = desc_iova;
                buf_vec[vec_id].buf_addr = desc_addr;
                buf_vec[vec_id].buf_len  = desc_chunck_len;
lib/vhost/virtio_net.c +0 -0
@@ -1808,8 +1806,6 @@ virtio_dev_rx_async_submit_split(struct
virtio_net *dev, struct vhost_virtqueue
         */
        avail_head = __atomic_load_n(&vq->avail->idx, __ATOMIC_ACQUIRE);
lib/vhost/virtio_net.c +0 -3
-       rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
-
        async_iter_reset(async);
 
        for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {
@@ -1997,7 +1993,6 @@ virtio_dev_rx_async_packed_batch_enqueue(struct
 virtio_net *dev,
        uint16_t i;
 
        vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) {
-               rte_prefetch0((void *)(uintptr_t)desc_addrs[i]);
                desc = vhost_iova_to_vva(dev, vq, desc_addrs[i],
 &lens[i], VHOST_ACCESS_RW);
                hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)desc;
                lens[i] = pkts[i]->pkt_len +
lib/vhost/virtio_net.c +0 -0
@@ -2106,8 +2101,6 @@ virtio_dev_rx_async_submit_packed(struct
virtio_net *dev, struct vhost_virtqueue
        uint16_t i;

        do {
lib/vhost/virtio_net.c +0 -2
-               rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
-
                if (count - pkt_idx >= PACKED_BATCH_SIZE) {
                        if (!virtio_dev_rx_async_packed_batch(dev, vq,
&pkts[pkt_idx],
                                        dma_id, vchan_id)) {
Quoted 9 lines
Quoted 3 lines
If any prefetch is left, the warning disappears.
So the asm usage probably impacts/disables some compiler feature, for this code path.
So as an aside one of the reasons often cited as to why intrinsics are preferred over inline asm is that inline asm can't be considered when the compiler performs optimization. It could be that when moving to use the intrinsics the scope of what can be optimized around the prefetch0 calls can be used for optimization leading to the observations here.

Quoted 4 lines

Agreed.
While we decide the next steps on the vhost library, and for making progress on this series, I decided to keep the previous asm implementation and put intrinsics under a #ifdef MSVC.

I'm fine with this, we can micro-optimize things out of the MSVC only blocks later in more targeted changes. These kinds of little things is why earlier versions of the series only used intrinsics for MSVC bah!

Thanks

Quoted 3 lines

Signature · 2 lines
-- 
David Marchand

Tyler Retzlaff

author

2023-08-11 19:21 UTC #224 lore raw

PATCH v11 7/16 eal: use byte swap intrinsics

In reply to Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead expand use _byteswap_u{ushort,ulong,uint64} intrinsics instead.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_byteorder.h | 11 +++++++++++
 lib/eal/x86/include/rte_byteorder.h     |  4 ++++
 2 files changed, 15 insertions(+)

lib/eal/include/generic/rte_byteorder.h +11 -0

index a67e1d7..1e32a1b 100644
@@ -234,6 +234,7 @@
 #endif /* __DOXYGEN__ */
 
 #ifdef RTE_FORCE_INTRINSICS
+#ifndef RTE_TOOLCHAIN_MSVC
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
 #define rte_bswap16(x) __builtin_bswap16(x)
 #endif

@@ -241,6 +242,16 @@
 #define rte_bswap32(x) __builtin_bswap32(x)
 
 #define rte_bswap64(x) __builtin_bswap64(x)
+#else
+/*
+ * note: we may want to #pragma intrsinsic(_byteswap_u{short,long,uint64})
+ */
+#define rte_bswap16(x) _byteswap_ushort(x)
+
+#define rte_bswap32(x) _byteswap_ulong(x)
+
+#define rte_bswap64(x) _byteswap_uint64(x)
+#endif
 
 #endif

lib/eal/x86/include/rte_byteorder.h +4 -0

index a2dfecc..3d6e58e 100644
@@ -18,6 +18,7 @@
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
 
+#ifndef RTE_TOOLCHAIN_MSVC
 /*
  * An architecture-optimized byte swap for a 16-bit value.
  *

@@ -69,6 +70,7 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 				   rte_arch_bswap16(x)))
 #endif
 #endif
+#endif
 
 #define rte_cpu_to_le_16(x) (x)
 #define rte_cpu_to_le_32(x) (x)

@@ -86,11 +88,13 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 #define rte_be_to_cpu_32(x) rte_bswap32(x)
 #define rte_be_to_cpu_64(x) rte_bswap64(x)
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifdef RTE_ARCH_I686
 #include "rte_byteorder_32.h"
 #else
 #include "rte_byteorder_64.h"
 #endif
+#endif
 
 #ifdef __cplusplus
 }

Signature · 1 line

1.8.3.1

David Marchand

2023-08-25 08:45 UTC #225 lore raw

Re: [PATCH v11 07/16] eal: use byte swap intrinsics

Quoted 6 lines · Tyler Retzlaff

Inline assembly is not supported for MSVC x64 instead expand use _byteswap_u{ushort,ulong,uint64} intrinsics instead.
Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>

For the record, I rebased this patch on top of
https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/
and moved blocks under a single #ifdef RTE_FORCE_INTRINSICS.

Signature · 1 line

David Marchand

Tyler Retzlaff

author

2023-08-11 19:22 UTC #226 lore raw

PATCH v11 8/16 eal: hide GCC extension based alignment markers

In reply to Tyler Retzlaff

When compiling with MSVC don't expose typedefs used as alignment markers.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_common.h | 4 ++++
 1 file changed, 4 insertions(+)

lib/eal/include/rte_common.h +4 -0

index 15765b4..2f464e3 100644
@@ -460,6 +460,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
 
 /*********** Structure alignment markers ********/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /** Generic marker for any place in a structure. */
 __extension__ typedef void    *RTE_MARKER[0];
 /** Marker for 1B alignment in a structure. */

@@ -471,6 +473,8 @@ static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
 /** Marker for 8B alignment in a structure. */
 __extension__ typedef uint64_t RTE_MARKER64[0];
 
+#endif
+
 /**
  * Combines 32b inputs most significant set bits into the least
  * significant bits to construct a value with the same MSBs as x

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:21 UTC #227 lore raw

PATCH v11 9/16 eal: hide typedefs based on GCC vector extensions

In reply to Tyler Retzlaff

When compiling with MSVC don't expose typedefs based on GCC vector extensions.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_vect.h | 4 ++++
 1 file changed, 4 insertions(+)

lib/eal/include/generic/rte_vect.h +4 -0

index 3fec2bf..777510c 100644
@@ -17,6 +17,8 @@
 
 #include <rte_compat.h>
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /* Unsigned vector types */
 
 /**

@@ -186,6 +188,8 @@
  */
 typedef int64_t rte_v256s64_t __attribute__((vector_size(32), aligned(32)));
 
+#endif
+
 /**
  * The max SIMD bitwidth value to limit vector path selection.
  */

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:22 UTC #228 lore raw

PATCH v11 10/16 eal: expand most macros to empty when using MSVC

In reply to Tyler Retzlaff

Later we will return and provide (where possible) expansions that work correctly for msvc and where not possible provide some alternate macros to achieve the same outcome.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_branch_prediction.h |  8 +++++
 lib/eal/include/rte_common.h            | 54 +++++++++++++++++++++++++++++++++
 lib/eal/include/rte_compat.h            | 20 ++++++++++++
 3 files changed, 82 insertions(+)

lib/eal/include/rte_branch_prediction.h +8 -0

index 414cd92..c0356ca 100644
@@ -24,7 +24,11 @@
  *      do_stuff();
  */
 #ifndef likely
+#ifdef RTE_TOOLCHAIN_MSVC
+#define likely(x)	(!!(x))
+#else
 #define likely(x)	__builtin_expect(!!(x), 1)
+#endif
 #endif /* likely */
 
 /**

@@ -37,7 +41,11 @@
  *      do_stuff();
  */
 #ifndef unlikely
+#ifdef RTE_TOOLCHAIN_MSVC
+#define unlikely(x)	(!!(x))
+#else
 #define unlikely(x)	__builtin_expect(!!(x), 0)
+#endif
 #endif /* unlikely */
 
 #ifdef __cplusplus

lib/eal/include/rte_common.h +54 -0

147 lines not on this page · raw message

lib/eal/include/rte_compat.h +20 -0

index fc9fbaa..716bc03 100644
@@ -12,14 +12,22 @@
 
 #ifndef ALLOW_EXPERIMENTAL_API
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_experimental
+#else
 #define __rte_experimental \
 __attribute__((deprecated("Symbol is not yet part of stable ABI"), \
 section(".text.experimental")))
+#endif
 
 #else
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_experimental
+#else
 #define __rte_experimental \
 __attribute__((section(".text.experimental")))
+#endif
 
 #endif

@@ -30,23 +38,35 @@
 
 #if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 __attribute__((error("Symbol is not public ABI"), \
 section(".text.internal")))
+#endif
 
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 _Pragma("GCC diagnostic push") \
 _Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
 section(".text.internal"))) \
 _Pragma("GCC diagnostic pop")
+#endif
 
 #else
 
+#ifdef RTE_TOOLCHAIN_MSVC
+#define __rte_internal
+#else
 #define __rte_internal \
 __attribute__((section(".text.internal")))
+#endif
 
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:22 UTC #229 lore raw

PATCH v11 11/16 eal: exclude exposure of rte atomic APIs for MSVC builds

In reply to Tyler Retzlaff

It's discouraged to use rte_atomics APIs instead standard APIs should be used from C11. Since MSVC is a new toolchain/platform combination block visibility of the rte_atomic APIs from day 1.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_atomic.h | 7 +++++++
 lib/eal/x86/include/rte_atomic.h     | 4 ++++
 2 files changed, 11 insertions(+)

lib/eal/include/generic/rte_atomic.h +7 -0

index 2d36eb0..b14e9fe 100644
@@ -161,6 +161,8 @@
 
 /*------------------------- 16 bit atomic operations -------------------------*/
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Atomic compare and set.
  *

@@ -1068,8 +1070,11 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 }
 #endif
 
+#endif
+
 /*------------------------ 128 bit atomic operations -------------------------*/
 
+
 /**
  * 128-bit integer structure.
  */

@@ -1079,8 +1084,10 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
 	union {
 		uint64_t val[2];
 #ifdef RTE_ARCH_64
+#ifndef RTE_TOOLCHAIN_MSVC
 		__extension__ __int128 int128;
 #endif
+#endif
 	};
 } __rte_aligned(16) rte_int128_t;

lib/eal/x86/include/rte_atomic.h +4 -0

index 7aba1c3..9ba61f8 100644
@@ -83,6 +83,8 @@
 
 #define rte_io_rmb() rte_compiler_barrier()
 
+#ifndef RTE_TOOLCHAIN_MSVC
+
 /**
  * Synchronization fence between threads based on the specified memory order.
  *

@@ -279,6 +281,8 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 #include "rte_atomic_64.h"
 #endif
 
+#endif
+
 #ifdef __cplusplus
 }
 #endif

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:21 UTC #230 lore raw

PATCH v11 12/16 eal: always define MSVC as little endian

In reply to Tyler Retzlaff

The MSVC compiler does not target big endian platforms so define little endian always.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/generic/rte_byteorder.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/generic/rte_byteorder.h +2 -0

index 1e32a1b..375f118 100644
@@ -45,6 +45,8 @@
 #define RTE_BYTE_ORDER RTE_BIG_ENDIAN
 #elif defined __LITTLE_ENDIAN__
 #define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#elif defined RTE_TOOLCHAIN_MSVC
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
 #endif
 #if !defined(RTE_BYTE_ORDER)
 #error Unknown endianness.

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:22 UTC #231 lore raw

PATCH v11 13/16 eal: do not define typeof macro when building with MSVC

In reply to Tyler Retzlaff

When building with MSVC do not assume typeof is a macro and don't define a typeof macro that conflicts with C23 typeof keyword.

Signed-off-by Tyler Retzlaff <[email protected]>

Acked-by Morten Brørup <[email protected]>

---
 lib/eal/include/rte_common.h | 2 ++
 1 file changed, 2 insertions(+)

lib/eal/include/rte_common.h +2 -0

index b087532..cf7d8d8 100644
@@ -24,9 +24,11 @@
 /* OS specific include */
 #include <rte_os.h>
 
+#ifndef RTE_TOOLCHAIN_MSVC
 #ifndef typeof
 #define typeof __typeof__
 #endif
+#endif
 
 #ifndef __cplusplus
 #ifndef asm

Signature · 1 line

1.8.3.1

Tyler Retzlaff

author

2023-08-11 19:22 UTC #232 lore raw

PATCH v11 14/16 log: use standard ternary operator instead of GCC extension

In reply to Tyler Retzlaff

Use standard ternary operator instead of gcc extension. There is no concern of side-effects for this evaluation so allow the code to be portable.

While here update the condition to compare default_log_stream directly against NULL.

Signed-off-by Tyler Retzlaff <[email protected]>

---
 lib/log/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/log/log.c +1 -1

index 52c771f..b80725a 100644
@@ -93,7 +93,7 @@ struct log_cur_msg {
 		 * of stderr, even if the application closes and
 		 * reopens it.
 		 */
-		return default_log_stream ? : stderr;
+		return default_log_stream != NULL ? default_log_stream : stderr;
 	}
 	return f;
 }

Signature · 1 line

1.8.3.1

Morten Brørup

Reviewed-by on v11 14/16

2023-09-25 06:25 UTC #233 lore raw

RE: [PATCH v11 14/16] log: use standard ternary operator instead of GCC extension

Quoted 20 lines

From: Tyler Retzlaff [mailto:[email protected]]
Sent: Friday, 11 August 2023 21.21
Use standard ternary operator instead of gcc extension. There is no concern of side-effects for this evaluation so allow the code to be portable.
While here update the condition to compare default_log_stream directly against NULL.
Signed-off-by Tyler Retzlaff <[email protected]>
---
 lib/log/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/log/log.c +0 -0
index 52c771f..b80725a 100644

lib/log/log.c struct log_cur_msg { · line 96

Quoted 4 lines

lib/log/log.c +0 -1

@@ -93,7 +93,7 @@ struct log_cur_msg {
 		 * of stderr, even if the application closes and
 		 * reopens it.
 		 */
-		return default_log_stream ? : stderr;

I didn't know this GCC extension.

Someone please consider disallowing this in checkpatches.

Quoted 6 lines

lib/log/log.c +1 -1

+		return default_log_stream != NULL ? default_log_stream : stderr;
 	}
 	return f;
 }
--

1.8.3.1

Reviewed-by Morten Brørup <[email protected]>

Stephen Hemminger

2023-09-25 15:09 UTC #234 lore raw

Re: [PATCH v11 14/16] log: use standard ternary operator instead of GCC extension

Quoted 3 lines · On Mon, 25 Sep 2023 08:24:58 +0200 Morten Brørup <[email protected]>…

I didn't know this GCC extension.
Someone please consider disallowing this in checkpatches.

We don't need to make checkpatch more complex.
When MSVC is in build system, it will catch it

Tyler Retzlaff

author

2023-08-11 19:22 UTC #235 lore raw

PATCH v11 15/16 eal: use standard ternary operator instead of GCC extension

In reply to Tyler Retzlaff

Use standard ternary operator instead of gcc extension. There is no concern of side-effects for this evaluation so allow the code to be portable.

While here update the condition to compare default_log_stream directly against NULL.

Signed-off-by Tyler Retzlaff <[email protected]>

---
 lib/eal/common/eal_common_hexdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

lib/eal/common/eal_common_hexdump.c +1 -1

index 63bbbdc..6fd6e21 100644
@@ -15,7 +15,7 @@
 	char line[LINE_LEN];	/* space needed 8+16*3+3+16 == 75 */
 
 	fprintf(f, "%s at [%p], len=%u\n",
-		title ? : "  Dump data", data, len);
+		title != NULL ? title : "  Dump data", data, len);
 	ofs = 0;
 	while (ofs < len) {
 		/* format the line in the buffer */

Signature · 1 line

1.8.3.1

Morten Brørup

Reviewed-by on v11 15/16

2023-09-25 06:25 UTC #236 lore raw

RE: [PATCH v11 15/16] eal: use standard ternary operator instead of GCC extension

Quoted 30 lines

From: Tyler Retzlaff [mailto:[email protected]]
Sent: Friday, 11 August 2023 21.21
Use standard ternary operator instead of gcc extension. There is no concern of side-effects for this evaluation so allow the code to be portable.
While here update the condition to compare default_log_stream directly against NULL.
Signed-off-by Tyler Retzlaff <[email protected]>
---
 lib/eal/common/eal_common_hexdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/eal/common/eal_common_hexdump.c +1 -2
index 63bbbdc..6fd6e21 100644
@@ -15,7 +15,7 @@
 	char line[LINE_LEN];	/* space needed 8+16*3+3+16 == 75 */
 
 	fprintf(f, "%s at [%p], len=%u\n",
-		title ? : "  Dump data", data, len);
+		title != NULL ? title : "  Dump data", data, len);
 	ofs = 0;
 	while (ofs < len) {
 		/* format the line in the buffer */
--

Quoted 1 line

1.8.3.1

Reviewed-by Morten Brørup <[email protected]>

Tyler Retzlaff

author

2023-08-11 19:22 UTC #237 lore raw

PATCH v11 16/16 eal: define priority based ctor dtor for MSVC

In reply to Tyler Retzlaff

Provide RTE_INIT_PRIO and RTE_FINI_PRIO for MSVC allowing priority based equivalents to __attribute__(({constructor,destructor})

Signed-off-by Tyler Retzlaff <[email protected]>

---
 lib/eal/include/rte_common.h | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

lib/eal/include/rte_common.h +37 -0

index cf7d8d8..791ceed 100644
@@ -214,8 +214,29 @@
  *   Lowest number is the first to run.
  */
 #ifndef RTE_INIT_PRIO /* Allow to override from EAL */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define RTE_INIT_PRIO(func, prio) \
 static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
+#else
+/* definition from the Microsoft CRT */
+typedef int(__cdecl *_PIFV)(void);
+
+#define CTOR_SECTION_LOG ".CRT$XIB"
+#define CTOR_SECTION_BUS ".CRT$XIC"
+#define CTOR_SECTION_CLASS ".CRT$XID"
+#define CTOR_SECTION_LAST ".CRT$XIY"
+
+#define CTOR_PRIORITY_TO_SECTION(priority) CTOR_SECTION_ ## priority
+
+#define RTE_INIT_PRIO(name, priority) \
+	static void name(void); \
+	static int __cdecl name ## _thunk(void) { name(); return 0; } \
+	__pragma(const_seg(CTOR_PRIORITY_TO_SECTION(priority))) \
+	__declspec(allocate(CTOR_PRIORITY_TO_SECTION(priority))) \
+	    _PIFV name ## _pointer = &name ## _thunk; \
+	__pragma(const_seg()) \
+	static void name(void)
+#endif
 #endif
 
 /**

@@ -239,8 +260,24 @@ static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
  *   Lowest number is the last to run.
  */
 #ifndef RTE_FINI_PRIO /* Allow to override from EAL */
+#ifndef RTE_TOOLCHAIN_MSVC
 #define RTE_FINI_PRIO(func, prio) \
 static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
+#else
+#define DTOR_SECTION_LOG "mydtor$B"
+#define DTOR_SECTION_BUS "mydtor$C"
+#define DTOR_SECTION_CLASS "mydtor$D"
+#define DTOR_SECTION_LAST "mydtor$Y"
+
+#define DTOR_PRIORITY_TO_SECTION(priority) DTOR_SECTION_ ## priority
+
+#define RTE_FINI_PRIO(name, priority) \
+	static void name(void); \
+	__pragma(const_seg(DTOR_PRIORITY_TO_SECTION(priority))) \
+	__declspec(allocate(DTOR_PRIORITY_TO_SECTION(priority))) name ## _pointer = &name; \
+	__pragma(const_seg()) \
+	static void name(void)
+#endif
 #endif
 
 /**

Signature · 1 line

1.8.3.1

Morten Brørup

Acked-by on v11 16/16

2023-09-25 06:28 UTC #238 lore raw

RE: [PATCH v11 16/16] eal: define priority based ctor dtor for MSVC

Quoted 8 lines

From: Tyler Retzlaff [mailto:[email protected]]
Sent: Friday, 11 August 2023 21.21
Provide RTE_INIT_PRIO and RTE_FINI_PRIO for MSVC allowing priority based equivalents to __attribute__(({constructor,destructor})
Signed-off-by Tyler Retzlaff <[email protected]>
---

Acked-by Morten Brørup <[email protected]>

David Marchand

2023-08-25 08:47 UTC #239 lore raw

In reply to Tyler Retzlaff

Quoted 9 lines · Tyler Retzlaff

In accordance with draft plan
http://mails.dpdk.org/archives/web/2023-February/002023.html
introduces conditionally compiled code to enable building with MSVC that _does not_ require C99/C11 meaning it can be integrated now.
This series covers minimal changes for item #2 in draft plan for EAL dependencies log, kvargs, telemetry and consumed EAL public headers.

Quoted 8 lines

Note if any patch in the series requires in-depth discussion I'll detach it from this series for separate submission & more focused discussion so it doesn't block the entire series.
Note other "common" intrinsics were investigated for viability but were not adopted either because they were not available on older versions of GCC or because they generate different code for msvc vs GCC.

As I mentionned on the prefetch and byteorder patches, I made some adjustments while rebasing the series.
Hopefully, I broke nothing.

Series applied, thanks Tyler and reviewers.

Signature · 1 line

David Marchand