linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Segher Boessenkool <segher@kernel.crashing.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Will Deacon <will@kernel.org>,
	Andrea Parri <parri.andrea@gmail.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Nick Piggin <npiggin@gmail.com>,
	David Howells <dhowells@redhat.com>,
	Jade Alglave <j.alglave@ucl.ac.uk>,
	Luc Maranget <luc.maranget@inria.fr>,
	Akira Yokosawa <akiyks@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-toolchains@vger.kernel.org,
	linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [RFC] LKMM: Add volatile_if()
Date: Sun, 6 Jun 2021 14:41:50 -0400	[thread overview]
Message-ID: <20210606184150.GA1742067@rowland.harvard.edu> (raw)
In-Reply-To: <CAHk-=wgUsReyz4uFymB8mmpphuP0vQ3DktoWU_x4u6impbzphg@mail.gmail.com>

On Sat, Jun 05, 2021 at 08:41:00PM -0700, Linus Torvalds wrote:
> On Sat, Jun 5, 2021 at 6:29 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > Interesting.  And changing one of the branches from barrier() to __asm__
> > __volatile__("nop": : :"memory") also causes a branch to be emitted.  So
> > even though the compiler doesn't "look inside" assembly code, it does
> > compare two pieces at least textually and apparently assumes if they are
> > identical then they do the same thing.
> 
> That's actually a feature in some cases, ie the ability to do CSE on
> asm statements (ie the "always has the same output" optimization that
> the docs talk about).
> 
> So gcc has always looked at the asm string for that reason, afaik.
> 
> I think it's something of a bug when it comes to "asm volatile", but
> the documentation isn't exactly super-specific.
> 
> There is a statement of "Under certain circumstances, GCC may
> duplicate (or remove duplicates of) your assembly code when
> optimizing" and a suggestion of using "%=" to generate a unique
> instance of an asm.
> 
> Which might actually be a good idea for "barrier()", just in case.
> However, the problem with that is that I don't think we are guaranteed
> to have a universal comment character for asm statements.
> 
> IOW, it might be a good idea to do something like
> 
>    #define barrier() \
>         __asm__ __volatile__("# barrier %=": : :"memory")
> 
> but I'm  not 100% convinced that '#' is always a comment in asm code,
> so the above might not actually build everywhere.
> 
> However, *testing* the above (in my config, where '#' does work as a
> comment character) shows that gcc doesn't actually consider them to be
> distinct EVEN THEN, and will still merge two barrier statements.
> 
> That's distressing.
> 
> So the gcc docs are actively wrong, and %= does nothing - it will
> still compare as the exact same inline asm, because the string
> equality testing is apparently done before any expansion.
> 
> Something like this *does* seem to work:
> 
>    #define ____barrier(id) __asm__ __volatile__("#" #id: : :"memory")
>    #define __barrier(id) ____barrier(id)
>    #define barrier() __barrier(__COUNTER__)
> 
> which is "interesting" or "disgusting" depending on how you happen to feel.
> 
> And again - the above works only as long as "#" is a valid comment
> character in the assembler. And I have this very dim memory of us
> having comments in inline asm, and it breaking certain configurations
> (for when the assembler that the compiler uses is a special
> human-unfriendly one that only accepts compiler output).
> 
> You could make even more disgusting hacks, and have it generate something like
> 
>     .pushsection .discard.barrier
>     .long #id
>     .popsection
> 
> instead of a comment. We already expect that to work and have generic
> inline asm cases that generate code like that.

I tried the experiment with this code:

#define READ_ONCE(x) (*(volatile typeof(x) *)&(x))
#define WRITE_ONCE(x, val) (READ_ONCE(x) = (val))
#define barrier() __asm__ __volatile__("": : :"memory")

int x, y;

int main(int argc, char *argv[])
{
    if (READ_ONCE(x)) {
        barrier();
        y = 1;
    } else {
        y = 1;
    }
    return 0;
}

The output from gcc -O2 is:

main:
        mov     eax, DWORD PTR x[rip]
        test    eax, eax
        je      .L2
.L2:
        mov     DWORD PTR y[rip], 1

The output from clang is essentially the same (the mov and test are 
replaced by a cmp).

This does what we want, but I wouldn't bet against a future 
optimization pass getting rid of the "useless" test and branch.

Alan

  parent reply	other threads:[~2021-06-06 18:41 UTC|newest]

Thread overview: 127+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 10:12 [RFC] LKMM: Add volatile_if() Peter Zijlstra
2021-06-04 10:44 ` Will Deacon
2021-06-04 11:13   ` Will Deacon
2021-06-04 11:31   ` Peter Zijlstra
2021-06-04 13:44     ` Will Deacon
2021-06-04 13:56       ` Peter Zijlstra
2021-06-04 15:13         ` Will Deacon
2021-06-04 15:22           ` Peter Zijlstra
2021-06-04 15:36             ` Alan Stern
2021-06-04 15:42             ` Peter Zijlstra
2021-06-04 15:51               ` Alan Stern
2021-06-04 16:17                 ` Peter Zijlstra
2021-06-04 18:27                   ` Alan Stern
2021-06-04 19:09                     ` Linus Torvalds
2021-06-04 19:18                       ` Linus Torvalds
2021-06-04 20:56                         ` Paul E. McKenney
2021-06-04 21:27                           ` Linus Torvalds
2021-06-04 21:40                             ` Paul E. McKenney
2021-06-04 22:19                               ` Linus Torvalds
2021-06-05 14:57                                 ` Alan Stern
2021-06-06  0:14                                   ` Paul E. McKenney
2021-06-06  1:29                                     ` Alan Stern
2021-06-06  3:41                                       ` Linus Torvalds
2021-06-06  4:43                                         ` Paul E. McKenney
2021-06-06 13:17                                           ` Segher Boessenkool
2021-06-06 19:07                                             ` Paul E. McKenney
2021-06-06 12:59                                         ` Segher Boessenkool
2021-06-06 13:47                                           ` Alan Stern
2021-06-06 17:13                                             ` Segher Boessenkool
2021-06-06 18:25                                           ` Linus Torvalds
2021-06-06 19:19                                             ` Segher Boessenkool
2021-06-06 18:41                                         ` Alan Stern [this message]
2021-06-06 18:59                                         ` Jakub Jelinek
2021-06-06 19:15                                           ` Paul E. McKenney
2021-06-06 19:22                                           ` Linus Torvalds
2021-06-06 20:11                                             ` Segher Boessenkool
2021-06-06 21:19                                             ` Alexander Monakov
2021-06-06 22:38                                               ` Linus Torvalds
2021-06-06 23:39                                                 ` Rasmus Villemoes
2021-06-06 23:44                                                   ` Rasmus Villemoes
2021-06-07  8:01                                                 ` Alexander Monakov
2021-06-07  8:27                                                   ` Marco Elver
2021-06-07 15:28                                                     ` Paul E. McKenney
2021-06-07 17:04                                                       ` Marco Elver
2021-06-08  9:30                                                         ` Marco Elver
2021-06-08 11:22                                                           ` Peter Zijlstra
2021-06-08 15:28                                                             ` Segher Boessenkool
2021-06-09 12:44                                                               ` Marco Elver
2021-06-09 15:31                                                                 ` Segher Boessenkool
2021-06-09 16:13                                                                   ` Marco Elver
2021-06-09 17:14                                                                     ` Segher Boessenkool
2021-06-09 17:31                                                                       ` Nick Desaulniers
2021-06-09 20:24                                                                         ` Segher Boessenkool
2021-06-09 18:25                                                                     ` Linus Torvalds
2021-06-07 17:52                                                   ` Segher Boessenkool
2021-06-07 18:07                                                     ` Alexander Monakov
2021-06-07 18:18                                                       ` Segher Boessenkool
2021-06-07 17:42                                                 ` Segher Boessenkool
2021-06-07 20:31                                                   ` Linus Torvalds
2021-06-07 22:54                                                     ` Segher Boessenkool
2021-06-06 11:53                                       ` Segher Boessenkool
2021-06-06 13:45                                         ` Alan Stern
2021-06-06 18:04                                         ` Linus Torvalds
2021-06-06 18:22                                           ` Alan Stern
2021-06-06 18:43                                             ` Linus Torvalds
2021-06-07 10:43                                               ` Peter Zijlstra
2021-06-07 11:52                                                 ` Will Deacon
2021-06-07 15:25                                                   ` Paul E. McKenney
2021-06-07 16:02                                                     ` Will Deacon
2021-06-07 18:08                                                       ` Paul E. McKenney
     [not found]                                                   ` <20210730172020.GA32396@knuckles.cs.ucl.ac.uk>
2021-07-30 20:35                                                     ` Alan Stern
2021-08-02 21:18                                                     ` Alan Stern
2021-08-02 23:31                                                     ` Paul E. McKenney
2021-08-04 20:09                                                       ` Alan Stern
2021-08-05 19:47                                                     ` Alan Stern
2021-08-07  0:51                                                     ` Alan Stern
2021-06-06 18:40                                           ` Segher Boessenkool
2021-06-06 18:48                                             ` Linus Torvalds
2021-06-06 18:53                                               ` Linus Torvalds
2021-06-06 19:52                                               ` Segher Boessenkool
2021-06-06 20:11                                                 ` Linus Torvalds
2021-06-06 20:26                                                   ` Segher Boessenkool
2021-06-06 23:37                                                     ` Paul E. McKenney
2021-06-07 14:12                                                       ` Segher Boessenkool
2021-06-07 15:27                                                         ` Paul E. McKenney
2021-06-07 18:23                                                           ` Segher Boessenkool
2021-06-07 19:51                                                             ` Alan Stern
2021-06-07 20:16                                                               ` Paul E. McKenney
2021-06-07 22:40                                                                 ` Segher Boessenkool
2021-06-07 23:26                                                                   ` Paul E. McKenney
2021-06-07 10:52                                                     ` Peter Zijlstra
2021-06-07 14:16                                                       ` Segher Boessenkool
2021-06-04 22:05                             ` Peter Zijlstra
2021-06-05  3:14                       ` Alan Stern
2021-06-05 16:24                         ` Linus Torvalds
2021-06-04 15:50         ` Segher Boessenkool
2021-06-04 15:47     ` Segher Boessenkool
2021-06-04 11:44 ` Peter Zijlstra
2021-06-04 14:13   ` Paul E. McKenney
2021-06-04 15:35   ` Segher Boessenkool
2021-06-04 16:10     ` Peter Zijlstra
2021-06-04 16:40       ` Segher Boessenkool
2021-06-04 18:55         ` Paul E. McKenney
2021-06-04 19:53           ` Segher Boessenkool
2021-06-04 20:40             ` Paul E. McKenney
2021-06-06 11:36               ` Segher Boessenkool
2021-06-06 19:01                 ` Paul E. McKenney
2021-06-04 14:25 ` Alan Stern
2021-06-04 16:09 ` Segher Boessenkool
2021-06-04 16:33   ` Peter Zijlstra
2021-06-04 16:30 ` Linus Torvalds
2021-06-04 16:37   ` Peter Zijlstra
2021-06-04 16:52     ` Segher Boessenkool
2021-06-04 17:10     ` Linus Torvalds
2021-06-04 17:24       ` Segher Boessenkool
2021-06-04 17:38         ` Linus Torvalds
2021-06-04 18:25           ` Segher Boessenkool
2021-06-04 19:17         ` Peter Zijlstra
2021-06-04 20:43           ` Paul E. McKenney
2021-06-04 18:23       ` Alan Stern
2021-06-08 12:48 ` David Laight
2021-09-24 18:38 ` Mathieu Desnoyers
2021-09-24 19:52   ` Alan Stern
2021-09-24 20:22     ` Mathieu Desnoyers
2021-09-24 19:55   ` Segher Boessenkool
2021-09-24 20:39     ` Mathieu Desnoyers
2021-09-24 22:07       ` Mathieu Desnoyers

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=20210606184150.GA1742067@rowland.harvard.edu \
    --to=stern@rowland.harvard.edu \
    --cc=akiyks@gmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=npiggin@gmail.com \
    --cc=parri.andrea@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=segher@kernel.crashing.org \
    --cc=torvalds@linux-foundation.org \
    --cc=will@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
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).