Patchwork [dpdk-dev,v1] hash: optimize signature compare by using neon intrinsic

login
register
mail settings
Submitter Ruifeng Wang
Date Feb. 11, 2019, 7:30 a.m.
Message ID <20190211073001.37438-1-ruifeng.wang@arm.com>
Download mbox | patch
Permalink /patch/722581/
State New
Headers show

Comments

Ruifeng Wang - Feb. 11, 2019, 7:30 a.m.
Implemented signature compare function based on neon intrinsic.
Hash bulk lookup had 3% - 6% performance gain after optimization.

Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 32 ++++++++++++++++++++++++++++++-
 lib/librte_hash/rte_cuckoo_hash.h |  1 +
 2 files changed, 32 insertions(+), 1 deletion(-)
Jerin Jacob Kollanukkaran - Feb. 11, 2019, 7:49 a.m.
On Mon, 2019-02-11 at 15:30 +0800, Ruifeng Wang wrote:
> 

> -------------------------------------------------------------------

> ---

> Implemented signature compare function based on neon intrinsic.

> Hash bulk lookup had 3% - 6% performance gain after optimization.

> 

> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>

> ---

>  lib/librte_hash/rte_cuckoo_hash.c | 32

> ++++++++++++++++++++++++++++++-

>  lib/librte_hash/rte_cuckoo_hash.h |  1 +

>  2 files changed, 32 insertions(+), 1 deletion(-)

> 

> diff --git a/lib/librte_hash/rte_cuckoo_hash.c

> b/lib/librte_hash/rte_cuckoo_hash.c

> index c01489ba5..5745a254f 100644

> --- a/lib/librte_hash/rte_cuckoo_hash.c

> +++ b/lib/librte_hash/rte_cuckoo_hash.c

> @@ -26,6 +26,9 @@

>  #include <rte_spinlock.h>

>  #include <rte_ring.h>

>  #include <rte_compat.h>

> +#if defined(RTE_ARCH_ARM64)

> +#include <arm_neon.h>

> +#endif



The use of rte_vector.h will remove the need for #if defined...


>  

>  #include "rte_hash.h"

>  #include "rte_cuckoo_hash.h"

> @@ -407,6 +410,10 @@ rte_hash_create(const struct rte_hash_parameters

> *params)

>  	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE2))

>  		h->sig_cmp_fn = RTE_HASH_COMPARE_SSE;

>  	else

> +#elif defined(RTE_ARCH_ARM64)

> +	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))

> +		h->sig_cmp_fn = RTE_HASH_COMPARE_NEON;

> +	else

>  #endif

>  		h->sig_cmp_fn = RTE_HASH_COMPARE_SCALAR;

>  

> @@ -1578,10 +1585,15 @@ compare_signatures(uint32_t

> *prim_hash_matches, uint32_t *sec_hash_matches,

>  			enum rte_hash_sig_compare_function sig_cmp_fn)

>  {

>  	unsigned int i;

> +#ifdef RTE_MACHINE_CPUFLAG_NEON

> +	uint16x8_t vmat, vsig, x;

> +	uint64x2_t x64;

> +	int16x8_t shift = {-15, -13, -11, -9, -7, -5, -3, -1};

> +#endif


Is it possible move down the variable declaration? to avoid the need
for #ifdef here

>  

>  	/* For match mask the first bit of every two bits indicates the

> match */

>  	switch (sig_cmp_fn) {

> -#ifdef RTE_MACHINE_CPUFLAG_SSE2

> +#if defined(RTE_MACHINE_CPUFLAG_SSE2)

>  	case RTE_HASH_COMPARE_SSE:

>  		/* Compare all signatures in the bucket */

>  		*prim_hash_matches = _mm_movemask_epi8(_mm_cmpeq_epi16(

> @@ -1594,6 +1606,24 @@ compare_signatures(uint32_t

> *prim_hash_matches, uint32_t *sec_hash_matches,

>  					(__m128i const *)sec_bkt-

> >sig_current),

>  				_mm_set1_epi16(sig)));

>  		break;

> +#elif defined(RTE_MACHINE_CPUFLAG_NEON)

> +	case RTE_HASH_COMPARE_NEON:

> +		vsig = vld1q_dup_u16((uint16_t const *)&sig);

> +		/* Compare all signatures in the primary bucket */

> +		vmat = vceqq_u16(vsig,

> +			vld1q_u16((uint16_t const *)prim_bkt-

> >sig_current));

> +		x = vshlq_u16(vandq_u16(vmat, vdupq_n_u16(0x8000)),

> shift);

> +		x64 = vpaddlq_u32(vpaddlq_u16(x));

> +		*prim_hash_matches = (uint32_t)(vgetq_lane_u64(x64, 0)

> +

> +			vgetq_lane_u64(x64, 1));

> +		/* Compare all signatures in the secondary bucket */

> +		vmat = vceqq_u16(vsig,

> +			vld1q_u16((uint16_t const *)sec_bkt-

> >sig_current));

> +		x = vshlq_u16(vandq_u16(vmat, vdupq_n_u16(0x8000)),

> shift);

> +		x64 = vpaddlq_u32(vpaddlq_u16(x));

> +		*sec_hash_matches = (uint32_t)(vgetq_lane_u64(x64, 0) +

> +			vgetq_lane_u64(x64, 1));

> +		break;

>  #endif

>  	default:

>  		for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {

> diff --git a/lib/librte_hash/rte_cuckoo_hash.h

> b/lib/librte_hash/rte_cuckoo_hash.h

> index eacdaa8d4..0548c97f0 100644

> --- a/lib/librte_hash/rte_cuckoo_hash.h

> +++ b/lib/librte_hash/rte_cuckoo_hash.h

> @@ -141,6 +141,7 @@ struct rte_hash_key {

>  enum rte_hash_sig_compare_function {

>  	RTE_HASH_COMPARE_SCALAR = 0,

>  	RTE_HASH_COMPARE_SSE,

> +	RTE_HASH_COMPARE_NEON,

>  	RTE_HASH_COMPARE_NUM

>  };

>
Ruifeng Wang - Feb. 11, 2019, 9:47 a.m.
Hi Jerin,

> -----Original Message-----

> From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>

> Sent: Monday, February 11, 2019 15:49

> To: Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>;

> yipeng1.wang@intel.com

> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; nd

> <nd@arm.com>; dev@dpdk.org

> Subject: Re: [EXT] [PATCH v1] hash: optimize signature compare by using

> neon intrinsic

> 

> On Mon, 2019-02-11 at 15:30 +0800, Ruifeng Wang wrote:

> >

> > -------------------------------------------------------------------

> > ---

> > Implemented signature compare function based on neon intrinsic.

> > Hash bulk lookup had 3% - 6% performance gain after optimization.

> >

> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>

> > ---

> >  lib/librte_hash/rte_cuckoo_hash.c | 32

> > ++++++++++++++++++++++++++++++-

> >  lib/librte_hash/rte_cuckoo_hash.h |  1 +

> >  2 files changed, 32 insertions(+), 1 deletion(-)

> >

> > diff --git a/lib/librte_hash/rte_cuckoo_hash.c

> > b/lib/librte_hash/rte_cuckoo_hash.c

> > index c01489ba5..5745a254f 100644

> > --- a/lib/librte_hash/rte_cuckoo_hash.c

> > +++ b/lib/librte_hash/rte_cuckoo_hash.c

> > @@ -26,6 +26,9 @@

> >  #include <rte_spinlock.h>

> >  #include <rte_ring.h>

> >  #include <rte_compat.h>

> > +#if defined(RTE_ARCH_ARM64)

> > +#include <arm_neon.h>

> > +#endif

> 

> 

> The use of rte_vector.h will remove the need for #if defined...

> 


OK. Will change to rte_vect.h in next version.

> 

> >

> >  #include "rte_hash.h"

> >  #include "rte_cuckoo_hash.h"

> > @@ -407,6 +410,10 @@ rte_hash_create(const struct

> rte_hash_parameters

> > *params)

> >  	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE2))

> >  		h->sig_cmp_fn = RTE_HASH_COMPARE_SSE;

> >  	else

> > +#elif defined(RTE_ARCH_ARM64)

> > +	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))

> > +		h->sig_cmp_fn = RTE_HASH_COMPARE_NEON;

> > +	else

> >  #endif

> >  		h->sig_cmp_fn = RTE_HASH_COMPARE_SCALAR;

> >

> > @@ -1578,10 +1585,15 @@ compare_signatures(uint32_t

> > *prim_hash_matches, uint32_t *sec_hash_matches,

> >  			enum rte_hash_sig_compare_function sig_cmp_fn)

> {

> >  	unsigned int i;

> > +#ifdef RTE_MACHINE_CPUFLAG_NEON

> > +	uint16x8_t vmat, vsig, x;

> > +	uint64x2_t x64;

> > +	int16x8_t shift = {-15, -13, -11, -9, -7, -5, -3, -1}; #endif

> 

> Is it possible move down the variable declaration? to avoid the need for

> #ifdef here

> 


Will move down the variable declaration into 'case' block.

> >

> >  	/* For match mask the first bit of every two bits indicates the

> > match */

> >  	switch (sig_cmp_fn) {

> > -#ifdef RTE_MACHINE_CPUFLAG_SSE2

> > +#if defined(RTE_MACHINE_CPUFLAG_SSE2)

> >  	case RTE_HASH_COMPARE_SSE:

> >  		/* Compare all signatures in the bucket */

> >  		*prim_hash_matches =

> _mm_movemask_epi8(_mm_cmpeq_epi16(

> > @@ -1594,6 +1606,24 @@ compare_signatures(uint32_t

> *prim_hash_matches,

> > uint32_t *sec_hash_matches,

> >  					(__m128i const *)sec_bkt-

> > >sig_current),

> >  				_mm_set1_epi16(sig)));

> >  		break;

> > +#elif defined(RTE_MACHINE_CPUFLAG_NEON)

> > +	case RTE_HASH_COMPARE_NEON:

> > +		vsig = vld1q_dup_u16((uint16_t const *)&sig);

> > +		/* Compare all signatures in the primary bucket */

> > +		vmat = vceqq_u16(vsig,

> > +			vld1q_u16((uint16_t const *)prim_bkt-

> > >sig_current));

> > +		x = vshlq_u16(vandq_u16(vmat, vdupq_n_u16(0x8000)),

> > shift);

> > +		x64 = vpaddlq_u32(vpaddlq_u16(x));

> > +		*prim_hash_matches = (uint32_t)(vgetq_lane_u64(x64, 0)

> > +

> > +			vgetq_lane_u64(x64, 1));

> > +		/* Compare all signatures in the secondary bucket */

> > +		vmat = vceqq_u16(vsig,

> > +			vld1q_u16((uint16_t const *)sec_bkt-

> > >sig_current));

> > +		x = vshlq_u16(vandq_u16(vmat, vdupq_n_u16(0x8000)),

> > shift);

> > +		x64 = vpaddlq_u32(vpaddlq_u16(x));

> > +		*sec_hash_matches = (uint32_t)(vgetq_lane_u64(x64, 0) +

> > +			vgetq_lane_u64(x64, 1));

> > +		break;

> >  #endif

> >  	default:

> >  		for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) { diff --git

> > a/lib/librte_hash/rte_cuckoo_hash.h

> > b/lib/librte_hash/rte_cuckoo_hash.h

> > index eacdaa8d4..0548c97f0 100644

> > --- a/lib/librte_hash/rte_cuckoo_hash.h

> > +++ b/lib/librte_hash/rte_cuckoo_hash.h

> > @@ -141,6 +141,7 @@ struct rte_hash_key {  enum

> > rte_hash_sig_compare_function {

> >  	RTE_HASH_COMPARE_SCALAR = 0,

> >  	RTE_HASH_COMPARE_SSE,

> > +	RTE_HASH_COMPARE_NEON,

> >  	RTE_HASH_COMPARE_NUM

> >  };

> >

Patch

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index c01489ba5..5745a254f 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -26,6 +26,9 @@ 
 #include <rte_spinlock.h>
 #include <rte_ring.h>
 #include <rte_compat.h>
+#if defined(RTE_ARCH_ARM64)
+#include <arm_neon.h>
+#endif
 
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
@@ -407,6 +410,10 @@  rte_hash_create(const struct rte_hash_parameters *params)
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE2))
 		h->sig_cmp_fn = RTE_HASH_COMPARE_SSE;
 	else
+#elif defined(RTE_ARCH_ARM64)
+	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))
+		h->sig_cmp_fn = RTE_HASH_COMPARE_NEON;
+	else
 #endif
 		h->sig_cmp_fn = RTE_HASH_COMPARE_SCALAR;
 
@@ -1578,10 +1585,15 @@  compare_signatures(uint32_t *prim_hash_matches, uint32_t *sec_hash_matches,
 			enum rte_hash_sig_compare_function sig_cmp_fn)
 {
 	unsigned int i;
+#ifdef RTE_MACHINE_CPUFLAG_NEON
+	uint16x8_t vmat, vsig, x;
+	uint64x2_t x64;
+	int16x8_t shift = {-15, -13, -11, -9, -7, -5, -3, -1};
+#endif
 
 	/* For match mask the first bit of every two bits indicates the match */
 	switch (sig_cmp_fn) {
-#ifdef RTE_MACHINE_CPUFLAG_SSE2
+#if defined(RTE_MACHINE_CPUFLAG_SSE2)
 	case RTE_HASH_COMPARE_SSE:
 		/* Compare all signatures in the bucket */
 		*prim_hash_matches = _mm_movemask_epi8(_mm_cmpeq_epi16(
@@ -1594,6 +1606,24 @@  compare_signatures(uint32_t *prim_hash_matches, uint32_t *sec_hash_matches,
 					(__m128i const *)sec_bkt->sig_current),
 				_mm_set1_epi16(sig)));
 		break;
+#elif defined(RTE_MACHINE_CPUFLAG_NEON)
+	case RTE_HASH_COMPARE_NEON:
+		vsig = vld1q_dup_u16((uint16_t const *)&sig);
+		/* Compare all signatures in the primary bucket */
+		vmat = vceqq_u16(vsig,
+			vld1q_u16((uint16_t const *)prim_bkt->sig_current));
+		x = vshlq_u16(vandq_u16(vmat, vdupq_n_u16(0x8000)), shift);
+		x64 = vpaddlq_u32(vpaddlq_u16(x));
+		*prim_hash_matches = (uint32_t)(vgetq_lane_u64(x64, 0) +
+			vgetq_lane_u64(x64, 1));
+		/* Compare all signatures in the secondary bucket */
+		vmat = vceqq_u16(vsig,
+			vld1q_u16((uint16_t const *)sec_bkt->sig_current));
+		x = vshlq_u16(vandq_u16(vmat, vdupq_n_u16(0x8000)), shift);
+		x64 = vpaddlq_u32(vpaddlq_u16(x));
+		*sec_hash_matches = (uint32_t)(vgetq_lane_u64(x64, 0) +
+			vgetq_lane_u64(x64, 1));
+		break;
 #endif
 	default:
 		for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h
index eacdaa8d4..0548c97f0 100644
--- a/lib/librte_hash/rte_cuckoo_hash.h
+++ b/lib/librte_hash/rte_cuckoo_hash.h
@@ -141,6 +141,7 @@  struct rte_hash_key {
 enum rte_hash_sig_compare_function {
 	RTE_HASH_COMPARE_SCALAR = 0,
 	RTE_HASH_COMPARE_SSE,
+	RTE_HASH_COMPARE_NEON,
 	RTE_HASH_COMPARE_NUM
 };