On 12/5/2020 5:52 AM, Dmitry Kozlyuk wrote:
On Fri, 4 Dec 2020 17:10:19 -0800, Pallavi Kadam wrote:
You could drop "add changes" and "i40e PMD" from subject line, as any commit
changes something and topic is "net/i40e" already.
Adding build changes to compile i40e PMD on windows.
This is redundant given the commit subject.
Please use present simple tense for changes description (this applies to
sibling patches).
Ok, I'll update the title and description in v2.
Disabling few warnings with Clang such as comparison of integers of
different signs and macro redefinitions.
Also, adding linking dependency source file rte_random.c file to
Windows.
Signed-off-by: Pallavi Kadam <pallavi.ka...@intel.com>
Reviewed-by: Ranjit Menon <ranjit.me...@intel.com>
---
drivers/net/i40e/base/i40e_osdep.h | 3 +++
drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
drivers/net/i40e/i40e_rxtx_vec_avx2.c | 2 ++
drivers/net/i40e/i40e_tm.c | 2 +-
drivers/net/meson.build | 9 ++++++---
lib/librte_eal/common/meson.build | 1 +
lib/librte_eal/rte_eal_exports.def | 1 +
lib/librte_eal/windows/include/rte_windows.h | 5 +++++
8 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/net/i40e/base/i40e_osdep.h
b/drivers/net/i40e/base/i40e_osdep.h
index 9b5033024..fa22df122 100644
--- a/drivers/net/i40e/base/i40e_osdep.h
+++ b/drivers/net/i40e/base/i40e_osdep.h
@@ -67,8 +67,11 @@ typedef enum i40e_status_code i40e_status;
#define false 0
#define true 1
+/* Avoid macro redifinition warning on Windows */
+#ifndef RTE_EXEC_ENV_WINDOWS
#define min(a,b) RTE_MIN(a,b)
#define max(a,b) RTE_MAX(a,b)
+#endif
Windows min() and max() macros evaluate arguments twice [1], which can be
unacceptable in driver code if used with MMIO. Better #undef min and max
first, then let PMD define them.
It seems we'll have to do that for many PMDs, because rte_os.h must not erase
platform-specific macros, and rte_windows.h is not for generic PMDs.
[1]: https://docs.microsoft.com/en-us/windows/win32/multimedia/max
Thanks for the suggestion. Will first #undef min and max in the same file in v2.
diff --git a/drivers/net/i40e/i40e_rxtx_vec_avx2.c
b/drivers/net/i40e/i40e_rxtx_vec_avx2.c
index 7a558fc73..cf2dc88c5 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_avx2.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_avx2.c
@@ -12,7 +12,9 @@
#include "i40e_rxtx.h"
#include "i40e_rxtx_vec_common.h"
+#ifndef RTE_EXEC_ENV_WINDOWS
#include <x86intrin.h>
+#endif
Just #include <rte_vect.h>, it takes care of x86intrin.h for Windows.
Ok, will #include <rte_vect.h> instead.
diff --git a/lib/librte_eal/windows/include/rte_windows.h
b/lib/librte_eal/windows/include/rte_windows.h
index b82af34f6..822922c11 100644
--- a/lib/librte_eal/windows/include/rte_windows.h
+++ b/lib/librte_eal/windows/include/rte_windows.h
@@ -18,6 +18,11 @@
#define WIN32_LEAN_AND_MEAN
#endif
+#ifdef __clang__
+#undef _m_prefetchw
+#define _m_prefetchw __m_prefetchw
+#endif
+
Please explain in the commit message which problem this solves.
Sure, will explain it in v2.
Can't x86intrin.h be replaced by rte_vect.h in rte_random.c?
Please correct if I am wrong. Here, cause of an error is indirectly due
to rte_windows.h
and not because of #include <x86intrin.h>
Please see below error:
FAILED: lib/76b5a35@@rte_eal@sta/librte_eal_common_rte_random.c.obj
clang @lib/76b5a35@@rte_eal@sta/librte_eal_common_rte_random.c.obj.rsp
In file included from ../lib/librte_eal/common/rte_random.c:13:
In file included from ..\lib/librte_eal/include\rte_eal.h:20:
In file included from ..\lib/librte_eal/include\rte_per_lcore.h:25:
In file included from ..\lib/librte_eal/windows/include\pthread.h:21:
In file included from ..\lib/librte_eal/windows/include\rte_windows.h:27:
In file included from C:\Program Files (x86)\Windows
Kits\10\include\10.0.18362.0\um\windows.h:171:
In file included from C:\Program Files (x86)\Windows
Kits\10\include\10.0.18362.0\shared\windef.h:24:
In file included from C:\Program Files (x86)\Windows
Kits\10\include\10.0.18362.0\shared\minwindef.h:182:
C:\Program Files (x86)\Windows
Kits\10\include\10.0.18362.0\um\winnt.h:3324:1: error: conflicting types
for '_m_prefetchw'
_m_prefetchw (
^
C:\Program Files\LLVM\lib\clang\10.0.0\include\prfchwintrin.h:50:1:
note: previous definition is here
_m_prefetchw(void *__P)
^
1 error generated.
If it's still necessary, __clang__ should be RTE_TOOLCHAIN_CLANG.
will update this in v2. Thank you.
