Re: [dpdk-dev] [PATCH v3 7/8] fib6: introduce AVX512 lookup

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> 
> Add new lookup implementation for FIB6 trie algorithm using
> AVX512 instruction set
> 
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> ---
>  lib/librte_fib/Makefile      |  10 ++
>  lib/librte_fib/meson.build   |   9 ++
>  lib/librte_fib/rte_fib6.h    |   3 +-
>  lib/librte_fib/trie.c        |  21 ++++
>  lib/librte_fib/trie_avx512.c | 269 +++++++++++++++++++++++++++++++++++++++++++
>  lib/librte_fib/trie_avx512.h |  20 ++++
>  6 files changed, 331 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_fib/trie_avx512.c
>  create mode 100644 lib/librte_fib/trie_avx512.h
> 
> diff --git a/lib/librte_fib/Makefile b/lib/librte_fib/Makefile
> index 3958da1..761c7c8 100644
> --- a/lib/librte_fib/Makefile
> +++ b/lib/librte_fib/Makefile
> @@ -25,12 +25,22 @@ grep -q __AVX512F__ && echo 1)
>  CC_AVX512DQ_SUPPORT=$(shell $(CC) -mavx512dq -dM -E - </dev/null 2>&1 | \
>  grep -q __AVX512DQ__ && echo 1)
> 
> +CC_AVX512BW_SUPPORT=$(shell $(CC) -mavx512bw -dM -E - </dev/null 2>&1 | \
> +grep -q __AVX512BW__ && echo 1)
> +
>  ifeq ($(CC_AVX512F_SUPPORT), 1)
>  	ifeq ($(CC_AVX512DQ_SUPPORT), 1)
>  		SRCS-$(CONFIG_RTE_LIBRTE_FIB) += dir24_8_avx512.c
>  		CFLAGS_dir24_8_avx512.o += -mavx512f
>  		CFLAGS_dir24_8_avx512.o += -mavx512dq
>  		CFLAGS_dir24_8.o += -DCC_DIR24_8_AVX512_SUPPORT
> +		ifeq ($(CC_AVX512BW_SUPPORT), 1)
> +			SRCS-$(CONFIG_RTE_LIBRTE_FIB) += trie_avx512.c
> +			CFLAGS_trie_avx512.o += -mavx512f
> +			CFLAGS_trie_avx512.o += -mavx512dq
> +			CFLAGS_trie_avx512.o += -mavx512bw
> +			CFLAGS_trie.o += -DCC_TRIE_AVX512_SUPPORT
> +		endif
>  	endif
>  endif
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_fib/meson.build b/lib/librte_fib/meson.build
> index 0963f3c..98adf11 100644
> --- a/lib/librte_fib/meson.build
> +++ b/lib/librte_fib/meson.build
> @@ -14,5 +14,14 @@ if dpdk_conf.has('RTE_ARCH_X86') and cc.has_argument('-mavx512f')
>  			c_args: cflags + ['-mavx512f'] + ['-mavx512dq'])
>  		objs += dir24_8_avx512_tmp.extract_objects('dir24_8_avx512.c')
>  		cflags += '-DCC_DIR24_8_AVX512_SUPPORT'
> +		if cc.has_argument('-mavx512bw')
> +			trie_avx512_tmp = static_library('trie_avx512_tmp',
> +				'trie_avx512.c',
> +				dependencies: static_rte_eal,
> +				c_args: cflags + ['-mavx512f'] + \
> +					['-mavx512dq'] + ['-mavx512bw'])
> +			objs += trie_avx512_tmp.extract_objects('trie_avx512.c')
> +			cflags += '-DCC_TRIE_AVX512_SUPPORT'
> +		endif
>  	endif
>  endif
> diff --git a/lib/librte_fib/rte_fib6.h b/lib/librte_fib/rte_fib6.h
> index b70369a..c55efdf 100644
> --- a/lib/librte_fib/rte_fib6.h
> +++ b/lib/librte_fib/rte_fib6.h
> @@ -53,7 +53,8 @@ enum rte_fib_trie_nh_sz {
>  };
> 
>  enum rte_fib_trie_lookup_type {
> -	RTE_FIB6_TRIE_SCALAR
> +	RTE_FIB6_TRIE_SCALAR,
> +	RTE_FIB6_TRIE_VECTOR

As a nit - does this enum needs to be public?
If it does, then probably worth to name it VECTOR_AVX512,
in case someone in future will want to add another vector implementation.
Probably same thought for v4.
Apart from that - LGTM.
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>