RE: [PATCH 0/5] use abstracted bit count functions

["Morten Brørup" <mb@smartsharesystems.com>]

> 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?