> -----Original Message----- > From: David Marchand <david.march...@redhat.com> > Sent: Wednesday, January 27, 2021 9:05 PM > To: Ruifeng Wang <ruifeng.w...@arm.com> > Cc: jer...@marvell.com; Jan Viktorin <vikto...@rehivetech.com>; Bruce > Richardson <bruce.richard...@intel.com>; Vladimir Medvedkin > <vladimir.medved...@intel.com>; dev <dev@dpdk.org>; Pavan Nikhilesh > <pbhagavat...@marvell.com>; hemant.agra...@nxp.com; Honnappa > Nagarahalli <honnappa.nagaraha...@arm.com>; nd <n...@arm.com> > Subject: Re: [dpdk-dev] [PATCH v3 1/5] lpm: add sve support for lookup on > Arm platform > > On Tue, Jan 12, 2021 at 3:57 AM Ruifeng Wang <ruifeng.w...@arm.com> > wrote: > > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index > > 1afe55cdc..28b57683b 100644 > > --- a/lib/librte_lpm/rte_lpm.h > > +++ b/lib/librte_lpm/rte_lpm.h > > @@ -402,7 +402,11 @@ rte_lpm_lookupx4(const struct rte_lpm *lpm, > xmm_t ip, uint32_t hop[4], > > uint32_t defv); > > > > #if defined(RTE_ARCH_ARM) > > +#ifdef __ARM_FEATURE_SVE > > +#include "rte_lpm_sve.h" > > +#else > > #include "rte_lpm_neon.h" > > +#endif > > #elif defined(RTE_ARCH_PPC_64) > > #include "rte_lpm_altivec.h" > > #else > > diff --git a/lib/librte_lpm/rte_lpm_sve.h > > b/lib/librte_lpm/rte_lpm_sve.h new file mode 100644 index > > 000000000..2e319373e > > --- /dev/null > > +++ b/lib/librte_lpm/rte_lpm_sve.h > > @@ -0,0 +1,83 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2020 Arm Limited > > + */ > > + > > +#ifndef _RTE_LPM_SVE_H_ > > +#define _RTE_LPM_SVE_H_ > > + > > +#include <rte_vect.h> > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +__rte_internal > > +static void > > I was looking into use of the __rte_internal tag in the tree. > > This helper is called from a inlined API used by applications, so out of the > DPDK build. > It looks like the compiler is not complaining when compiling examples (I > hacked my env to cross compile with gcc 10 + SVE enabled) but this seems > incorrect to me. > > Is there really a need for this helper? > It is only used below afaics.
My intention was to keep the helper generic. So it can be used not only in rte_lpm_lookupx4 as below, but also in other lookup functions like rte_lpm_lookup_bulk where number of IPs to be looked up is not a fixed value. Will removing __rte_internal tag resolve the issue? > > > > +__rte_lpm_lookup_vec(const struct rte_lpm *lpm, const uint32_t *ips, > > + uint32_t *__rte_restrict next_hops, const uint32_t n) > > +{ > > [snip] > > > > +} > > + > > +static inline void > > +rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip, uint32_t hop[4], > > + uint32_t defv) > > +{ > > + uint32_t i, ips[4]; > > + > > + vst1q_s32((int32_t *)ips, ip); > > + for (i = 0; i < 4; i++) > > + hop[i] = defv; > > + > > + __rte_lpm_lookup_vec(lpm, ips, hop, 4); } > > > -- > David Marchand