[PATCH 0/3] lib: fix AVX2 checks and macro exposure

[PATCH 0/3] lib: fix AVX2 checks and macro exposure

[Thomas Monjalon]

These are fixes for AVX2 in efd and member libraries.
While at it, I've hidden a macro which was wrongly exported in the API
without having a correct prefix.

Thomas Monjalon (3):
  efd: fix AVX2 support
  member: remove AVX2 build-time checks
  member: hide internal macro

 lib/efd/rte_efd.c           |  3 ++-
 lib/efd/rte_efd_x86.h       | 10 ----------
 lib/member/member.h         |  9 +++++++++
 lib/member/rte_member.h     |  9 ---------
 lib/member/rte_member_ht.c  | 14 +++++++-------
 lib/member/rte_member_x86.h |  3 ---
 6 files changed, 18 insertions(+), 30 deletions(-)

-- 
2.51.0

[PATCH 1/3] efd: fix AVX2 support

[Thomas Monjalon]

When switching to Meson build, the compilation check on CC_SUPPORT_AVX2
became obsolete, thus the case EFD_LOOKUP_AVX2 became dead.
The function efd_lookup_internal_avx2() was never called,
and its header include rte_efd_x86.h has been removed later.

AVX2 is assumed to be always supported on x86 with supported compilers,
so the checks for AVX2 are simply removed, and the include is added back.

Fixes: 5b9656b157d3 ("lib: build with meson")
Fixes: 30a1de105a5f ("lib: remove unneeded header includes")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/efd/rte_efd.c     |  3 ++-
 lib/efd/rte_efd_x86.h | 10 ----------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/lib/efd/rte_efd.c b/lib/efd/rte_efd.c
index b0e44e5c51..ebf1e0655f 100644
--- a/lib/efd/rte_efd.c
+++ b/lib/efd/rte_efd.c
@@ -26,6 +26,7 @@
 
 #include "rte_efd.h"
 #if defined(RTE_ARCH_X86)
+#include "rte_efd_x86.h"
 #elif defined(RTE_ARCH_ARM64)
 #include "rte_efd_arm64.h"
 #endif
@@ -1279,7 +1280,7 @@ efd_lookup_internal(const struct efd_online_group_entry * const group,
 
 	switch (lookup_fn) {
 
-#if defined(RTE_ARCH_X86) && defined(CC_SUPPORT_AVX2)
+#if defined(RTE_ARCH_X86)
 	case EFD_LOOKUP_AVX2:
 		return efd_lookup_internal_avx2(group->hash_idx,
 					group->lookup_table,
diff --git a/lib/efd/rte_efd_x86.h b/lib/efd/rte_efd_x86.h
index e2f9dcca88..f7776db8a5 100644
--- a/lib/efd/rte_efd_x86.h
+++ b/lib/efd/rte_efd_x86.h
@@ -19,7 +19,6 @@ efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
 		const efd_lookuptbl_t *group_lookup_table,
 		const uint32_t hash_val_a, const uint32_t hash_val_b)
 {
-#ifdef __AVX2__
 	efd_value_t value = 0;
 	uint32_t i = 0;
 	__m256i vhash_val_a = _mm256_set1_epi32(hash_val_a);
@@ -45,13 +44,4 @@ efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
 	}
 
 	return value;
-#else
-	RTE_SET_USED(group_hash_idx);
-	RTE_SET_USED(group_lookup_table);
-	RTE_SET_USED(hash_val_a);
-	RTE_SET_USED(hash_val_b);
-	/* Return dummy value, only to avoid compilation breakage */
-	return 0;
-#endif
-
 }
-- 
2.51.0

[PATCH 2/3] member: remove AVX2 build-time checks

[Thomas Monjalon]

Since all supported compilers can generate AVX2 code,
no need to check for AVX2 support when x86 arch is already checked.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/member/rte_member_ht.c  | 14 +++++++-------
 lib/member/rte_member_x86.h |  3 ---
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/member/rte_member_ht.c b/lib/member/rte_member_ht.c
index 738471b378..0a5b206778 100644
--- a/lib/member/rte_member_ht.c
+++ b/lib/member/rte_member_ht.c
@@ -13,7 +13,7 @@
 #include "rte_member.h"
 #include "rte_member_ht.h"
 
-#if defined(RTE_ARCH_X86)
+#ifdef RTE_ARCH_X86
 #include "rte_member_x86.h"
 #endif
 
@@ -113,7 +113,7 @@ rte_member_create_ht(struct rte_member_setsum *ss,
 		for (j = 0; j < RTE_MEMBER_BUCKET_ENTRIES; j++)
 			buckets[i].sets[j] = RTE_MEMBER_NO_MATCH;
 	}
-#if defined(RTE_ARCH_X86)
+#ifdef RTE_ARCH_X86
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2) &&
 			RTE_MEMBER_BUCKET_ENTRIES == 16 &&
 			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_256)
@@ -179,7 +179,7 @@ rte_member_lookup_ht(const struct rte_member_setsum *ss,
 	get_buckets_index(ss, key, &prim_bucket, &sec_bucket, &tmp_sig);
 
 	switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 	case RTE_MEMBER_COMPARE_AVX2:
 		if (search_bucket_single_avx(prim_bucket, tmp_sig, buckets,
 				set_id) ||
@@ -219,7 +219,7 @@ rte_member_lookup_bulk_ht(const struct rte_member_setsum *ss,
 
 	for (i = 0; i < num_keys; i++) {
 		switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 		case RTE_MEMBER_COMPARE_AVX2:
 			if (search_bucket_single_avx(prim_buckets[i],
 					tmp_sig[i], buckets, &set_id[i]) ||
@@ -256,7 +256,7 @@ rte_member_lookup_multi_ht(const struct rte_member_setsum *ss,
 	get_buckets_index(ss, key, &prim_bucket, &sec_bucket, &tmp_sig);
 
 	switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 	case RTE_MEMBER_COMPARE_AVX2:
 		search_bucket_multi_avx(prim_bucket, tmp_sig, buckets,
 			&num_matches, match_per_key, set_id);
@@ -299,7 +299,7 @@ rte_member_lookup_multi_bulk_ht(const struct rte_member_setsum *ss,
 		match_cnt_tmp = 0;
 
 		switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 		case RTE_MEMBER_COMPARE_AVX2:
 			search_bucket_multi_avx(prim_buckets[i], tmp_sig[i],
 				buckets, &match_cnt_tmp, match_per_key,
@@ -360,7 +360,7 @@ try_update(struct member_ht_bucket *buckets, uint32_t prim, uint32_t sec,
 		enum rte_member_sig_compare_function cmp_fn)
 {
 	switch (cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 	case RTE_MEMBER_COMPARE_AVX2:
 		if (update_entry_search_avx(prim, sig, buckets, set_id) ||
 				update_entry_search_avx(sec, sig, buckets,
diff --git a/lib/member/rte_member_x86.h b/lib/member/rte_member_x86.h
index 4de453485b..29cc2f0132 100644
--- a/lib/member/rte_member_x86.h
+++ b/lib/member/rte_member_x86.h
@@ -11,8 +11,6 @@
 extern "C" {
 #endif
 
-#if defined(__AVX2__)
-
 static inline int
 update_entry_search_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 		struct member_ht_bucket *buckets,
@@ -69,7 +67,6 @@ search_bucket_multi_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 		hitmask &= ~(3U << ((hit_idx) << 1));
 	}
 }
-#endif
 
 #ifdef __cplusplus
 }
-- 
2.51.0

[PATCH 3/3] member: hide internal macro

[Thomas Monjalon]

The hash function used by the library is not supposed
to be exposed and be part of the API.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/member/member.h     | 9 +++++++++
 lib/member/rte_member.h | 9 ---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/member/member.h b/lib/member/member.h
index 609b326a8a..96003f7543 100644
--- a/lib/member/member.h
+++ b/lib/member/member.h
@@ -10,3 +10,12 @@ extern int librte_member_logtype;
 #define MEMBER_LOG(level, ...) \
 	RTE_LOG_LINE_PREFIX(level, MEMBER, \
 		"%s(): ", __func__, __VA_ARGS__)
+
+/* Hash function used by membership library. */
+#if defined(RTE_ARCH_X86) || defined(__ARM_FEATURE_CRC32)
+#include <rte_hash_crc.h>
+#define MEMBER_HASH_FUNC       rte_hash_crc
+#else
+#include <rte_jhash.h>
+#define MEMBER_HASH_FUNC       rte_jhash
+#endif
diff --git a/lib/member/rte_member.h b/lib/member/rte_member.h
index 0235bb0a81..6d9740e0f1 100644
--- a/lib/member/rte_member.h
+++ b/lib/member/rte_member.h
@@ -87,15 +87,6 @@ typedef uint16_t member_set_t;
 /** For sketch, use the flag if to count packet size instead of packet count */
 #define RTE_MEMBER_SKETCH_COUNT_BYTE 0x02
 
-/** @internal Hash function used by membership library. */
-#if defined(RTE_ARCH_X86) || defined(__ARM_FEATURE_CRC32)
-#include <rte_hash_crc.h>
-#define MEMBER_HASH_FUNC       rte_hash_crc
-#else
-#include <rte_jhash.h>
-#define MEMBER_HASH_FUNC       rte_jhash
-#endif
-
 #ifdef __cplusplus
 extern "C" {
 #endif
-- 
2.51.0

Re: [PATCH 1/3] efd: fix AVX2 support

[Bruce Richardson]

On Thu, Sep 18, 2025 at 09:28:03AM +0200, Thomas Monjalon wrote:
> When switching to Meson build, the compilation check on CC_SUPPORT_AVX2
> became obsolete, thus the case EFD_LOOKUP_AVX2 became dead.
> The function efd_lookup_internal_avx2() was never called,
> and its header include rte_efd_x86.h has been removed later.
> 
> AVX2 is assumed to be always supported on x86 with supported compilers,
> so the checks for AVX2 are simply removed, and the include is added back.
> 
> Fixes: 5b9656b157d3 ("lib: build with meson")
> Fixes: 30a1de105a5f ("lib: remove unneeded header includes")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Some comments inline below. Not sure all these removals are entirely
correct - though it may be that the original code has issues itself, rather
than just this patch.

/Bruce

> ---
>  lib/efd/rte_efd.c     |  3 ++-
>  lib/efd/rte_efd_x86.h | 10 ----------
>  2 files changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/efd/rte_efd.c b/lib/efd/rte_efd.c
> index b0e44e5c51..ebf1e0655f 100644
> --- a/lib/efd/rte_efd.c
> +++ b/lib/efd/rte_efd.c
> @@ -26,6 +26,7 @@
>  
>  #include "rte_efd.h"
>  #if defined(RTE_ARCH_X86)
> +#include "rte_efd_x86.h"
>  #elif defined(RTE_ARCH_ARM64)
>  #include "rte_efd_arm64.h"
>  #endif
> @@ -1279,7 +1280,7 @@ efd_lookup_internal(const struct efd_online_group_entry * const group,
>  
>  	switch (lookup_fn) {
>  
> -#if defined(RTE_ARCH_X86) && defined(CC_SUPPORT_AVX2)
> +#if defined(RTE_ARCH_X86)

Ok to remove this, because indeed all supported compilers have AVX2.

However, given that the efd meson.build file doesn't check for compiler
support and optionally build some extra files with the AVX2 flags, I wonder
if this define should actually be changed to an __AVX2__ one, to detect if
the build has AVX2 support rather than just the compiler.

>  	case EFD_LOOKUP_AVX2:
>  		return efd_lookup_internal_avx2(group->hash_idx,
>  					group->lookup_table,
> diff --git a/lib/efd/rte_efd_x86.h b/lib/efd/rte_efd_x86.h
> index e2f9dcca88..f7776db8a5 100644
> --- a/lib/efd/rte_efd_x86.h
> +++ b/lib/efd/rte_efd_x86.h
> @@ -19,7 +19,6 @@ efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
>  		const efd_lookuptbl_t *group_lookup_table,
>  		const uint32_t hash_val_a, const uint32_t hash_val_b)
>  {
> -#ifdef __AVX2__

This may not be safe to remove though, because AVX2 support may not
actually be present in the build. For example, when doing a default build
with -march=corei7, __AVX2__ will not be defined, because the target CPU
doesn't support it, even though the compiler does.

>  	efd_value_t value = 0;
>  	uint32_t i = 0;
>  	__m256i vhash_val_a = _mm256_set1_epi32(hash_val_a);
> @@ -45,13 +44,4 @@ efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
>  	}
>  
>  	return value;
> -#else
> -	RTE_SET_USED(group_hash_idx);
> -	RTE_SET_USED(group_lookup_table);
> -	RTE_SET_USED(hash_val_a);
> -	RTE_SET_USED(hash_val_b);
> -	/* Return dummy value, only to avoid compilation breakage */
> -	return 0;
> -#endif
> -
>  }
> -- 
> 2.51.0
> 

Re: [PATCH 2/3] member: remove AVX2 build-time checks

[Bruce Richardson]

On Thu, Sep 18, 2025 at 09:28:04AM +0200, Thomas Monjalon wrote:
> Since all supported compilers can generate AVX2 code,
> no need to check for AVX2 support when x86 arch is already checked.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/member/rte_member_ht.c  | 14 +++++++-------
>  lib/member/rte_member_x86.h |  3 ---
>  2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/lib/member/rte_member_ht.c b/lib/member/rte_member_ht.c
> index 738471b378..0a5b206778 100644
> --- a/lib/member/rte_member_ht.c
> +++ b/lib/member/rte_member_ht.c
> @@ -13,7 +13,7 @@
>  #include "rte_member.h"
>  #include "rte_member_ht.h"
>  
> -#if defined(RTE_ARCH_X86)
> +#ifdef RTE_ARCH_X86
>  #include "rte_member_x86.h"
>  #endif
>  
> @@ -113,7 +113,7 @@ rte_member_create_ht(struct rte_member_setsum *ss,
>  		for (j = 0; j < RTE_MEMBER_BUCKET_ENTRIES; j++)
>  			buckets[i].sets[j] = RTE_MEMBER_NO_MATCH;
>  	}
> -#if defined(RTE_ARCH_X86)
> +#ifdef RTE_ARCH_X86
>  	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2) &&
>  			RTE_MEMBER_BUCKET_ENTRIES == 16 &&
>  			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_256)
> @@ -179,7 +179,7 @@ rte_member_lookup_ht(const struct rte_member_setsum *ss,
>  	get_buckets_index(ss, key, &prim_bucket, &sec_bucket, &tmp_sig);
>  
>  	switch (ss->sig_cmp_fn) {
> -#if defined(RTE_ARCH_X86) && defined(__AVX2__)
> +#ifdef RTE_ARCH_X86
>  	case RTE_MEMBER_COMPARE_AVX2:
>  		if (search_bucket_single_avx(prim_bucket, tmp_sig, buckets,
>  				set_id) ||
> @@ -219,7 +219,7 @@ rte_member_lookup_bulk_ht(const struct rte_member_setsum *ss,
>  
>  	for (i = 0; i < num_keys; i++) {
>  		switch (ss->sig_cmp_fn) {
> -#if defined(RTE_ARCH_X86) && defined(__AVX2__)
> +#ifdef RTE_ARCH_X86

As with previous patch, the AVX2 flag cannot be removed here. Compiler
support does not mean build support.

/Bruce

Re: [PATCH 3/3] member: hide internal macro

[Bruce Richardson]

On Thu, Sep 18, 2025 at 09:28:05AM +0200, Thomas Monjalon wrote:
> The hash function used by the library is not supposed
> to be exposed and be part of the API.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH 0/3] lib: fix AVX2 checks and macro exposure

[Thomas Monjalon]

18/09/2025 09:28, Thomas Monjalon:
> These are fixes for AVX2 in efd and member libraries.
> While at it, I've hidden a macro which was wrongly exported in the API
> without having a correct prefix.
> 
> Thomas Monjalon (3):
>   efd: fix AVX2 support
>   member: remove AVX2 build-time checks
>   member: hide internal macro

The AVX2 changes break the compilation of "x86-generic" with these messages:

lib/member/rte_member_x86.h: In function 'search_bucket_single_avx':
lib/member/rte_member_x86.h:35:28: error: AVX vector return without AVX enabled changes the ABI [-Werror=psabi]
   35 |         uint32_t hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(

lib/efd/rte_efd_x86.h: In function 'efd_lookup_internal_avx2':
lib/efd/rte_efd_x86.h:24:17: error: AVX vector return without AVX enabled changes the ABI [-Werror=psabi]
   24 |         __m256i vhash_val_a = _mm256_set1_epi32(hash_val_a);

AVX2 must be forced on these headers.
The solution is probably to move these functions in .c files
declared as sources_avx2 in meson.build.

Re: [PATCH 1/3] efd: fix AVX2 support

[Thomas Monjalon]

18/09/2025 09:48, Bruce Richardson:
> On Thu, Sep 18, 2025 at 09:28:03AM +0200, Thomas Monjalon wrote:
> > -#if defined(RTE_ARCH_X86) && defined(CC_SUPPORT_AVX2)
> > +#if defined(RTE_ARCH_X86)
> 
> Ok to remove this, because indeed all supported compilers have AVX2.
> 
> However, given that the efd meson.build file doesn't check for compiler
> support and optionally build some extra files with the AVX2 flags, I wonder
> if this define should actually be changed to an __AVX2__ one, to detect if
> the build has AVX2 support rather than just the compiler.

Correct

> > @@ -19,7 +19,6 @@ efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
> >  		const efd_lookuptbl_t *group_lookup_table,
> >  		const uint32_t hash_val_a, const uint32_t hash_val_b)
> >  {
> > -#ifdef __AVX2__
> 
> This may not be safe to remove though, because AVX2 support may not
> actually be present in the build. For example, when doing a default build
> with -march=corei7, __AVX2__ will not be defined, because the target CPU
> doesn't support it, even though the compiler does.

Correct

I was sending you an email to explain I was at this point in this series,
but your review was too fast :)

Because compilers have the support, I suppose we could force AVX2
on this function with the impact of not being inline.
But given it was not used for years, I believe it is OK.

Re: [PATCH 0/3] lib: fix AVX2 checks and macro exposure

[Bruce Richardson]

On Thu, Sep 18, 2025 at 10:10:59AM +0200, Thomas Monjalon wrote:
> 18/09/2025 09:28, Thomas Monjalon:
> > These are fixes for AVX2 in efd and member libraries.
> > While at it, I've hidden a macro which was wrongly exported in the API
> > without having a correct prefix.
> > 
> > Thomas Monjalon (3):
> >   efd: fix AVX2 support
> >   member: remove AVX2 build-time checks
> >   member: hide internal macro
> 
> The AVX2 changes break the compilation of "x86-generic" with these messages:
> 
> lib/member/rte_member_x86.h: In function 'search_bucket_single_avx':
> lib/member/rte_member_x86.h:35:28: error: AVX vector return without AVX enabled changes the ABI [-Werror=psabi]
>    35 |         uint32_t hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
> 
> lib/efd/rte_efd_x86.h: In function 'efd_lookup_internal_avx2':
> lib/efd/rte_efd_x86.h:24:17: error: AVX vector return without AVX enabled changes the ABI [-Werror=psabi]
>    24 |         __m256i vhash_val_a = _mm256_set1_epi32(hash_val_a);
> 
> AVX2 must be forced on these headers.
> The solution is probably to move these functions in .c files
> declared as sources_avx2 in meson.build.
>
Yes, this is probably the best approach 

[PATCH v2 0/4] lib: fix AVX2 checks and macro exposure

[Thomas Monjalon]

I've found the AVX2 function is not used in efd library.
Then I tried to make AVX2 compilation simpler in efd and member libraries.

While at it, I've hidden a macro which was wrongly exported in the API
without having a correct prefix.

Thomas Monjalon (4):
  efd: fix AVX2 support
  efd: remove AVX2 build-time check
  member: remove AVX2 build-time checks
  member: hide internal macro

 lib/efd/{rte_efd_x86.h => efd_avx2.c}         | 18 ++---
 lib/efd/meson.build                           |  1 +
 lib/efd/rte_efd.c                             |  3 +-
 lib/efd/rte_efd_x86.h                         | 54 +--------------
 lib/member/member.h                           |  9 +++
 .../{rte_member_x86.h => member_avx2.c}       | 23 ++-----
 lib/member/meson.build                        |  2 +-
 lib/member/rte_member.h                       |  9 ---
 lib/member/rte_member_ht.c                    | 14 ++--
 lib/member/rte_member_x86.h                   | 68 ++-----------------
 10 files changed, 39 insertions(+), 162 deletions(-)
 copy lib/efd/{rte_efd_x86.h => efd_avx2.c} (80%)
 copy lib/member/{rte_member_x86.h => member_avx2.c} (87%)

-- 
2.51.0

[PATCH v2 1/4] efd: fix AVX2 support

[Thomas Monjalon]

When switching to Meson build, the compilation check on CC_SUPPORT_AVX2
became obsolete, thus the case EFD_LOOKUP_AVX2 became dead.
The function efd_lookup_internal_avx2() was never called,
and its header include rte_efd_x86.h has been removed later.

AVX2 is assumed to be always supported on x86 with supported compilers,
so the check for AVX2 is simply removed, and the include is added back.

Fixes: 5b9656b157d3 ("lib: build with meson")
Fixes: 30a1de105a5f ("lib: remove unneeded header includes")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/efd/rte_efd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efd/rte_efd.c b/lib/efd/rte_efd.c
index b0e44e5c51..ebf1e0655f 100644
--- a/lib/efd/rte_efd.c
+++ b/lib/efd/rte_efd.c
@@ -26,6 +26,7 @@
 
 #include "rte_efd.h"
 #if defined(RTE_ARCH_X86)
+#include "rte_efd_x86.h"
 #elif defined(RTE_ARCH_ARM64)
 #include "rte_efd_arm64.h"
 #endif
@@ -1279,7 +1280,7 @@ efd_lookup_internal(const struct efd_online_group_entry * const group,
 
 	switch (lookup_fn) {
 
-#if defined(RTE_ARCH_X86) && defined(CC_SUPPORT_AVX2)
+#if defined(RTE_ARCH_X86)
 	case EFD_LOOKUP_AVX2:
 		return efd_lookup_internal_avx2(group->hash_idx,
 					group->lookup_table,
-- 
2.51.0

[PATCH v2 2/4] efd: remove AVX2 build-time check

[Thomas Monjalon]

Since all supported compilers can generate AVX2 code,
it is possible to force AVX2 compilation on the specific function
and remove the check for AVX2 support.

The function has to be moved in a .c file, losing inlining.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/efd/{rte_efd_x86.h => efd_avx2.c} | 18 ++-------
 lib/efd/meson.build                   |  1 +
 lib/efd/rte_efd_x86.h                 | 54 +--------------------------
 3 files changed, 7 insertions(+), 66 deletions(-)
 copy lib/efd/{rte_efd_x86.h => efd_avx2.c} (80%)

diff --git a/lib/efd/rte_efd_x86.h b/lib/efd/efd_avx2.c
similarity index 80%
copy from lib/efd/rte_efd_x86.h
copy to lib/efd/efd_avx2.c
index e2f9dcca88..653976e96b 100644
--- a/lib/efd/rte_efd_x86.h
+++ b/lib/efd/efd_avx2.c
@@ -2,11 +2,11 @@
  * Copyright(c) 2016-2017 Intel Corporation
  */
 
-/* rte_efd_x86.h
- * This file holds all x86 specific EFD functions
- */
 #include <immintrin.h>
 
+#include "rte_efd.h"
+#include "rte_efd_x86.h"
+
 #if (RTE_EFD_VALUE_NUM_BITS == 8 || RTE_EFD_VALUE_NUM_BITS == 16 || \
 	RTE_EFD_VALUE_NUM_BITS == 24 || RTE_EFD_VALUE_NUM_BITS == 32)
 #define EFD_LOAD_SI128(val) _mm_load_si128(val)
@@ -14,12 +14,11 @@
 #define EFD_LOAD_SI128(val) _mm_lddqu_si128(val)
 #endif
 
-static inline efd_value_t
+efd_value_t
 efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
 		const efd_lookuptbl_t *group_lookup_table,
 		const uint32_t hash_val_a, const uint32_t hash_val_b)
 {
-#ifdef __AVX2__
 	efd_value_t value = 0;
 	uint32_t i = 0;
 	__m256i vhash_val_a = _mm256_set1_epi32(hash_val_a);
@@ -45,13 +44,4 @@ efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
 	}
 
 	return value;
-#else
-	RTE_SET_USED(group_hash_idx);
-	RTE_SET_USED(group_lookup_table);
-	RTE_SET_USED(hash_val_a);
-	RTE_SET_USED(hash_val_b);
-	/* Return dummy value, only to avoid compilation breakage */
-	return 0;
-#endif
-
 }
diff --git a/lib/efd/meson.build b/lib/efd/meson.build
index 343f14e1f3..e5459f566d 100644
--- a/lib/efd/meson.build
+++ b/lib/efd/meson.build
@@ -8,5 +8,6 @@ if is_windows
 endif
 
 sources = files('rte_efd.c')
+sources_avx2 += files('efd_avx2.c')
 headers = files('rte_efd.h')
 deps += ['ring', 'hash']
diff --git a/lib/efd/rte_efd_x86.h b/lib/efd/rte_efd_x86.h
index e2f9dcca88..c67bc716e6 100644
--- a/lib/efd/rte_efd_x86.h
+++ b/lib/efd/rte_efd_x86.h
@@ -2,56 +2,6 @@
  * Copyright(c) 2016-2017 Intel Corporation
  */
 
-/* rte_efd_x86.h
- * This file holds all x86 specific EFD functions
- */
-#include <immintrin.h>
-
-#if (RTE_EFD_VALUE_NUM_BITS == 8 || RTE_EFD_VALUE_NUM_BITS == 16 || \
-	RTE_EFD_VALUE_NUM_BITS == 24 || RTE_EFD_VALUE_NUM_BITS == 32)
-#define EFD_LOAD_SI128(val) _mm_load_si128(val)
-#else
-#define EFD_LOAD_SI128(val) _mm_lddqu_si128(val)
-#endif
-
-static inline efd_value_t
-efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
+efd_value_t efd_lookup_internal_avx2(const efd_hashfunc_t *group_hash_idx,
 		const efd_lookuptbl_t *group_lookup_table,
-		const uint32_t hash_val_a, const uint32_t hash_val_b)
-{
-#ifdef __AVX2__
-	efd_value_t value = 0;
-	uint32_t i = 0;
-	__m256i vhash_val_a = _mm256_set1_epi32(hash_val_a);
-	__m256i vhash_val_b = _mm256_set1_epi32(hash_val_b);
-
-	for (; i < RTE_EFD_VALUE_NUM_BITS; i += 8) {
-		__m256i vhash_idx =
-				_mm256_cvtepu16_epi32(EFD_LOAD_SI128(
-				(__m128i const *) &group_hash_idx[i]));
-		__m256i vlookup_table = _mm256_cvtepu16_epi32(
-				EFD_LOAD_SI128((__m128i const *)
-				&group_lookup_table[i]));
-		__m256i vhash = _mm256_add_epi32(vhash_val_a,
-				_mm256_mullo_epi32(vhash_idx, vhash_val_b));
-		__m256i vbucket_idx = _mm256_srli_epi32(vhash,
-				EFD_LOOKUPTBL_SHIFT);
-		__m256i vresult = _mm256_srlv_epi32(vlookup_table,
-				vbucket_idx);
-
-		value |= (_mm256_movemask_ps(
-			(__m256) _mm256_slli_epi32(vresult, 31))
-			& ((1 << (RTE_EFD_VALUE_NUM_BITS - i)) - 1)) << i;
-	}
-
-	return value;
-#else
-	RTE_SET_USED(group_hash_idx);
-	RTE_SET_USED(group_lookup_table);
-	RTE_SET_USED(hash_val_a);
-	RTE_SET_USED(hash_val_b);
-	/* Return dummy value, only to avoid compilation breakage */
-	return 0;
-#endif
-
-}
+		const uint32_t hash_val_a, const uint32_t hash_val_b);
-- 
2.51.0

[PATCH v2 3/4] member: remove AVX2 build-time checks

[Thomas Monjalon]

Since all supported compilers can generate AVX2 code,
it is possible to force AVX2 compilation on some specific functions
and remove the checks for AVX2 support when x86 arch is already checked.

The functions have to be moved in a .c file, losing inlining.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 .../{rte_member_x86.h => member_avx2.c}       | 23 ++-----
 lib/member/meson.build                        |  2 +-
 lib/member/rte_member_ht.c                    | 14 ++--
 lib/member/rte_member_x86.h                   | 68 ++-----------------
 4 files changed, 21 insertions(+), 86 deletions(-)
 copy lib/member/{rte_member_x86.h => member_avx2.c} (87%)

diff --git a/lib/member/rte_member_x86.h b/lib/member/member_avx2.c
similarity index 87%
copy from lib/member/rte_member_x86.h
copy to lib/member/member_avx2.c
index 4de453485b..4c909d5b48 100644
--- a/lib/member/rte_member_x86.h
+++ b/lib/member/member_avx2.c
@@ -2,18 +2,14 @@
  * Copyright(c) 2017 Intel Corporation
  */
 
-#ifndef _RTE_MEMBER_X86_H_
-#define _RTE_MEMBER_X86_H_
-
 #include <x86intrin.h>
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+#include <rte_bitops.h>
 
-#if defined(__AVX2__)
+#include "rte_member.h"
+#include "rte_member_x86.h"
 
-static inline int
+int
 update_entry_search_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 		struct member_ht_bucket *buckets,
 		member_set_t set_id)
@@ -29,7 +25,7 @@ update_entry_search_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 	return 0;
 }
 
-static inline int
+int
 search_bucket_single_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 		struct member_ht_bucket *buckets,
 		member_set_t *set_id)
@@ -48,7 +44,7 @@ search_bucket_single_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 	return 0;
 }
 
-static inline void
+void
 search_bucket_multi_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 				struct member_ht_bucket *buckets,
 				uint32_t *counter,
@@ -69,10 +65,3 @@ search_bucket_multi_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 		hitmask &= ~(3U << ((hit_idx) << 1));
 	}
 }
-#endif
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif /* _RTE_MEMBER_X86_H_ */
diff --git a/lib/member/meson.build b/lib/member/meson.build
index 07f9afaed9..fc08b70019 100644
--- a/lib/member/meson.build
+++ b/lib/member/meson.build
@@ -20,7 +20,7 @@ sources = files(
 
 deps += ['hash', 'ring']
 
-# compile AVX512 version if we have avx512 on MSVC or the 'ifma' flag on GCC/Clang
+sources_avx2 += files('member_avx2.c')
 if dpdk_conf.has('RTE_ARCH_X86_64')
     if is_ms_compiler
         sources_avx512 += files('rte_member_sketch_avx512.c')
diff --git a/lib/member/rte_member_ht.c b/lib/member/rte_member_ht.c
index 738471b378..0a5b206778 100644
--- a/lib/member/rte_member_ht.c
+++ b/lib/member/rte_member_ht.c
@@ -13,7 +13,7 @@
 #include "rte_member.h"
 #include "rte_member_ht.h"
 
-#if defined(RTE_ARCH_X86)
+#ifdef RTE_ARCH_X86
 #include "rte_member_x86.h"
 #endif
 
@@ -113,7 +113,7 @@ rte_member_create_ht(struct rte_member_setsum *ss,
 		for (j = 0; j < RTE_MEMBER_BUCKET_ENTRIES; j++)
 			buckets[i].sets[j] = RTE_MEMBER_NO_MATCH;
 	}
-#if defined(RTE_ARCH_X86)
+#ifdef RTE_ARCH_X86
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2) &&
 			RTE_MEMBER_BUCKET_ENTRIES == 16 &&
 			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_256)
@@ -179,7 +179,7 @@ rte_member_lookup_ht(const struct rte_member_setsum *ss,
 	get_buckets_index(ss, key, &prim_bucket, &sec_bucket, &tmp_sig);
 
 	switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 	case RTE_MEMBER_COMPARE_AVX2:
 		if (search_bucket_single_avx(prim_bucket, tmp_sig, buckets,
 				set_id) ||
@@ -219,7 +219,7 @@ rte_member_lookup_bulk_ht(const struct rte_member_setsum *ss,
 
 	for (i = 0; i < num_keys; i++) {
 		switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 		case RTE_MEMBER_COMPARE_AVX2:
 			if (search_bucket_single_avx(prim_buckets[i],
 					tmp_sig[i], buckets, &set_id[i]) ||
@@ -256,7 +256,7 @@ rte_member_lookup_multi_ht(const struct rte_member_setsum *ss,
 	get_buckets_index(ss, key, &prim_bucket, &sec_bucket, &tmp_sig);
 
 	switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 	case RTE_MEMBER_COMPARE_AVX2:
 		search_bucket_multi_avx(prim_bucket, tmp_sig, buckets,
 			&num_matches, match_per_key, set_id);
@@ -299,7 +299,7 @@ rte_member_lookup_multi_bulk_ht(const struct rte_member_setsum *ss,
 		match_cnt_tmp = 0;
 
 		switch (ss->sig_cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 		case RTE_MEMBER_COMPARE_AVX2:
 			search_bucket_multi_avx(prim_buckets[i], tmp_sig[i],
 				buckets, &match_cnt_tmp, match_per_key,
@@ -360,7 +360,7 @@ try_update(struct member_ht_bucket *buckets, uint32_t prim, uint32_t sec,
 		enum rte_member_sig_compare_function cmp_fn)
 {
 	switch (cmp_fn) {
-#if defined(RTE_ARCH_X86) && defined(__AVX2__)
+#ifdef RTE_ARCH_X86
 	case RTE_MEMBER_COMPARE_AVX2:
 		if (update_entry_search_avx(prim, sig, buckets, set_id) ||
 				update_entry_search_avx(sec, sig, buckets,
diff --git a/lib/member/rte_member_x86.h b/lib/member/rte_member_x86.h
index 4de453485b..dea2a9f7aa 100644
--- a/lib/member/rte_member_x86.h
+++ b/lib/member/rte_member_x86.h
@@ -5,74 +5,20 @@
 #ifndef _RTE_MEMBER_X86_H_
 #define _RTE_MEMBER_X86_H_
 
-#include <x86intrin.h>
+#include "rte_member_ht.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-#if defined(__AVX2__)
-
-static inline int
-update_entry_search_avx(uint32_t bucket_id, member_sig_t tmp_sig,
+int update_entry_search_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 		struct member_ht_bucket *buckets,
-		member_set_t set_id)
-{
-	uint32_t hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
-		_mm256_load_si256((__m256i const *)buckets[bucket_id].sigs),
-		_mm256_set1_epi16(tmp_sig)));
-	if (hitmask) {
-		uint32_t hit_idx = rte_ctz32(hitmask) >> 1;
-		buckets[bucket_id].sets[hit_idx] = set_id;
-		return 1;
-	}
-	return 0;
-}
+		member_set_t set_id);
 
-static inline int
-search_bucket_single_avx(uint32_t bucket_id, member_sig_t tmp_sig,
+int search_bucket_single_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 		struct member_ht_bucket *buckets,
-		member_set_t *set_id)
-{
-	uint32_t hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
-		_mm256_load_si256((__m256i const *)buckets[bucket_id].sigs),
-		_mm256_set1_epi16(tmp_sig)));
-	while (hitmask) {
-		uint32_t hit_idx = rte_ctz32(hitmask) >> 1;
-		if (buckets[bucket_id].sets[hit_idx] != RTE_MEMBER_NO_MATCH) {
-			*set_id = buckets[bucket_id].sets[hit_idx];
-			return 1;
-		}
-		hitmask &= ~(3U << ((hit_idx) << 1));
-	}
-	return 0;
-}
+		member_set_t *set_id);
 
-static inline void
-search_bucket_multi_avx(uint32_t bucket_id, member_sig_t tmp_sig,
+void search_bucket_multi_avx(uint32_t bucket_id, member_sig_t tmp_sig,
 				struct member_ht_bucket *buckets,
 				uint32_t *counter,
 				uint32_t match_per_key,
-				member_set_t *set_id)
-{
-	uint32_t hitmask = _mm256_movemask_epi8((__m256i)_mm256_cmpeq_epi16(
-		_mm256_load_si256((__m256i const *)buckets[bucket_id].sigs),
-		_mm256_set1_epi16(tmp_sig)));
-	while (hitmask) {
-		uint32_t hit_idx = rte_ctz32(hitmask) >> 1;
-		if (buckets[bucket_id].sets[hit_idx] != RTE_MEMBER_NO_MATCH) {
-			set_id[*counter] = buckets[bucket_id].sets[hit_idx];
-			(*counter)++;
-			if (*counter >= match_per_key)
-				return;
-		}
-		hitmask &= ~(3U << ((hit_idx) << 1));
-	}
-}
-#endif
-
-#ifdef __cplusplus
-}
-#endif
+				member_set_t *set_id);
 
 #endif /* _RTE_MEMBER_X86_H_ */
-- 
2.51.0

[PATCH v2 4/4] member: hide internal macro

[Thomas Monjalon]

The hash function used by the library is not supposed
to be exposed and be part of the API.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/member/member.h     | 9 +++++++++
 lib/member/rte_member.h | 9 ---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/member/member.h b/lib/member/member.h
index 609b326a8a..96003f7543 100644
--- a/lib/member/member.h
+++ b/lib/member/member.h
@@ -10,3 +10,12 @@ extern int librte_member_logtype;
 #define MEMBER_LOG(level, ...) \
 	RTE_LOG_LINE_PREFIX(level, MEMBER, \
 		"%s(): ", __func__, __VA_ARGS__)
+
+/* Hash function used by membership library. */
+#if defined(RTE_ARCH_X86) || defined(__ARM_FEATURE_CRC32)
+#include <rte_hash_crc.h>
+#define MEMBER_HASH_FUNC       rte_hash_crc
+#else
+#include <rte_jhash.h>
+#define MEMBER_HASH_FUNC       rte_jhash
+#endif
diff --git a/lib/member/rte_member.h b/lib/member/rte_member.h
index 0235bb0a81..6d9740e0f1 100644
--- a/lib/member/rte_member.h
+++ b/lib/member/rte_member.h
@@ -87,15 +87,6 @@ typedef uint16_t member_set_t;
 /** For sketch, use the flag if to count packet size instead of packet count */
 #define RTE_MEMBER_SKETCH_COUNT_BYTE 0x02
 
-/** @internal Hash function used by membership library. */
-#if defined(RTE_ARCH_X86) || defined(__ARM_FEATURE_CRC32)
-#include <rte_hash_crc.h>
-#define MEMBER_HASH_FUNC       rte_hash_crc
-#else
-#include <rte_jhash.h>
-#define MEMBER_HASH_FUNC       rte_jhash
-#endif
-
 #ifdef __cplusplus
 extern "C" {
 #endif
-- 
2.51.0

Re: [PATCH v2 1/4] efd: fix AVX2 support

[Thomas Monjalon]

18/09/2025 11:08, Thomas Monjalon:
> When switching to Meson build, the compilation check on CC_SUPPORT_AVX2
> became obsolete, thus the case EFD_LOOKUP_AVX2 became dead.
> The function efd_lookup_internal_avx2() was never called,
> and its header include rte_efd_x86.h has been removed later.
> 
> AVX2 is assumed to be always supported on x86 with supported compilers,
> so the check for AVX2 is simply removed, and the include is added back.
[...]
> -#if defined(RTE_ARCH_X86) && defined(CC_SUPPORT_AVX2)
> +#if defined(RTE_ARCH_X86)
>  	case EFD_LOOKUP_AVX2:
>  		return efd_lookup_internal_avx2(group->hash_idx,
>  					group->lookup_table,

I've forgotten to say that there is a runtime check:

    if (RTE_EFD_VALUE_NUM_BITS > 3
            && rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2)
            && rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_256)
        table->lookup_fn = EFD_LOOKUP_AVX2;

Re: [PATCH v2 1/4] efd: fix AVX2 support

[Thomas Monjalon]

18/09/2025 11:40, Thomas Monjalon:
> 18/09/2025 11:08, Thomas Monjalon:
> > When switching to Meson build, the compilation check on CC_SUPPORT_AVX2
> > became obsolete, thus the case EFD_LOOKUP_AVX2 became dead.
> > The function efd_lookup_internal_avx2() was never called,
> > and its header include rte_efd_x86.h has been removed later.
> > 
> > AVX2 is assumed to be always supported on x86 with supported compilers,
> > so the check for AVX2 is simply removed, and the include is added back.
> [...]
> > -#if defined(RTE_ARCH_X86) && defined(CC_SUPPORT_AVX2)
> > +#if defined(RTE_ARCH_X86)
> >  	case EFD_LOOKUP_AVX2:
> >  		return efd_lookup_internal_avx2(group->hash_idx,
> >  					group->lookup_table,
> 
> I've forgotten to say that there is a runtime check:
> 
>     if (RTE_EFD_VALUE_NUM_BITS > 3
>             && rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2)
>             && rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_256)
>         table->lookup_fn = EFD_LOOKUP_AVX2;

So the end of the commit log must be changed with this explanation:

  EFD_LOOKUP_AVX2 is chosen at runtime after checking AVX2 availability,
  so the obsolete build-time check for AVX2 can be simply removed,
  and the missing include added back.