Compiler level memory barrier in Go i.e. asm volatile("" : : : "memory")

[tsuna]

Hi,
I have to implement Go support for an existing system that uses shared memory.

There are various places in the code where I have a loop in which I
need to read some sort of a uint32 version number from shared memory,
then read some more stuff, and read the version number again, and
ensure it didn’t change (if it did, I loop again).

I need to make sure the compiler doesn’t reorder the code as it
optimizes things, so that the version number is indeed read before
what follows.

In the existing C++ implementation, we use asm volatile("" : : :
"memory”) to ask the compiler to not reorder things, but I found
nothing equivalent to this in Go.

--
Benoit "tsuna" Sigoure

[Dmitry Vyukov]

Use sync/atomic.LoadUint32.

[tsuna]

I already use it. Why does it act like a compiler memory barrier?
Because it’s a native function?

I don’t know if the compiler is smart enough to see the ASM
implementation in sync/atomic/asm_amd64.s, but if it was, what would
prevent it from optimizing things further in the backend and reorder
stuff such that instead of having:

for {
version := atomic.LoadUint32(..)
foo()
newVersion := atomic.LoadUint32(..)
if version == newVersion {
break
}
}

The code could potentially be reordered as:

for {
foo()
version := atomic.LoadUint32(..)
newVersion := atomic.LoadUint32(..)
if version == newVersion {
break
}
}

--
Benoit "tsuna" Sigoure

[dan....@3smobile.com]

As I understand it, the Go memory model doesn't permit that kind of re-ordering around sync.atomic stuff. If it did, many uses of sync.atomic would break, including in Go and it's libraries itself.

[Ian Lance Taylor]

On Feb 7, 2015 12:40 PM, "tsuna" <tsun...@gmail.com> wrote:
>
> On Sat, Feb 7, 2015 at 12:13 PM, Dmitry Vyukov <dvy...@google.com> wrote:
> > On Sat, Feb 7, 2015 at 11:11 PM, tsuna <tsun...@gmail.com> wrote:
> >> Hi,
> >> I have to implement Go support for an existing system that uses shared memory.
> >>
> >> There are various places in the code where I have a loop in which I
> >> need to read some sort of a uint32 version number from shared memory,
> >> then read some more stuff, and read the version number again, and
> >> ensure it didn’t change (if it did, I loop again).
> >>
> >> I need to make sure the compiler doesn’t reorder the code as it
> >> optimizes things, so that the version number is indeed read before
> >> what follows.
> >>
> >> In the existing C++ implementation, we use asm volatile("" : : :
> >> "memory”) to ask the compiler to not reorder things, but I found
> >> nothing equivalent to this in Go.
> >
> >
> > Use sync/atomic.LoadUint32.
>
> I already use it.  Why does it act like a compiler memory barrier?

It has to, or the runtime package wouldn't work as it must.

The sync/atomic functions probably ought to be in the memory model, but we're tired of arguing about the memory model doc.

Ian

[John Souvestre]

Ø  It has to, or the runtime package wouldn't work as it must.

Ø  The sync/atomic functions probably ought to be in the memory model, but we're tired of arguing about the memory model doc.

You are saying that Go itself, and programs which use sync/atomic functions, would fail if it is not true.  Yet it’s not guaranteed (documented)?

 

If I understand issue 5045 correctly, it would seem that the argument against the guarantee has been superseded (resolved?) by implementation.  So why not now document this essential and often-questioned issue?

 

John

    John Souvestre - New Orleans LA

 

From: golan...@googlegroups.com [mailto:golan...@googlegroups.com] On Behalf Of Ian Lance Taylor
Sent: 2015 February 07, Sat 21:27
To: tsuna
Cc: golang-nuts; Dmitry Vyukov
Subject: Re: [go-nuts] Compiler level memory barrier in Go i.e. asm volatile("" : : : "memory")

 

[Rob Pike]

The sync/atomic functions are mentioned in the memory model doc.

-rob

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[John Souvestre]

It doesn’t say which primitives in sync/atomic are sequentially consistent / memory barriers.  Perhaps if it were clearer about this then it wouldn’t confuse so many users and even Go developers?

 

John

    John Souvestre - New Orleans LA

 

[Rob Pike]

The documentation of sync/atomic is simple and clear.

-rob

[tsuna]

Follow up question, just to be sure:

Do we then agree that if I see code like this:

for {
version := shmem.version // this reads a uint32 from shared memory
doSomething()
if version == shmem.version {
break
}
}

then this code is buggy because the Go compiler could re-order the
code or factor out the subexpression “shmem.version”, or the CPU could
even potentially execute things out of order? i.e. the only way to
make this code correct is to atomic.LoadUint32(&shmem.version), right?

I think until recently the Go compiler wasn’t optimizing much (before
the SSA work started, was gc doing any kind of common subexpression
elimination or whatnot?), but now that this is changing (fast), I’m
finding more code following this pattern and I want to be 100% certain
I can say “this is buggy, we need to change this”.

--
Benoit "tsuna" Sigoure

[Ian Lance Taylor]

On Fri, Nov 4, 2016 at 3:01 PM, tsuna <tsun...@gmail.com> wrote:
>
> Follow up question, just to be sure:
>
> Do we then agree that if I see code like this:
>
> for {
> version := shmem.version // this reads a uint32 from shared memory
> doSomething()
> if version == shmem.version {
> break
> }
> }
>
> then this code is buggy because the Go compiler could re-order the
> code or factor out the subexpression “shmem.version”, or the CPU could
> even potentially execute things out of order? i.e. the only way to
> make this code correct is to atomic.LoadUint32(&shmem.version), right?

Right (and of course modify shmem.version using atomic.StoreUint32).

> I think until recently the Go compiler wasn’t optimizing much (before
> the SSA work started, was gc doing any kind of common subexpression
> elimination or whatnot?), but now that this is changing (fast), I’m
> finding more code following this pattern and I want to be 100% certain
> I can say “this is buggy, we need to change this”.

Note that it's not just the compiler. As I was saying on another
thread earlier today, it's also the memory ordering of the processor.
Without using the atomic functions, nothing ensures that that the core
reading shmem.version will also see the other values written into
memory before shmem.version was written. In this case that may not
matter, but often it does.

Ian

[John Souvestre]

+1

Please note my new email address: Jo...@Souvestre.com

John

John Souvestre - New Orleans LA

-----Original Message-----
From: Ian Lance Taylor [mailto:ia...@golang.org]
Sent: 2016 November 04, Fri 17:09
To: tsuna
Cc: Rob Pike; John Souvestre; golang-nuts; Dmitry Vyukov
Subject: Re: [go-nuts] Compiler level memory barrier in Go i.e. asm volatile("" : : : "memory")