RE: [PATCH 0/4] log2: make is_power_of_2() more generic

From: David Laight
Date: Thu Mar 30 2023 - 17:53:15 EST


From: Andrew Morton
> Sent: 30 March 2023 20:51
>
> On Thu, 30 Mar 2023 13:42:39 +0300 Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
>
> > is_power_of_2() only works for types <= sizeof(unsigned long) and it's
> > also not a constant expression. There are a number of places in kernel
> > where is_power_of_2() is called on u64, which fails on 32-bit
> > builds. Try to remedy that. While at it, make it a constant expression
> > when possible.
>
> Yes, the current `is_power_of_2(unsigned long n)' isn't very general.
>
> But wouldn't all these issues be addressed by simply doing
>
> #define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))
>
> ?
>
> (With suitable tweaks to avoid evaluating `n' more than once)

I think you need to use the 'horrid tricks' from min() to get
a constant expression from constant inputs.

For non-constants this looks ok (see https://godbolt.org/z/G73MTr9jn)

David

static inline int lisp2(unsigned long n)
{
return n && !(n & (n - 1));
}

static inline int llisp2(unsigned long long lln)
{
#if 0 // I think this looks worse, esp. for gcc on x86
return lln && !(lln & (lln - 1));
#else
unsigned long n = lln;
if (lln >= 1ull << 32) {
if (n)
return 0;
n = lln >> 32;
}
return lisp2(n);
#endif
}

#define isp2(n) (sizeof ((n)+0) == sizeof (long) ? lisp2(n) : llisp2(n))

int is12(unsigned long long i)
{
return isp2(i);
}

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)