Undefined behavior
Your code runs into undefined behavior when the input number is 0x80000001
or larger. The undefined behavior comes here:
uint32_t p = (is_pot ? n : 1 << msb_index);
because msb_index
will be 32, and shifting a uint32_t
left by 32 is undefined behavior. However, the problem description does not specify what should be returned if the input is that large (since there is no larger power of two that will fit in a uint32_t
), so it's not clear whether returning an arbitrary value is acceptable.
Type of 1
Another small note is that when you shift like this:
1 << msb_index
the type of 1
is int
, not uint32_t
. So if you were on a machine with 16-bit integers, you would get incorrect results. To fix this, you should cast the 1
to a uint32_t
like this:
(uint32_t) 1 << msb_index
Branchless
Your current function seems pretty fast, but suffers from a branch because you check for a power of two as a special case. In order to avoid the special case, you can do a trick and instead of doing fls(n)
, you can do fls(n+n-1)
. This will give you the bit number of the next higher bit while taking into account the special case of a perfect power of two.
The only problem with this technique is for the input of 0. To fix the special case for 0, you can do the following:
n += !n;
Here, the n += !n
will not have any effect unless n
is zero, in which case it adds 1 to n. So the final rewrite of your function would be:
uint32_t nextPowerOfTwo(uint32_t n) {
n += !n;
return (uint32_t) 1 << (fls(n+n-1) - 1);
}
Benchmarking
I didn't have fls()
available on my system, so I used 32 - __builtin_clz()
instead, which should be the same as fls()
. The code I used to test was:
uint32_t nextPowerOfTwo(uint32_t n)
{
n += !n;
return (uint32_t) 1 << (31 - __builtin_clz(n+n-1));
}
I ran the function on each number from 0 to 0xffffffff. And the timing results were:
Original function: 12.7 seconds
Adriano Repetti : 8.8 seconds
JS1 : 4.7 seconds
I also ran some of the other proposed solutions, but for me they ended up slower than the original. So perhaps they didn't translate to my platform (Gnu C compiler on Cygwin/Windows 7/32-bit X86).
fls
isn't available in POSIX, andstd::__ctz
looks like a Clang instrinsic. However,fls
in Free/OpenBSD is implemented as a loop. \$\endgroup\$return pow(2, ceil(log2(n)));
\$\endgroup\$pow
andlog2
are expensive functions to evaluate compared to twiddling with the bits. Just try to test it: your suggestion is much slower compared to the solutions below. On my system it's a factor 4 slower on average when tested on n in the range [1,10^9]. \$\endgroup\$0x80000000
? Your code has undefined behavior (shift left by 32) when that happens. \$\endgroup\$