linux-toolchains.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-toolchains@vger.kernel.org
Subject: Re: ilog2 vs. GCC inlining heuristics
Date: Thu, 22 Oct 2020 09:12:33 +0200	[thread overview]
Message-ID: <20201022071233.GK2176@tucnak> (raw)
In-Reply-To: <b5dc06c5-6a2c-9f12-ba43-dc83eed9a503@infradead.org>

On Wed, Oct 21, 2020 at 09:01:01PM -0700, Randy Dunlap wrote:
> I ran a (new) lib/test_log2.ko before and after Jakub's patch and got the
> same results, although I am not claiming that it has an exhaustive set of
> tests in it.  [it is attached]

The test most likely tests the non-constant paths (which is something
I haven't changed), unless all the loops would be completely unrolled,
all the arrays determined to be never written to and turned into const and
all the loads from those arrays optimized into constants.

If you want to test both the non-constant and constant paths, it would
be good to make sure to guarantee that.  For non-constant, e.g. use those
arrays but add some optimization barrier so that the compiler can't
optimize them into constants (e.g. make the arrays volatile, or add
__asm__ ("" : : "g" (test_power_of_2) : "memory");
etc. barriers at the start of test function etc.
And for constants, perhaps put the test values into macros, through
which to place it both
#define TEST_ILOG2_UINT \
  TEST (1, 0) TEST (2, 1) TEST (4, 2) TEST (~0U, 31) \
  TEST (0x80000000U, 31)
and once define TEST(x, y) as { x, y }, then #undef TEST
and in another place as if (ilog2(x) != y) ...

Also, you do want to test even behavior of ilog2 at 0, e.g. the
powerpc fls/fls64 implementations are incorrect for that value as could be
seen with -fsanitize=undefined.

And you do want to test also unsigned long long.

	Jakub


      reply	other threads:[~2020-10-22  7:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 13:27 ilog2 vs. GCC inlining heuristics Jakub Jelinek
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 [this message]

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=20201022071233.GK2176@tucnak \
    --to=jakub@redhat.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).