Re: ilog2 vs. GCC inlining heuristics

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 21/10/2020 à 15:27, Jakub Jelinek a écrit :
> Hi!
> 
> Based on the GCC PR97445 discussions, I'd like to propose following change,
> which should significantly decrease the amount of code in inline functions
> that use ilog2, but as I'm already two decades out of the Linux kernel
> development, I'd appreciate if some kernel developer could try that (all
> I have done is check that it gives the same results as before) and if it
> works submit it for inclusion into the kernel?
> 
> Thanks.
> 
> 
> Improve ilog2 for constant arguments
> 
> As discussed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97445
> the const_ilog2 macro generates a lot of code which interferes badly
> with GCC inlining heuristics, until it can be proven that the ilog2
> argument can or can't be simplified into a constant.
> 
> It can be expressed using __builtin_clzll builtin which is supported
> by GCC 3.4 and later and when used only in the __builtin_constant_p guarded
> code it ought to always fold back to a constant.
> Other compilers support the same builtin for many years too.
> 
> Other option would be to change the const_ilog2 macro, though as the
> description says it is meant to be used also in C constant expressions,
> and while GCC will fold it to constant with constant argument even in
> those, perhaps it is better to avoid using extensions in that case.
> 
> Signed-off-by: Jakub Jelinek <jakub@redhat.com>
> 
> diff --git a/include/linux/log2.h b/include/linux/log2.h
> index c619ec6eff4a..4307d3477642 100644
> --- a/include/linux/log2.h
> +++ b/include/linux/log2.h
> @@ -156,7 +156,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>   #define ilog2(n) \
>   ( \
>   	__builtin_constant_p(n) ?	\
> -	const_ilog2(n) :		\
> +	((n) < 2 ? 0 :			\
> +	 63 - __builtin_clzll (n)) :	\

That is likely to work only on 64 bits targets.
On 32 bits targets I guess it needs to be  31 - __builtin_clz (n), doesn't it ?

>   	(sizeof(n) <= 4) ?		\
>   	__ilog2_u32(n) :		\
>   	__ilog2_u64(n)			\
> 
> 	Jakub
> 

Christophe