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

From: David Laight <David.Laight@ACULAB.COM>
To: 'Andrew Morton' <akpm@linux-foundation.org>,
	Jani Nikula <jani.nikula@intel.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"Christian König" <christian.koenig@amd.com>,
	"David Gow" <davidgow@google.com>
Subject: RE: [PATCH 0/4] log2: make is_power_of_2() more generic
Date: Thu, 30 Mar 2023 21:53:03 +0000	[thread overview]
Message-ID: <549987e4967d45159573901d330c96a0@AcuMS.aculab.com> (raw)
In-Reply-To: <20230330125041.83b0f39fa3a4ec1a42dfd95f@linux-foundation.org>

From: Andrew Morton
> Sent: 30 March 2023 20:51
> 
> On Thu, 30 Mar 2023 13:42:39 +0300 Jani Nikula <jani.nikula@intel.com> 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)