[PATCH 1/2] server: Always place compiler barrier before atomic memory stores.

[Rémi Bernon]

On 2/10/22 18:18, Jinoh Kang wrote:
> GCC, clang, and other compilers do not actually provide acquire/release
> semantics on volatile memory accesses.  This is also true on MSVC with
> the /volatile:iso (use strict ISO C semantics for volatile accesses)
> flag.
> 
> Consider the following test program:
> 
>      void func(int *foo, volatile int *bar)
>      {
>          *foo = 1;
>          *bar = 2;  /* NOTE: *not* immune to reordering! */
>          *foo = 3;
>      }
> 
> After store reordering and dead code removal, the function above
> compiles into the following x86-64 assembly on GCC 11.2 (gcc -O2):
> 
>      movl $2, (%rsi)
>      movl $3, (%rdi)
>      ret
> 
> Note that the first write to "*foo" has been ellided; the compiler
> decided that it is safe to reorder writes to "*foo" around writes to the
> volatile variable "*bar", so it simply merged the first "*foo" write
> into the second one.
> 
> Fix this by explicitly specifying a compiler memory barrier before the
> atomic store.  As a workaround for GCC bug #81316, we do this even for
> other architectures where we explicitly use the atomic memory access
> builtin.
> 
FWIW I believe that the only thing that matters for these helpers and 
where they are used (to write to user shared data section) is that the 
ordering is guaranteed between the volatile stores, not really with 
other load or stores on non-volatile data around them. This should be 
the case.

Then maybe they shouldn't be named atomic, because they aren't. Or, if 
we really want to fix the semantics here, maybe we should just use the 
__atomic intrinsics everywhere. The concern was the compatibility with 
older GCC or non-GCC compilers, and I don't think they are much less 
portable than inline assembly at this point.

-- 
Rémi Bernon <rbernon at codeweavers.com>