| Message ID | 20220623070119.989558-1-sunil.pai.g@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] dpif-netdev: Simplify AVX512 build time checks to enhance readability. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
> -----Original Message----- > From: Pai G, Sunil <sunil.pai.g@intel.com> > Sent: Thursday 23 June 2022 08:01 > To: dev@openvswitch.org > Cc: echaudro@redhat.com; Finn, Emma <emma.finn@intel.com>; Van Haaren, Harry > <harry.van.haaren@intel.com>; Ferriter, Cian <cian.ferriter@intel.com> > Subject: [PATCH] dpif-netdev: Simplify AVX512 build time checks to enhance readability. > > The preprocessor comparison string to check AVX512 capabilities are > lengthy and effecting user readability. Simpify this by aliasing the checks. > > Suggested-by: Eelco Chaudron <echaudro@redhat.com> > Signed-off-by: Sunil Pai G <sunil.pai.g@intel.com> > --- > lib/dpif-netdev-private-dpif.c | 6 ++++-- > lib/dpif-netdev-private-extract.c | 3 +-- > lib/dpif-netdev-private-extract.h | 10 +++++----- > 3 files changed, 10 insertions(+), 9 deletions(-) > I looked to see if we could apply the simplification done below for DPIF and MFEX to the DPCLS code, but I can see that the AVX512 build checks are only performed in one place for the DPCLS, so it doesn't make sense to do it for DPCLS. I guess if there are more places to perform the DPCLS AVX512 build checks in the future, then we can add a similar alias. For now what you've done makes sense. <snip actual diff away> All the changes LGTM. I also tested on the usual GCC 4.8, 4.9, 5 and 9 to test with all the different AVX512 ISA generating capabilities present and not. This all still works as expected. Thanks for making the code better here Sunil. Acked-by: Cian Ferriter <cian.ferriter@intel.com>
diff --git a/lib/dpif-netdev-private-dpif.c b/lib/dpif-netdev-private-dpif.c index 84d4ec156..ffd6ff907 100644 --- a/lib/dpif-netdev-private-dpif.c +++ b/lib/dpif-netdev-private-dpif.c @@ -27,6 +27,8 @@ #include "util.h" VLOG_DEFINE_THIS_MODULE(dpif_netdev_impl); +#define DPIF_NETDEV_IMPL_AVX512_CHECK (__x86_64__ && HAVE_AVX512F \ + && HAVE_LD_AVX512_GOOD && __SSE4_2__) enum dpif_netdev_impl_info_idx { DPIF_NETDEV_IMPL_SCALAR, @@ -40,7 +42,7 @@ static struct dpif_netdev_impl_info_t dpif_impls[] = { .probe = NULL, .name = "dpif_scalar", }, -#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__) +#if DPIF_NETDEV_IMPL_AVX512_CHECK /* Only available on x86_64 bit builds with SSE 4.2 used for OVS core. */ [DPIF_NETDEV_IMPL_AVX512] = { .input_func = dp_netdev_input_outer_avx512, .probe = dp_netdev_input_outer_avx512_probe, @@ -59,7 +61,7 @@ dp_netdev_impl_get_default(void) int dpif_idx = DPIF_NETDEV_IMPL_SCALAR; /* Configure-time overriding to run test suite on all implementations. */ -#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && __SSE4_2__) +#if DPIF_NETDEV_IMPL_AVX512_CHECK #ifdef DPIF_AVX512_DEFAULT dp_netdev_input_func_probe probe; diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private-extract.c index 9ce4e0909..a70ab6a4d 100644 --- a/lib/dpif-netdev-private-extract.c +++ b/lib/dpif-netdev-private-extract.c @@ -54,8 +54,7 @@ static struct dpif_miniflow_extract_impl mfex_impls[] = { .name = "study", }, /* Compile in implementations only if the compiler ISA checks pass. */ -#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && HAVE_AVX512BW \ - && __SSE4_2__) +#if MFEX_IMPL_AVX512_CHECK #if HAVE_AVX512VBMI [MFEX_IMPL_VBMI_IPv4_UDP] = { .probe = mfex_avx512_vbmi_probe, diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private-extract.h index 092126106..b2cd6779d 100644 --- a/lib/dpif-netdev-private-extract.h +++ b/lib/dpif-netdev-private-extract.h @@ -19,6 +19,9 @@ #include <sys/types.h> +#define MFEX_IMPL_AVX512_CHECK (__x86_64__ && HAVE_AVX512F \ + && HAVE_LD_AVX512_GOOD && HAVE_AVX512BW && __SSE4_2__) + /* Forward declarations. */ struct dp_packet; struct miniflow; @@ -81,8 +84,7 @@ enum dpif_miniflow_extract_impl_idx { MFEX_IMPL_AUTOVALIDATOR, MFEX_IMPL_SCALAR, MFEX_IMPL_STUDY, -#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && HAVE_AVX512BW \ - && __SSE4_2__) +#if MFEX_IMPL_AVX512_CHECK #if HAVE_AVX512VBMI MFEX_IMPL_VBMI_IPv4_UDP, #endif @@ -108,9 +110,7 @@ extern struct ovs_mutex dp_netdev_mutex; /* Define a index which points to the first traffic optimized MFEX * option from the enum list else holds max value. */ -#if (__x86_64__ && HAVE_AVX512F && HAVE_LD_AVX512_GOOD && HAVE_AVX512BW \ - && __SSE4_2__) - +#if MFEX_IMPL_AVX512_CHECK #if HAVE_AVX512VBMI #define MFEX_IMPL_START_IDX MFEX_IMPL_VBMI_IPv4_UDP #else
The preprocessor comparison string to check AVX512 capabilities are lengthy and effecting user readability. Simpify this by aliasing the checks. Suggested-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Sunil Pai G <sunil.pai.g@intel.com> --- lib/dpif-netdev-private-dpif.c | 6 ++++-- lib/dpif-netdev-private-extract.c | 3 +-- lib/dpif-netdev-private-extract.h | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-)