diff mbox series

[v2] hash: optimize signature compare by using neon intrinsic

Message ID 20190212070104.33830-1-ruifeng.wang@arm.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] hash: optimize signature compare by using neon intrinsic |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Ruifeng Wang Feb. 12, 2019, 7:01 a.m. UTC 
  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>
Reviewed-by: Gavin Hu <gavin.hu@arm.com>
---
v2:
* Use 'rte_vect.h' instead of 'arm_neon.h'.
* Moved down variable declaration to avoid extra #ifdef.

 lib/librte_hash/rte_cuckoo_hash.c | 29 ++++++++++++++++++++++++++++-
 lib/librte_hash/rte_cuckoo_hash.h |  1 +
 2 files changed, 29 insertions(+), 1 deletion(-)

Comments

Wang, Yipeng1 Feb. 13, 2019, 8:31 p.m. UTC | #1 
Hi, Ruifeng,

I don't have an ARM Platform at hand to test but I gone through the code against a NEON instruction manual, 
and the logic looks good to me. One small comment is the coding style for switch case, I think it is more common
to leave the braces on their own lines.

Otherwise:
Acked-by: Yipeng Wang<yipeng1.wang@intel.com>


>-----Original Message-----
>From: Ruifeng Wang [mailto:ruifeng.wang@arm.com]
>Sent: Monday, February 11, 2019 11:01 PM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; jerinj@marvell.com
>Cc: dev@dpdk.org; honnappa.nagarahalli@arm.com; gavin.hu@arm.com; nd@arm.com; Ruifeng Wang <ruifeng.wang@arm.com>
>Subject: [PATCH v2] hash: optimize signature compare by using neon intrinsic
>
>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>
>Reviewed-by: Gavin Hu <gavin.hu@arm.com>
Ruifeng Wang Feb. 14, 2019, 2:40 a.m. UTC | #2 
Hi Yipeng,

Thanks for your review and acknowledgement.
I did think about braces on separate lines. One concern is, in this way, extra indent will be needed and 
make it hard to keep code lines within 80 characters. So I made the style compromise. 

Regards,
/Ruifeng

> -----Original Message-----
> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> Sent: Thursday, February 14, 2019 4:32
> To: Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>;
> jerinj@marvell.com
> Cc: dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v2] hash: optimize signature compare by using neon
> intrinsic
> 
> Hi, Ruifeng,
> 
> I don't have an ARM Platform at hand to test but I gone through the code
> against a NEON instruction manual, and the logic looks good to me. One small
> comment is the coding style for switch case, I think it is more common to
> leave the braces on their own lines.
> 
> Otherwise:
> Acked-by: Yipeng Wang<yipeng1.wang@intel.com>
> 
> 
> >-----Original Message-----
> >From: Ruifeng Wang [mailto:ruifeng.wang@arm.com]
> >Sent: Monday, February 11, 2019 11:01 PM
> >To: Wang, Yipeng1 <yipeng1.wang@intel.com>; jerinj@marvell.com
> >Cc: dev@dpdk.org; honnappa.nagarahalli@arm.com; gavin.hu@arm.com;
> >nd@arm.com; Ruifeng Wang <ruifeng.wang@arm.com>
> >Subject: [PATCH v2] hash: optimize signature compare by using neon
> >intrinsic
> >
> >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>
> >Reviewed-by: Gavin Hu <gavin.hu@arm.com>
Jerin Jacob Kollanukkaran Feb. 25, 2019, 5:52 p.m. UTC | #3 
On Tue, 2019-02-12 at 15:01 +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>
> Reviewed-by: Gavin Hu <gavin.hu@arm.com>
> ---
> v2:
> * Use 'rte_vect.h' instead of 'arm_neon.h'.
> * Moved down variable declaration to avoid extra #ifdef.


Acked-by: Jerin Jacob <jerinj@marvell.com>
Thomas Monjalon March 28, 2019, 6:49 p.m. UTC | #4 
25/02/2019 18:52, Jerin Jacob Kollanukkaran:
> On Tue, 2019-02-12 at 15:01 +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>
> > Reviewed-by: Gavin Hu <gavin.hu@arm.com>
> 
> Acked-by: Jerin Jacob <jerinj@marvell.com>

Applied, thanks
diff mbox series

Patch

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index c01489ba5..0dddce226 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -26,6 +26,7 @@ 
 #include <rte_spinlock.h>
 #include <rte_ring.h>
 #include <rte_compat.h>
+#include <rte_vect.h>
 
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
@@ -407,6 +408,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;
 
@@ -1581,7 +1586,7 @@  compare_signatures(uint32_t *prim_hash_matches, uint32_t *sec_hash_matches,
 
 	/* 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 +1599,28 @@  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: {
+		uint16x8_t vmat, vsig, x;
+		uint64x2_t x64;
+		int16x8_t shift = {-15, -13, -11, -9, -7, -5, -3, -1};
+
+		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
 };