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

[Thomas Monjalon]

05/01/2023 18:27, Tyler Retzlaff:
> On Thu, Jan 05, 2023 at 09:23:49AM -0800, Tyler Retzlaff wrote:
> > On Thu, Jan 05, 2023 at 10:04:46AM +0100, Thomas Monjalon wrote:
> > > 28/11/2022 18:27, Tyler Retzlaff:
> > > > On Mon, Nov 28, 2022 at 06:22:10PM +0100, Morten Brørup wrote:
> > > > > > From: Tyler Retzlaff [mailto:roretzla at linux.microsoft.com]
> > > > > > Sent: Monday, 28 November 2022 18.14
> > > > > > 
> > > > > > On Thu, Nov 24, 2022 at 11:17:23AM +0100, Morten Brørup wrote:
> > > > > > > > From: Tyler Retzlaff [mailto:roretzla at linux.microsoft.com]
> > > > > > > > Sent: Thursday, 24 November 2022 00.43
> > > > > > > >
> > > > > > > > Provide an abstraction for leading and trailing zero bit counting
> > > > > > > > functions to hide compiler specific intrinsics and builtins.
> > > > > > > >
> > > > > > > > Signed-off-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> > > > > > 
> > > > > > let me unpack what is being asked for here.
> > > > > > 
> > > > > > > Related functions already exist in lib/eal/include/rte_common.h.
> > > > > > 
> > > > > > i don't understand. are you saying these inline functions duplicate
> > > > > > existing bit counting functionality? if so i'll relocate any you
> > > > > > identify.
> > > > > 
> > > > > Sorry about not being clear about my intentions with the feedback.
> > > > > 
> > > > > I'm not asking for anything; I only wanted to point at the similar family of functions in rte_common.h, to make sure that you were aware of them.
> > > > 
> > > > oh! not a problem. i'm very keen to catch any mistakes, thought i had
> > > > missed something.
> > > 
> > > I think we should move all bit-related functions together.
> > > Please could you add another patch to your series
> > > moving "ms1b"/"bsf"/"fls" functions in this file?
> > > 
> > > 
> > 
> > okay, so there is already a rte_bitops.h. i guess everything should go
> > there including the leading/trailing count functions instead of adding a
> > new header.

Yes good idea to gather all in rte_bitops.h.

> > i'll introduce a new patch to the series that gathers the existing
> > functions into rte_bitops.h and place the new functions there too.
> > 
> > thanks
> 
> just as a further follow up, you do understand that this is technically
> an api break?

Yes

> moving functions from rte_common.h to rte_bitops.h will make translation
> units that included only rte_common.h but used these functions will
> fail to compile without being updated to include rte_bitops.h.

These functions are not used a lot, it is a "small" API break.