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.


Reply via email to