DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org, stable@dpdk.org,
	Byron Marohn <byron.marohn@intel.com>,
	Yipeng Wang <yipeng1.wang@intel.com>,
	Luca Boccassi <luca.boccassi@microsoft.com>
Subject: Re: [PATCH 1/3] efd: fix AVX2 support
Date: Thu, 18 Sep 2025 10:16:44 +0200	[thread overview]
Message-ID: <4582603.R56niFO833@thomas> (raw)
In-Reply-To: <aMu5MKGFjrrffCnv@bricha3-mobl1.ger.corp.intel.com>

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	other threads:[~2025-09-18  8:16 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-18  7:28 [PATCH 0/3] lib: fix AVX2 checks and macro exposure Thomas Monjalon
2025-09-18  7:28 ` [PATCH 1/3] efd: fix AVX2 support Thomas Monjalon
2025-09-18  7:48   ` Bruce Richardson
2025-09-18  8:16     ` Thomas Monjalon [this message]
2025-09-18  7:28 ` [PATCH 2/3] member: remove AVX2 build-time checks Thomas Monjalon
2025-09-18  7:49   ` Bruce Richardson
2025-09-18  7:28 ` [PATCH 3/3] member: hide internal macro Thomas Monjalon
2025-09-18  7:50   ` Bruce Richardson
2025-09-18  8:10 ` [PATCH 0/3] lib: fix AVX2 checks and macro exposure Thomas Monjalon
2025-09-18  8:59   ` Bruce Richardson
2025-09-18  9:08 ` [PATCH v2 0/4] " Thomas Monjalon
2025-09-18  9:08   ` [PATCH v2 1/4] efd: fix AVX2 support Thomas Monjalon
2025-09-18  9:40     ` Thomas Monjalon
2025-09-18  9:47       ` Thomas Monjalon
2025-09-18  9:08   ` [PATCH v2 2/4] efd: remove AVX2 build-time check Thomas Monjalon
2025-09-18  9:08   ` [PATCH v2 3/4] member: remove AVX2 build-time checks Thomas Monjalon
2025-09-18  9:08   ` [PATCH v2 4/4] member: hide internal macro Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4582603.R56niFO833@thomas \
    --to=thomas@monjalon.net \
    --cc=bruce.richardson@intel.com \
    --cc=byron.marohn@intel.com \
    --cc=dev@dpdk.org \
    --cc=luca.boccassi@microsoft.com \
    --cc=stable@dpdk.org \
    --cc=yipeng1.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).