From: "Sekhar, Ashwin" <Ashwin.Sekhar@cavium.com>
To: "Jacob, Jerin" <Jerin.JacobKollanukkaran@cavium.com>,
"jianbo.liu@linaro.org" <jianbo.liu@linaro.org>
Cc: "byron.marohn@intel.com" <byron.marohn@intel.com>,
"pablo.de.lara.guarch@intel.com" <pablo.de.lara.guarch@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v3] efd: support lookup using neon intrinsics
Date: Tue, 2 May 2017 08:04:57 +0000 [thread overview]
Message-ID: <1493712296.3602.3.camel@caviumnetworks.com> (raw)
In-Reply-To: <CAP4Qi3-+S5z8ORavtM-TRE1NZ+g2yxtvcH9aR-L__e=pSBs9TQ@mail.gmail.com>
On Tue, 2017-05-02 at 15:59 +0800, Jianbo Liu wrote:
> On 2 May 2017 at 14:41, Jerin Jacob <jerin.jacob@caviumnetworks.com>
> wrote:
> >
> > -----Original Message-----
> > >
> > > Date: Mon, 1 May 2017 22:59:53 -0700
> > > From: Ashwin Sekhar T K <ashwin.sekhar@caviumnetworks.com>
> > > To: byron.marohn@intel.com, pablo.de.lara.guarch@intel.com,
> > > jerin.jacob@caviumnetworks.com, jianbo.liu@linaro.org
> > > Cc: dev@dpdk.org, Ashwin Sekhar T K <ashwin.sekhar@caviumnetworks
> > > .com>
> > > Subject: [dpdk-dev] [PATCH v3] efd: support lookup using neon
> > > intrinsics
> > > X-Mailer: git-send-email 2.13.0.rc1
> > >
> > > * Added file lib/librte_efd/rte_efd_arm64.h to hold arm64
> > > specific definitions
> > > * Verified the changes with efd_autotest unit test case
> > >
> > > Signed-off-by: Ashwin Sekhar T K <ashwin.sekhar@caviumnetworks.co
> > > m>
> > > ---
> > > v2:
> > > * Slightly modified the content of the commit message body
> > > * Added prefix [dpdk-dev] to the email subject line
> > >
> > > v3:
> > > * Moved enum 'EFD_LOOKUP_NEON' under '#if
> > > defined(RTE_ARCH_ARM64)'
> > >
> > > MAINTAINERS | 1 +
> > > lib/librte_efd/rte_efd.c | 24 +++++++++++++
> > > lib/librte_efd/rte_efd_arm64.h | 76
> > > ++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 101 insertions(+)
> > > create mode 100644 lib/librte_efd/rte_efd_arm64.h
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index b6495d2..7d708ae 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -147,6 +147,7 @@ F:
> > > lib/librte_eal/common/include/arch/arm/*_64.h
> > > F: lib/librte_acl/acl_run_neon.*
> > > F: lib/librte_lpm/rte_lpm_neon.h
> > > F: lib/librte_hash/rte*_arm64.h
> > > +F: lib/librte_efd/rte*_arm64.h
> > > F: drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
> > > F: drivers/net/i40e/i40e_rxtx_vec_neon.c
> > > F: drivers/net/virtio/virtio_rxtx_simple_neon.c
> > > diff --git a/lib/librte_efd/rte_efd.c b/lib/librte_efd/rte_efd.c
> > > index f601d62..5cc6283 100644
> > > --- a/lib/librte_efd/rte_efd.c
> > > +++ b/lib/librte_efd/rte_efd.c
> > > @@ -53,6 +53,8 @@
> > > #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
> > >
> > > #define EFD_KEY(key_idx, table) (table->keys + ((key_idx) *
> > > table->key_len))
> > > @@ -103,6 +105,9 @@ allocated memory
> > > enum efd_lookup_internal_function {
> > > EFD_LOOKUP_SCALAR = 0,
> > > EFD_LOOKUP_AVX2,
> > > +#if defined(RTE_ARCH_ARM64)
> > > + EFD_LOOKUP_NEON,
> > > +#endif
> > I think, we can remove this ifdef to
> > - Make code looks clean
> > - In future, in some case a new enum value gets added then the
> > value
> > will be different for each build.
> >
> But the enum items are same for each ARCH.
> Besides, the ifdef could be considered as explanation to that enum.
> If
> someone knows nothing about arm/neon, he can ignore it totally after
> see the ifdef.
>
Have added the #if defined on your advice, but in my opinion also its
better not to have "#if defined" for enums. Because the same enum can
take different values for different builds.
For eg: If somebody adds an EFD_LOOKUP_AVX512 after EFD_LOOKUP_NEON
here, it will take value 2 for x86 builds but value 3 for arm64 builds.
> >
> > Any valid point to keep under RTE_ARCH_ARM64?
> >
> > >
> > > EFD_LOOKUP_NUM
> > > };
next prev parent reply other threads:[~2017-05-02 8:05 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-27 11:42 [dpdk-dev] [PATCH] " Ashwin Sekhar T K
2017-04-27 12:44 ` [dpdk-dev] [PATCH v2] " Ashwin Sekhar T K
2017-04-28 10:06 ` Jianbo Liu
2017-04-28 10:38 ` Sekhar, Ashwin
2017-05-02 1:56 ` Jianbo Liu
2017-07-03 13:29 ` Thomas Monjalon
2017-05-02 5:59 ` [dpdk-dev] [PATCH v3] " Ashwin Sekhar T K
2017-05-02 6:41 ` Jerin Jacob
2017-05-02 7:59 ` Jianbo Liu
2017-05-02 8:04 ` Sekhar, Ashwin [this message]
2017-06-29 20:58 ` De Lara Guarch, Pablo
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=1493712296.3602.3.camel@caviumnetworks.com \
--to=ashwin.sekhar@cavium.com \
--cc=Jerin.JacobKollanukkaran@cavium.com \
--cc=byron.marohn@intel.com \
--cc=dev@dpdk.org \
--cc=jianbo.liu@linaro.org \
--cc=pablo.de.lara.guarch@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).