Re: [PATCH v8 1/4] hash: pack the hitmask for hash in bulk lookup

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 17 Apr 2024 16:08:04 +0000
Yoan Picchi <yoan.picchi@arm.com> wrote:

> diff --git a/lib/hash/arch/common/compare_signatures.h b/lib/hash/arch/common/compare_signatures.h
> new file mode 100644
> index 0000000000..59157d31e1
> --- /dev/null
> +++ b/lib/hash/arch/common/compare_signatures.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2016 Intel Corporation
> + * Copyright(c) 2018-2024 Arm Limited
> + */
> +
> +/*
> + * The generic version could use either a dense or sparsely packed hitmask buffer,
> + * but the dense one is slightly faster.
> + */
> +
> +#include <inttypes.h>
> +#include <rte_common.h>
> +#include <rte_vect.h>
> +#include "rte_cuckoo_hash.h"
> +
> +#define DENSE_HASH_BULK_LOOKUP 1
> +
> +static inline void
> +compare_signatures_dense(uint16_t *hitmask_buffer,
> +			const uint16_t *prim_bucket_sigs,
> +			const uint16_t *sec_bucket_sigs,
> +			uint16_t sig,
> +			enum rte_hash_sig_compare_function sig_cmp_fn)
> +{
> +	(void) sig_cmp_fn;

Please use __rte_unused attribute for this.
> +
> +	static_assert(sizeof(*hitmask_buffer) >= 2 * (RTE_HASH_BUCKET_ENTRIES / 8),
> +	"The hitmask must be exactly wide enough to accept the whole hitmask if it is dense");

The message should be indented like multi-line function args.
If possible shorten the message.

> +
> +	/* For match mask every bits indicates the match */
> +	for (unsigned int i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
> +		*hitmask_buffer |=
> +			((sig == prim_bucket_sigs[i]) << i);

Don't really need () around the whole expression here.

> +		*hitmask_buffer |=
> +			((sig == sec_bucket_sigs[i]) << i) << RTE_HASH_BUCKET_ENTRIES;
> +	}
> +
> +}