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

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

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 5 January 2023 21.58
> 
> On Thu, Jan 05, 2023 at 09:27:12AM -0800, Tyler Retzlaff wrote:
> > On Thu, Jan 05, 2023 at 09:23:49AM -0800, Tyler Retzlaff wrote:
> > > > > 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.
> > >
> > > 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?
> >
> > 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.
> >
> > anyway, i'll submit v3 with this change anyway.
> 
> so when attempting to do this it became immediately obvious that moving
> just the bit op functions out is going to create a circular dependency
> between rte_common.h, rte_bitops.h
> 
> once the bit ops are moved out of common there are still other inline
> functions that remain in comman that require bringing bitops back in,
> but bitops depends on common.
> 
> my compromise will be to break log2 and pow2 inline functions into
> their
> own files to break the cycle (common no longer depends on bitops). i'll
> submit patches for this but it ends up touching a lot more of the
> tree to add back includes for log/pow inline use.
> 
> alternatively i can just not move the remaining bit manipulation
> functions, let me know which is preferred.

It seems that no perfect solution exists, so we will have to live with a compromise. Here is another proposal for a compromise, for yours and Thomas's consideration:

I noticed that rte_bitops.h is mainly for setting/getting bits, used for accessing hardware.

Your functions are mathematical functions, and so are the similar functions in rte_common.h (which is why it makes sense to keep them together with yours). If we cannot clean up rte_common.h by moving them out, perhaps we should accept the current situation (until we find a way to move them out) and just add your mathematical functions where the existing mathematical functions reside, i.e. in rte_common.h.

This proposal only makes the existing mess slightly larger; it doesn't create a new kind of mess.

-Morten