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). > 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 > 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. > 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. Can't x86intrin.h be replaced by rte_vect.h in rte_random.c? If it's still necessary, __clang__ should be RTE_TOOLCHAIN_CLANG.
