linux-toolchains.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: David Malcolm <dmalcolm@redhat.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	linux-toolchains@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Jason Baron <jbaron@akamai.com>,
	"Steven Rostedt (VMware)" <rostedt@goodmis.org>
Subject: Re: static_branch/jump_label vs branch merging
Date: Fri, 9 Apr 2021 20:40:00 +0200	[thread overview]
Message-ID: <YHCfgHwDtT7m4ffq@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <3c062f70ffef2dcd48a661f7c8162fb8fbaf6869.camel@redhat.com>

On Fri, Apr 09, 2021 at 09:48:33AM -0400, David Malcolm wrote:
> You tried __pure on arch_static_branch; did you try it on
> static_branch_unlikely?

static_branch_unlikely() is a CPP macro that expands to a statement
expression, or as with the later patch, a _Generic(). I'm not sure how
to apply the attribute to either of them since it is a function
attribute.

I was hoping the attribute would percolate through, so to speak.

> With the caveat that my knowledge of GCC's middle-end is mostly about
> implementing warnings, rather than optimization, I did some
> experimentation, with gcc trunk on x86_64 FWIW.
> 
> Given:
> 
> int __attribute__((pure)) foo(void);
> 
> int t(void)
> {
>   int a;
>   if (foo())
>     a++;
>   if (foo())
>     a++;
>   if (foo())
>     a++;
>   return a;
> }
> 
> At -O1 and above this is optimized to a single call to foo, returning 0
> or 3 accordingly.
> 
> -fdump-tree-all shows that it's the "fre1" pass that eliminates the
> subsequent calls to foo, replacing them with reuses of the result of
> the first call.
> 
> This is in gcc/tree-ssa-sccvn.c, a value-numbering pass.
> 
> I think you want to somehow "teach" the compiler that:
>   static_branch_unlikely(&sched_schedstats)
> is "pure-ish", that for some portion of the surrounding code that you
> want the result to be treated as pure - though I suspect compiler
> maintainers with more experience than me are thinking "but which
> portion? what is it safe to assume, and what will users be annoyed
> about if we optimize away? what if t itself is inlined somewhere?" and
> similar concerns.

Right, pure or even const. As to the scope, as wide as possible. It
literally is a global constant, the value returned is the same
everywhere.

All we need GCC to do for the static_branch construct is to emit both
branches; that is, it must not treat the result as a constant and elide
the other branches. But it can consider consecutive calls (as far and
wide as it wants) to return the same value.

> Or maybe the asm stmt itself could somehow be marked as pure??? (with
> similar concerns about semantics as above)

Yeah, not sure, someone with more clue will have to inform us what, if
anything more than marking it either pure or const is required. Perhaps
that attribute is sufficient and the compiler just isn't optimizing for
an unrelated reason.

Regards,

Peter

  reply	other threads:[~2021-04-09 18:40 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-08 16:52 static_branch/jump_label vs branch merging Peter Zijlstra
2021-04-09  9:57 ` Ard Biesheuvel
2021-04-09 10:55   ` Florian Weimer
2021-04-09 11:16     ` Peter Zijlstra
2021-04-09 19:33       ` Nick Desaulniers
2021-04-09 20:11         ` Peter Zijlstra
2021-04-10 17:02         ` Segher Boessenkool
2021-04-09 11:12   ` Peter Zijlstra
2021-04-09 11:55     ` David Malcolm
2021-04-09 12:03       ` Peter Zijlstra
2021-04-09 13:01         ` Peter Zijlstra
2021-04-09 13:13           ` Peter Zijlstra
2021-04-09 13:48             ` David Malcolm
2021-04-09 18:40               ` Peter Zijlstra [this message]
2021-04-09 19:21                 ` David Malcolm
2021-04-09 20:09                   ` Peter Zijlstra
2021-04-09 21:07                     ` David Malcolm
2021-04-09 21:39                       ` Peter Zijlstra
2021-04-22 11:48                         ` Peter Zijlstra
2021-04-22 17:08                           ` Segher Boessenkool
2021-04-22 17:49                             ` Peter Zijlstra
2021-04-22 18:31                               ` Segher Boessenkool
2021-04-26 17:13                                 ` Peter Zijlstra
2021-04-10 12:44               ` David Laight
2021-04-09 13:03 ` Segher Boessenkool

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=YHCfgHwDtT7m4ffq@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=ardb@kernel.org \
    --cc=dmalcolm@redhat.com \
    --cc=jbaron@akamai.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=rostedt@goodmis.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).