Linux-Toolchains Archive at lore.kernel.org
 help / color / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: linux-toolchains@vger.kernel.org,
	Christophe Leroy <christophe.leroy@csgroup.eu>
Subject: ilog2 vs. GCC inlining heuristics
Date: Wed, 21 Oct 2020 15:27:18 +0200
Message-ID: <20201021132718.GB2176@tucnak> (raw)

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)) :	\
 	(sizeof(n) <= 4) ?		\
 	__ilog2_u32(n) :		\
 	__ilog2_u64(n)			\

	Jakub


             reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 13:27 Jakub Jelinek [this message]
2020-10-21 13:36 ` Christophe Leroy
2020-10-21 13:45   ` Jakub Jelinek
2020-10-21 15:19 ` Peter Zijlstra
2020-10-21 18:40   ` Christophe Leroy
2020-10-22  4:01     ` Randy Dunlap
2020-10-22  7:12       ` Jakub Jelinek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201021132718.GB2176@tucnak \
    --to=jakub@redhat.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-toolchains@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Toolchains Archive at lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-toolchains/0 linux-toolchains/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-toolchains linux-toolchains/ https://lore.kernel.org/linux-toolchains \
		linux-toolchains@vger.kernel.org
	public-inbox-index linux-toolchains

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-toolchains


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git