All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Rikard Falkeborn <rikard.falkeborn@gmail.com>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Yury Norov <yury.norov@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-m68k <linux-m68k@lists.linux-m68k.org>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	Linux-SH <linux-sh@vger.kernel.org>,
	Alexey Klimov <aklimov@redhat.com>,
	David Sterba <dsterba@suse.com>, Dennis Zhou <dennis@kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Jianpeng Ma <jianpeng.ma@intel.com>,
	Joe Perches <joe@perches.com>,
	John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Rich Felker <dalias@libc.org>,
	Stefano Brivio <sbrivio@redhat.com>,
	Wei Yang <richard.weiyang@linux.alibaba.com>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Yoshinori Sato <ysato@users.sourceforge.jp>
Subject: Re: [PATCH 11/12] tools: sync lib/find_bit implementation
Date: Wed, 12 May 2021 10:33:09 +0200	[thread overview]
Message-ID: <CAK8P3a1u3WnVAasCXG-sk=7cM2HCcitFWtXvn5hzCnpFU85Lxg@mail.gmail.com> (raw)
In-Reply-To: <030ae370-967c-22d4-56f8-cb0435be7540@rasmusvillemoes.dk>

On Wed, May 12, 2021 at 10:16 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> It's more complicated than that. __builtin_constant_p on something which
> is a bona-fide Integer Constant Expression (ICE) gets folded early to a
> 1. And then it turns out that such a __builtin_constant_p() that folds
> early to a 1 can be "stronger" than a literal 1, in the sense that when
> used as the controlling expression of a ?: with nonsense in the false
> branch, the former is OK but the latter fails:
>
> https://lore.kernel.org/lkml/c68a0f46-346c-70a0-a9b8-31747888f05f@rasmusvillemoes.dk/
>
> Now what happens when the argument to __builtin_constant_p is not an ICE
> is a lot more complicated. The argument _may_ be so obviously
> non-constant that it can be folded early to a 0, hence still be suitable
> as first argument to __b_c_e. But it is also possible that the compiler
> leaves it unevaluated, in the "hope" that a later optimization stage
> could prove the argument constant. And that's the case where __b_c_e
> will then break, because that can't be left unevaluated for very long -
> the very _type_ of the result depends on which branch is chosen.
>
> tl;dr: there's no "order in which the compiler processes those", __b_c_p
> can get evaluated (folded) early, before __b_c_e inspects it, or be left
> for later stages.

Thanks for the detailed explanation. Checking the actual behavior of
a trivial example, I find that

int f(void)
{
    const int i = 1;
    return __builtin_choose_expr(__builtin_constant_p(i), 1, 2);
}

used to return '2' with gcc-7, which is what I remembered.
With gcc-8 and up as well as any version of clang, it returns '1' now:
https://godbolt.org/z/7eKjbMocb

I have also seen a couple of cases where __builtin_constant_p()
without a __builtin_choose_expr() ended up unexpectedly
returning true when gcc found a code path that it would be constant
(e.g. conditionally initializing a variable to one of two possible
ICEs), but then later turning that back into a non-constant
expression in a later optimization stage. There is probably also
a much more detailed explanation behind those.


        Arnd

  reply	other threads:[~2021-05-12  8:34 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01  0:31 [PATCH v6 00/12] lib/find_bit: fast path for small bitmaps Yury Norov
2021-04-01  0:31 ` [PATCH 01/12] tools: disable -Wno-type-limits Yury Norov
2021-04-01  0:31 ` [PATCH 02/12] tools: bitmap: sync function declarations with the kernel Yury Norov
2021-04-01  0:31 ` [PATCH 03/12] tools: sync BITMAP_LAST_WORD_MASK() macro " Yury Norov
2021-04-01  0:31 ` [PATCH 04/12] arch: rearrange headers inclusion order in asm/bitops for m68k and sh Yury Norov
2021-04-01  0:31 ` [PATCH 05/12] lib: extend the scope of small_const_nbits() macro Yury Norov
2021-04-01  8:35   ` Andy Shevchenko
2021-04-01  0:31 ` [PATCH 06/12] tools: sync small_const_nbits() macro with the kernel Yury Norov
2021-04-01  0:31 ` [PATCH 07/12] lib: inline _find_next_bit() wrappers Yury Norov
2021-04-01  8:37   ` Andy Shevchenko
2021-04-01  0:31 ` [PATCH 08/12] tools: sync find_next_bit implementation Yury Norov
2021-04-01  0:31 ` [PATCH 09/12] lib: add fast path for find_next_*_bit() Yury Norov
2021-04-01  8:48   ` Andy Shevchenko
2021-04-01  0:31 ` [PATCH 10/12] lib: add fast path for find_first_*_bit() and find_last_bit() Yury Norov
2021-04-01  4:21   ` kernel test robot
2021-04-01  8:58   ` Andy Shevchenko
2021-04-01  0:31 ` [PATCH 11/12] tools: sync lib/find_bit implementation Yury Norov
2021-05-10 15:27   ` Tetsuo Handa
2021-05-10 15:44     ` Andy Shevchenko
2021-05-10 17:21       ` Yury Norov
2021-05-10 22:51       ` Rikard Falkeborn
2021-05-11  7:28         ` Andy Shevchenko
2021-05-11 10:36           ` Rikard Falkeborn
2021-05-11 11:53             ` Tetsuo Handa
2021-05-11 20:37               ` Rikard Falkeborn
2021-05-12  7:48                 ` Arnd Bergmann
2021-05-12  8:15                   ` Rasmus Villemoes
2021-05-12  8:33                     ` Arnd Bergmann [this message]
2021-05-11 12:17             ` Andy Shevchenko
2021-04-01  0:31 ` [PATCH 12/12] MAINTAINERS: Add entry for the bitmap API Yury Norov
2021-04-01  9:14 ` [PATCH v6 00/12] lib/find_bit: fast path for small bitmaps Andy Shevchenko
2021-04-01  9:28   ` Arnd Bergmann
2021-04-01  9:50     ` Andy Shevchenko
2021-04-02  0:32       ` Andrew Morton
  -- strict thread matches above, loose matches on Subject: below --
2021-03-21 21:54 [PATCH v5 " Yury Norov
2021-03-21 21:54 ` [PATCH 11/12] tools: sync lib/find_bit implementation Yury Norov

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='CAK8P3a1u3WnVAasCXG-sk=7cM2HCcitFWtXvn5hzCnpFU85Lxg@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=aklimov@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=dalias@libc.org \
    --cc=dennis@kernel.org \
    --cc=dsterba@suse.com \
    --cc=geert@linux-m68k.org \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=jianpeng.ma@intel.com \
    --cc=joe@perches.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=richard.weiyang@linux.alibaba.com \
    --cc=rikard.falkeborn@gmail.com \
    --cc=sbrivio@redhat.com \
    --cc=wsa+renesas@sang-engineering.com \
    --cc=ysato@users.sourceforge.jp \
    --cc=yury.norov@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.