Re: [dpdk-dev] [PATCH] librte_eal/common: fix return type of rte_bsf64

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

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tyler Retzlaff
> Sent: Saturday, March 13, 2021 2:10 AM
> 
> On Fri, Mar 12, 2021 at 10:13:30PM +0100, Morten Brørup wrote:
> > > >
> > > > Please also update the similar math functions in rte_common.h, so the
> > > return type is consistent across these functions:
> > > > - rte_bsf32()
> > > > - rte_bsf32_safe()
> > > > - rte_fls_u32()
> > > > - rte_bsf64()
> > > > - rte_fls_u64()
> > > > - rte_log2_u32()
> > > > - rte_log2_u64()
> > >
> > > agreed, happy to review the whole set and deal with it all at once.
> >
> > Ups. I should have omitted rte_bsf32_safe() from the list. It returns a
> Boolean.
> 
> if we can agree that we can use C11 as a base we could just do away with
> all this dumb 32-bit vs 64-bit static selection at the call site (at
> least for rte_bsf32() and rte_bsf64() and probably others).
> 
> i could just introduce the following macro and deprecate the current
> inline functions.
> 
> #define rte_bsf(v) \
>     (uint32_t)_Generic((v), \
>         uint8_t: __builtin_ctz, \
>         uint16_t: __builtin_ctz, \
>         uint32_t: __builtin_ctz, \
>         default: __builtin_ctzll)(v)
> 
> uint8_t a = ...;
> uint16_t b = ...;
> uint32_t c = ...;
> uint64_t d = ...;
> 
> the following would jw as intended, though given the range of the value
> that may be returned we could narrow the cast to uint8_t so we don't
> have to sprinkle casts in places where usage like uint8_t x = rte_bsf(v);
> exists.
> 
> rte_bsf(a);
> rte_bsf(b);
> rte_bsf(c);
> rte_bsf(d);
> 
> anyway, if people care let me know otherwise i'm just going to review
> and fix what is already there.

That would certainly be a nice improvement. But going down that road should include reviewing and fixing all DPDK libraries with 32/64 versions of the same functions, e.g. rte_bitops.h - which probably requires a significant effort.

So just limit the task to fixing what is already there.