[v9,4/4] hash: add SVE support for bulk key lookup

Message ID 20240430162743.1525484-5-yoan.picchi@arm.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series hash: add SVE support for bulk key lookup |

Checks

Context Check Description
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/checkpatch success coding style OK

Commit Message

Yoan Picchi April 30, 2024, 4:27 p.m. UTC
  - Implemented SVE code for comparing signatures in bulk lookup.
- New SVE code is ~5% slower than optimized NEON for N2 processor for
128b vectors.

Signed-off-by: Yoan Picchi <yoan.picchi@arm.com>
Signed-off-by: Harjot Singh <harjot.singh@arm.com>
Reviewed-by: Nathan Brown <nathan.brown@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 lib/hash/arch/arm/compare_signatures.h | 58 ++++++++++++++++++++++++++
 lib/hash/rte_cuckoo_hash.c             |  7 +++-
 lib/hash/rte_cuckoo_hash.h             |  1 +
 3 files changed, 65 insertions(+), 1 deletion(-)
  

Comments

David Marchand June 14, 2024, 1:42 p.m. UTC | #1
On Tue, Apr 30, 2024 at 6:28 PM Yoan Picchi <yoan.picchi@arm.com> wrote:
>
> - Implemented SVE code for comparing signatures in bulk lookup.
> - New SVE code is ~5% slower than optimized NEON for N2 processor for
> 128b vectors.
>
> Signed-off-by: Yoan Picchi <yoan.picchi@arm.com>
> Signed-off-by: Harjot Singh <harjot.singh@arm.com>
> Reviewed-by: Nathan Brown <nathan.brown@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  lib/hash/arch/arm/compare_signatures.h | 58 ++++++++++++++++++++++++++
>  lib/hash/rte_cuckoo_hash.c             |  7 +++-
>  lib/hash/rte_cuckoo_hash.h             |  1 +
>  3 files changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/lib/hash/arch/arm/compare_signatures.h b/lib/hash/arch/arm/compare_signatures.h
> index 72bd171484..b4b4cf04e9 100644
> --- a/lib/hash/arch/arm/compare_signatures.h
> +++ b/lib/hash/arch/arm/compare_signatures.h
> @@ -47,6 +47,64 @@ compare_signatures_dense(uint16_t *hitmask_buffer,
>                 *hitmask_buffer = vaddvq_u16(hit2);
>                 }
>                 break;
> +#endif
> +#if defined(RTE_HAS_SVE_ACLE)
> +       case RTE_HASH_COMPARE_SVE: {
> +               svuint16_t vsign, shift, sv_matches;
> +               svbool_t pred, match, bucket_wide_pred;
> +               int i = 0;
> +               uint64_t vl = svcnth();
> +
> +               vsign = svdup_u16(sig);
> +               shift = svindex_u16(0, 1);
> +
> +               if (vl >= 2 * RTE_HASH_BUCKET_ENTRIES && RTE_HASH_BUCKET_ENTRIES <= 8) {
> +                       svuint16_t primary_array_vect, secondary_array_vect;
> +                       bucket_wide_pred = svwhilelt_b16(0, RTE_HASH_BUCKET_ENTRIES);
> +                       primary_array_vect = svld1_u16(bucket_wide_pred, prim_bucket_sigs);
> +                       secondary_array_vect = svld1_u16(bucket_wide_pred, sec_bucket_sigs);
> +
> +                       /* We merged the two vectors so we can do both comparisons at once */
> +                       primary_array_vect = svsplice_u16(bucket_wide_pred,
> +                               primary_array_vect,
> +                               secondary_array_vect);
> +                       pred = svwhilelt_b16(0, 2*RTE_HASH_BUCKET_ENTRIES);
> +
> +                       /* Compare all signatures in the buckets */
> +                       match = svcmpeq_u16(pred, vsign, primary_array_vect);
> +                       if (svptest_any(svptrue_b16(), match)) {
> +                               sv_matches = svdup_u16(1);
> +                               sv_matches = svlsl_u16_z(match, sv_matches, shift);
> +                               *hitmask_buffer = svorv_u16(svptrue_b16(), sv_matches);
> +                       }
> +               } else {
> +                       do {
> +                               pred = svwhilelt_b16(i, RTE_HASH_BUCKET_ENTRIES);
> +                               uint16_t lower_half = 0;
> +                               uint16_t upper_half = 0;
> +                               /* Compare all signatures in the primary bucket */
> +                               match = svcmpeq_u16(pred, vsign, svld1_u16(pred,
> +                                                       &prim_bucket_sigs[i]));
> +                               if (svptest_any(svptrue_b16(), match)) {
> +                                       sv_matches = svdup_u16(1);
> +                                       sv_matches = svlsl_u16_z(match, sv_matches, shift);
> +                                       lower_half = svorv_u16(svptrue_b16(), sv_matches);
> +                               }
> +                               /* Compare all signatures in the secondary bucket */
> +                               match = svcmpeq_u16(pred, vsign, svld1_u16(pred,
> +                                                       &sec_bucket_sigs[i]));
> +                               if (svptest_any(svptrue_b16(), match)) {
> +                                       sv_matches = svdup_u16(1);
> +                                       sv_matches = svlsl_u16_z(match, sv_matches, shift);
> +                                       upper_half = svorv_u16(svptrue_b16(), sv_matches)
> +                                               << RTE_HASH_BUCKET_ENTRIES;
> +                               }
> +                               hitmask_buffer[i / 8] = upper_half | lower_half;
> +                               i += vl;
> +                       } while (i < RTE_HASH_BUCKET_ENTRIES);
> +               }
> +               }
> +               break;
>  #endif
>         default:
>                 for (unsigned int i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
> diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> index 0697743cdf..75f555ba2c 100644
> --- a/lib/hash/rte_cuckoo_hash.c
> +++ b/lib/hash/rte_cuckoo_hash.c
> @@ -450,8 +450,13 @@ rte_hash_create(const struct rte_hash_parameters *params)
>                 h->sig_cmp_fn = RTE_HASH_COMPARE_SSE;
>         else
>  #elif defined(RTE_ARCH_ARM64)
> -       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))
> +       if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON)) {
>                 h->sig_cmp_fn = RTE_HASH_COMPARE_NEON;
> +#if defined(RTE_HAS_SVE_ACLE)
> +               if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> +                       h->sig_cmp_fn = RTE_HASH_COMPARE_SVE;
> +#endif
> +       }
>         else
>  #endif
>                 h->sig_cmp_fn = RTE_HASH_COMPARE_SCALAR;
> diff --git a/lib/hash/rte_cuckoo_hash.h b/lib/hash/rte_cuckoo_hash.h
> index a528f1d1a0..01ad01c258 100644
> --- a/lib/hash/rte_cuckoo_hash.h
> +++ b/lib/hash/rte_cuckoo_hash.h
> @@ -139,6 +139,7 @@ enum rte_hash_sig_compare_function {
>         RTE_HASH_COMPARE_SCALAR = 0,
>         RTE_HASH_COMPARE_SSE,
>         RTE_HASH_COMPARE_NEON,
> +       RTE_HASH_COMPARE_SVE,
>         RTE_HASH_COMPARE_NUM
>  };

I am surprised the ABI check does not complain over this change.
RTE_HASH_COMPARE_NUM is not used and knowing the number of compare
function implementations should not be of interest for an application.
But it still seem an ABI breakage to me.

RTE_HASH_COMPARE_NUM can be removed in v24.11.
And ideally, sig_cmp_fn should be made opaque (or moved to an opaque
struct out of the rte_hash public struct).
  

Patch

diff --git a/lib/hash/arch/arm/compare_signatures.h b/lib/hash/arch/arm/compare_signatures.h
index 72bd171484..b4b4cf04e9 100644
--- a/lib/hash/arch/arm/compare_signatures.h
+++ b/lib/hash/arch/arm/compare_signatures.h
@@ -47,6 +47,64 @@  compare_signatures_dense(uint16_t *hitmask_buffer,
 		*hitmask_buffer = vaddvq_u16(hit2);
 		}
 		break;
+#endif
+#if defined(RTE_HAS_SVE_ACLE)
+	case RTE_HASH_COMPARE_SVE: {
+		svuint16_t vsign, shift, sv_matches;
+		svbool_t pred, match, bucket_wide_pred;
+		int i = 0;
+		uint64_t vl = svcnth();
+
+		vsign = svdup_u16(sig);
+		shift = svindex_u16(0, 1);
+
+		if (vl >= 2 * RTE_HASH_BUCKET_ENTRIES && RTE_HASH_BUCKET_ENTRIES <= 8) {
+			svuint16_t primary_array_vect, secondary_array_vect;
+			bucket_wide_pred = svwhilelt_b16(0, RTE_HASH_BUCKET_ENTRIES);
+			primary_array_vect = svld1_u16(bucket_wide_pred, prim_bucket_sigs);
+			secondary_array_vect = svld1_u16(bucket_wide_pred, sec_bucket_sigs);
+
+			/* We merged the two vectors so we can do both comparisons at once */
+			primary_array_vect = svsplice_u16(bucket_wide_pred,
+				primary_array_vect,
+				secondary_array_vect);
+			pred = svwhilelt_b16(0, 2*RTE_HASH_BUCKET_ENTRIES);
+
+			/* Compare all signatures in the buckets */
+			match = svcmpeq_u16(pred, vsign, primary_array_vect);
+			if (svptest_any(svptrue_b16(), match)) {
+				sv_matches = svdup_u16(1);
+				sv_matches = svlsl_u16_z(match, sv_matches, shift);
+				*hitmask_buffer = svorv_u16(svptrue_b16(), sv_matches);
+			}
+		} else {
+			do {
+				pred = svwhilelt_b16(i, RTE_HASH_BUCKET_ENTRIES);
+				uint16_t lower_half = 0;
+				uint16_t upper_half = 0;
+				/* Compare all signatures in the primary bucket */
+				match = svcmpeq_u16(pred, vsign, svld1_u16(pred,
+							&prim_bucket_sigs[i]));
+				if (svptest_any(svptrue_b16(), match)) {
+					sv_matches = svdup_u16(1);
+					sv_matches = svlsl_u16_z(match, sv_matches, shift);
+					lower_half = svorv_u16(svptrue_b16(), sv_matches);
+				}
+				/* Compare all signatures in the secondary bucket */
+				match = svcmpeq_u16(pred, vsign, svld1_u16(pred,
+							&sec_bucket_sigs[i]));
+				if (svptest_any(svptrue_b16(), match)) {
+					sv_matches = svdup_u16(1);
+					sv_matches = svlsl_u16_z(match, sv_matches, shift);
+					upper_half = svorv_u16(svptrue_b16(), sv_matches)
+						<< RTE_HASH_BUCKET_ENTRIES;
+				}
+				hitmask_buffer[i / 8] = upper_half | lower_half;
+				i += vl;
+			} while (i < RTE_HASH_BUCKET_ENTRIES);
+		}
+		}
+		break;
 #endif
 	default:
 		for (unsigned int i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
index 0697743cdf..75f555ba2c 100644
--- a/lib/hash/rte_cuckoo_hash.c
+++ b/lib/hash/rte_cuckoo_hash.c
@@ -450,8 +450,13 @@  rte_hash_create(const struct rte_hash_parameters *params)
 		h->sig_cmp_fn = RTE_HASH_COMPARE_SSE;
 	else
 #elif defined(RTE_ARCH_ARM64)
-	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))
+	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON)) {
 		h->sig_cmp_fn = RTE_HASH_COMPARE_NEON;
+#if defined(RTE_HAS_SVE_ACLE)
+		if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
+			h->sig_cmp_fn = RTE_HASH_COMPARE_SVE;
+#endif
+	}
 	else
 #endif
 		h->sig_cmp_fn = RTE_HASH_COMPARE_SCALAR;
diff --git a/lib/hash/rte_cuckoo_hash.h b/lib/hash/rte_cuckoo_hash.h
index a528f1d1a0..01ad01c258 100644
--- a/lib/hash/rte_cuckoo_hash.h
+++ b/lib/hash/rte_cuckoo_hash.h
@@ -139,6 +139,7 @@  enum rte_hash_sig_compare_function {
 	RTE_HASH_COMPARE_SCALAR = 0,
 	RTE_HASH_COMPARE_SSE,
 	RTE_HASH_COMPARE_NEON,
+	RTE_HASH_COMPARE_SVE,
 	RTE_HASH_COMPARE_NUM
 };