[0/5] use abstracted bit count functions

  The first set of conversions missed the long 'l' versions of the
builtins that were being used. This series completes the conversion
of remaining libraries from __builtin_ctzl and __builtin_clzl.

Tyler Retzlaff (5):
  table: use abstracted bit count functions
  distributor: use abstracted bit count functions
  hash: use abstracted bit count functions
  member: use abstracted bit count functions
  rcu: use abstracted bit count functions

 lib/distributor/rte_distributor_single.c |  2 +-
 lib/hash/rte_cuckoo_hash.c               | 16 ++++++++--------
 lib/member/rte_member_vbf.c              | 12 ++++++------
 lib/member/rte_member_x86.h              |  6 +++---
 lib/rcu/rte_rcu_qsbr.c                   |  4 ++--
 lib/rcu/rte_rcu_qsbr.h                   |  2 +-
 lib/table/rte_lru_arm64.h                |  2 +-
 lib/table/rte_swx_table_em.c             |  4 ++--
 lib/table/rte_table_hash_ext.c           |  4 ++--
 lib/table/rte_table_hash_lru.c           |  4 ++--
 10 files changed, 28 insertions(+), 28 deletions(-)
  

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 2 November 2023 02.05
> 
> The first set of conversions missed the long 'l' versions of the
> builtins that were being used. This series completes the conversion
> of remaining libraries from __builtin_ctzl and __builtin_clzl.

NAK to blind search/replace of __builtin_clzl()/clzl().

Although the size of long is 64 bit on 64 bit architectures, it only 32 bit on 32 bit architectures.

You need to look at the types these builtins operate on:
- E.g. in the hash library (patch 3/5) prim_hitmask[i]/sec_hitmask[i] are uint32_t, so rte_ctz32() would be the correct replacement. (I am now asking myself why they were using __builtin_ctzl() instead of __builtin_ctz() here... Probably by mistake.)
- And if the type is "long", you need conditional compiling (or a wrapper macro) to choose between the 32 bit or 64 bit variants.

NB: You can blindly replace __builtin_ctzll()/clzll(), if any, by 64 bit functions.
  

On Thu, Nov 02, 2023 at 08:39:04AM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 2 November 2023 02.05
> > 
> > The first set of conversions missed the long 'l' versions of the
> > builtins that were being used. This series completes the conversion
> > of remaining libraries from __builtin_ctzl and __builtin_clzl.
> 
> NAK to blind search/replace of __builtin_clzl()/clzl().
> 
> Although the size of long is 64 bit on 64 bit architectures, it only 32 bit on 32 bit architectures.
> 
> You need to look at the types these builtins operate on:
> - E.g. in the hash library (patch 3/5) prim_hitmask[i]/sec_hitmask[i] are uint32_t, so rte_ctz32() would be the correct replacement. (I am now asking myself why they were using __builtin_ctzl() instead of __builtin_ctz() here... Probably by mistake.)
> - And if the type is "long", you need conditional compiling (or a wrapper macro) to choose between the 32 bit or 64 bit variants.
> 
> NB: You can blindly replace __builtin_ctzll()/clzll(), if any, by 64 bit functions.

they haven't been blindly replaced. but i would like you to validate my
thinking.

in the case of counting trailing 0s it seems fine if the type is
promoted to 64-bits, in the case of leading i checked the type to make
sure it was already operating on a 64-bit type.

too naive?
  

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 2 November 2023 16.28
> 
> On Thu, Nov 02, 2023 at 08:39:04AM +0100, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Thursday, 2 November 2023 02.05
> > >
> > > The first set of conversions missed the long 'l' versions of the
> > > builtins that were being used. This series completes the conversion
> > > of remaining libraries from __builtin_ctzl and __builtin_clzl.
> >
> > NAK to blind search/replace of __builtin_clzl()/clzl().
> >
> > Although the size of long is 64 bit on 64 bit architectures, it only
> 32 bit on 32 bit architectures.
> >
> > You need to look at the types these builtins operate on:
> > - E.g. in the hash library (patch 3/5) prim_hitmask[i]/sec_hitmask[i]
> are uint32_t, so rte_ctz32() would be the correct replacement. (I am
> now asking myself why they were using __builtin_ctzl() instead of
> __builtin_ctz() here... Probably by mistake.)
> > - And if the type is "long", you need conditional compiling (or a
> wrapper macro) to choose between the 32 bit or 64 bit variants.
> >
> > NB: You can blindly replace __builtin_ctzll()/clzll(), if any, by 64
> bit functions.
> 
> they haven't been blindly replaced. but i would like you to validate my
> thinking.
> 
> in the case of counting trailing 0s it seems fine if the type is
> promoted to 64-bits,

This will give the correct result, yes. However the 64-bit operation might have a higher performance cost than the 32-bit operation, especially on 32-bit architectures.

> in the case of leading i checked the type to make
> sure it was already operating on a 64-bit type.

If already operating on a 64-bit type, using the 64-bit function is obviously correct.

> 
> too naive?
  

On Thu, Nov 02, 2023 at 04:33:57PM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 2 November 2023 16.28
> > 
> > On Thu, Nov 02, 2023 at 08:39:04AM +0100, Morten Brørup wrote:
> > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > Sent: Thursday, 2 November 2023 02.05
> > > >
> > > > The first set of conversions missed the long 'l' versions of the
> > > > builtins that were being used. This series completes the conversion
> > > > of remaining libraries from __builtin_ctzl and __builtin_clzl.
> > >
> > > NAK to blind search/replace of __builtin_clzl()/clzl().
> > >
> > > Although the size of long is 64 bit on 64 bit architectures, it only
> > 32 bit on 32 bit architectures.
> > >
> > > You need to look at the types these builtins operate on:
> > > - E.g. in the hash library (patch 3/5) prim_hitmask[i]/sec_hitmask[i]
> > are uint32_t, so rte_ctz32() would be the correct replacement. (I am
> > now asking myself why they were using __builtin_ctzl() instead of
> > __builtin_ctz() here... Probably by mistake.)
> > > - And if the type is "long", you need conditional compiling (or a
> > wrapper macro) to choose between the 32 bit or 64 bit variants.
> > >
> > > NB: You can blindly replace __builtin_ctzll()/clzll(), if any, by 64
> > bit functions.
> > 
> > they haven't been blindly replaced. but i would like you to validate my
> > thinking.
> > 
> > in the case of counting trailing 0s it seems fine if the type is
> > promoted to 64-bits,
> 
> This will give the correct result, yes. However the 64-bit operation might have a higher performance cost than the 32-bit operation, especially on 32-bit architectures.

true. okay let me clean this up. thanks for the feedback.

> 
> > in the case of leading i checked the type to make
> > sure it was already operating on a 64-bit type.
> 
> If already operating on a 64-bit type, using the 64-bit function is obviously correct.
> 
> > 
> > too naive?