On 24.05.2024 22:03, Andrew Cooper wrote: > Implement ffs64() and fls64() as plain static inlines, dropping the ifdefary > and intermediate generic_f?s64() forms. > > Add tests for all interesting bit positions at 32bit boundaries. > > No functional change. > > Signed-off-by: Andrew Cooper <[email protected]>
Reviewed-by: Jan Beulich <[email protected]> with two remarks: > --- a/xen/common/bitops.c > +++ b/xen/common/bitops.c > @@ -24,6 +24,22 @@ static void __init test_ffs(void) > CHECK(ffsl, 1UL << 32, 33); > CHECK(ffsl, 1UL << 63, 64); > #endif > + > + /* > + * unsigned int ffs64(uint64_t) > + * > + * 32-bit builds of Xen have to split this into two adjacent operations, > + * so test all interesting bit positions across the divide. > + */ > + CHECK(ffs64, 0, 0); > + CHECK(ffs64, 1, 1); > + CHECK(ffs64, 3, 1); > + CHECK(ffs64, 7, 1); > + CHECK(ffs64, 6, 2); > + > + CHECK(ffs64, 0x8000000080000000ULL, 32); > + CHECK(ffs64, 0x8000000100000000ULL, 33); > + CHECK(ffs64, 0x8000000000000000ULL, 64); With the intermediate blank line, the respective part of the comment doesn't look to be related to these 3 lines. Could I talk you into moving that part down? > --- a/xen/include/xen/bitops.h > +++ b/xen/include/xen/bitops.h > @@ -60,6 +60,14 @@ static always_inline __pure unsigned int ffsl(unsigned > long x) > #endif > } > > +static always_inline __pure unsigned int ffs64(uint64_t x) > +{ > + if ( BITS_PER_LONG == 64 ) In principle >= 64 would be okay here, and hence I'd prefer if we used that less strict form. Yet I'm not going to insist. Jan
