linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [GIT PULL] locking fix
Date: Sun, 27 Oct 2013 12:35:21 -0700	[thread overview]
Message-ID: <CA+55aFwOvMQSxwpu3TYc06=87EKam3D7cZgv08jkkX1=4hwuqg@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFz3MLHcAFqF=WBwmHGQ+8+uU+Hd7avXt=P9L-ohv08bnw@mail.gmail.com>

On Sun, Oct 27, 2013 at 12:23 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> The *ONLY* thing it is testing for is "how much can the compiler
> optimize this", and as such the *ONLY* thing it tests for is compiler
> differences.

Side note: testing "can the compiler optimize this expression at
compile time" is actually sometimes an interesting question, so it can
be a valid thing to test.

But people should understand that the question is literally about THAT
(ie visibility into compiler optimization) rather than about the value
itself.

So generally, the only thing that a __builtin_constant_p() test can be
used for is in *conjunction* with having an actual test for an actual
value, and then having special-case logic that pertains to that value.

So for example, *this* is a valid test:

    if (__builtin_constant_p(ww_ctx) && ww_ctx == NULL) {
        ... special compile-time shortcut for the NULL value ..
    } else {
        ... generic code that *also* handles the NULL value ..
    }

and it's useful for triggering a compile-time optimized code-sequence
that is only true for NULL. But because __builtin_constant_p() is
about "how well can the compiler optimize this", that "else" statement
had better be able to handle the generic case too.

And yes, there are a few places where we do expect a certain minimal
set of optimizations. So in some cases we *might* have the rule that
the only valid use of NULL in a case like the above is when the
pointer passed in is passed in as a constant. And then we might say
"we rely on the compiler always returning true for
__builtin_constant_p(NULL)", and then we might say "so the "generic"
version of the code is guaranteed to never see NULL".

But notice how *different* that

    __builtin_constant_p(ww_ctx) && ww_ctx == NULL

test is from

    __builtin_constant_p(ww_ctx == NULL)

and really, the two tests are *fundamentally* really really different.
The first one can make sense. While the second one is pure and utter
garbage.

                          Linus

  reply	other threads:[~2013-10-27 19:35 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-26 12:19 [GIT PULL] locking fix Ingo Molnar
2013-10-27 17:28 ` Linus Torvalds
2013-10-27 19:00   ` Maarten Lankhorst
2013-10-27 19:23     ` Linus Torvalds
2013-10-27 19:35       ` Linus Torvalds [this message]
2013-10-27 19:37       ` Maarten Lankhorst
2013-10-27 19:51         ` Linus Torvalds
2013-10-27 19:56           ` Maarten Lankhorst
2013-10-27 19:59             ` Linus Torvalds
2013-10-28  8:47               ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2024-04-14  8:01 Ingo Molnar
2024-04-14 18:48 ` pr-tracker-bot
2023-11-26  9:39 Ingo Molnar
2023-11-26 17:16 ` pr-tracker-bot
2023-02-11  8:54 Ingo Molnar
2023-02-11 19:24 ` pr-tracker-bot
2021-03-28 10:28 Ingo Molnar
2021-03-28 19:22 ` pr-tracker-bot
2019-07-14 11:36 Ingo Molnar
2019-07-14 18:45 ` pr-tracker-bot
2019-05-16 16:01 Ingo Molnar
2019-05-16 17:57 ` Linus Torvalds
2019-05-16 18:39   ` Greg KH
2019-05-16 18:42     ` Linus Torvalds
2019-05-16 23:55       ` Sasha Levin
2019-05-17 12:16         ` Greg KH
2019-05-16 18:20 ` pr-tracker-bot
2019-04-12 11:53 Ingo Molnar
2019-04-13  4:05 ` pr-tracker-bot
2017-07-21 10:11 Ingo Molnar
2016-09-13 18:11 Ingo Molnar
2016-04-16  9:16 Ingo Molnar
2015-08-14  7:08 Ingo Molnar
2015-03-28 10:07 Ingo Molnar
2015-03-01 16:57 Ingo Molnar
2011-02-15 17:02 Ingo Molnar

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='CA+55aFwOvMQSxwpu3TYc06=87EKam3D7cZgv08jkkX1=4hwuqg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@canonical.com \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    /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).