Re: [PATCH 1/2] eal: provide leading and trailing zero bit count abstraction

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

On 2022-11-23 23:14, Tyler Retzlaff wrote:
> Provide an abstraction for leading and trailing zero bit counting
> functions to hide compiler specific intrinsics and builtins.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>   lib/eal/include/meson.build    |   1 +
>   lib/eal/include/rte_bitcount.h | 257 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 258 insertions(+)
>   create mode 100644 lib/eal/include/rte_bitcount.h
> 
> diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
> index cfcd40a..8ff1d65 100644
> --- a/lib/eal/include/meson.build
> +++ b/lib/eal/include/meson.build
> @@ -5,6 +5,7 @@ includes += include_directories('.')
>   
>   headers += files(
>           'rte_alarm.h',
> +        'rte_bitcount.h',

Maybe rte_bitops.h should be the home of these new wrappers.

>           'rte_bitmap.h',
>           'rte_bitops.h',
>           'rte_branch_prediction.h',
> diff --git a/lib/eal/include/rte_bitcount.h b/lib/eal/include/rte_bitcount.h
> new file mode 100644
> index 0000000..2f4dd38
> --- /dev/null
> +++ b/lib/eal/include/rte_bitcount.h
> @@ -0,0 +1,257 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2022 Microsoft Corporation
> + */
> +
> +#ifndef EAL_BITCOUNT_H
> +#define EAL_BITCOUNT_H
> +
> +#include <rte_compat.h>
> +
> +#ifdef RTE_TOOLCHAIN_MSVC
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned

unsigned int

If you want to be __builtin_clz()-compatible, you should stay with "int" 
as the return type.

If you don't, you might want to change more than just the return type. 
In particular, it would be useful to have size-specific variants 
(although in practice they are on most [all?] DPDK-supported platforms).

unsigned int
rte_clz(uint32_t v);

or rte_clz32(), to follow the naming convention from some other wrapper 
RFC you posted.

> +rte_clz(unsigned int v)
> +{
> +	unsigned long rv;
> +
> +	(void)_BitScanReverse(&rv, v);
> +
> +	return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clzl(unsigned long v)
> +{
> +	unsigned long rv;
> +
> +	(void)_BitScanReverse(&rv, v);
> +
> +	return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clzll(unsigned long long v)
> +{
> +	unsigned long rv;
> +
> +	(void)_BitScanReverse64(&rv, v);
> +
> +	return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctz(unsigned int v)
> +{
> +	unsigned long rv;
> +
> +	(void)_BitScanForward(&rv, v);
> +
> +	return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctzl(unsigned long v)
> +{
> +	unsigned long rv;
> +
> +	(void)_BitScanForward(&rv, v);
> +
> +	return (unsigned)rv;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctzll(unsigned long long v)
> +{
> +	unsigned long rv;
> +
> +	(void)_BitScanForward64(&rv, v);
> +
> +	return (unsigned)rv;
> +}
> +
> +#else
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clz(unsigned int v)
> +{
> +	return (unsigned)__builtin_clz(v);
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clzl(unsigned long v)
> +{
> +	return (unsigned)__builtin_clzl(v);
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of leading 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of leading zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_clzll(unsigned long v)
> +{
> +	return (unsigned)__builtin_clzll(v);
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctz(unsigned int v)
> +{
> +	return (unsigned)__builtin_ctz(v);
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctzl(unsigned long v)
> +{
> +	return (unsigned)__builtin_ctzl(v);
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get the count of trailing 0-bits in v.
> + *
> + * @param v
> + *   The value.
> + * @return
> + *   The count of trailing zero bits.
> + */
> +__rte_experimental
> +static inline unsigned
> +rte_ctzll(unsigned long v)
> +{
> +	return (unsigned)__builtin_ctzll(v);
> +}
> +
> +#endif
> +
> +#endif /* EAL_BITCOUNT_H */
> +