[Bug libstdc++/54786] New: Missing fence in std::atomic::store() triggers wrong reordering.

2012-10-02 Thread ozabluda at gmail dot com 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54786



 Bug #: 54786

   Summary: Missing fence in std::atomic::store() triggers

wrong reordering.

Classification: Unclassified

   Product: gcc

   Version: 4.6.3

Status: UNCONFIRMED

  Severity: major

  Priority: P3

 Component: libstdc++

AssignedTo: unassig...@gcc.gnu.org

ReportedBy: ozabl...@gmail.com

CC: hans_bo...@hp.com

  Host: x86_64-linux-gnu

Target: x86_64-linux-gnu

 Build: x86_64-linux-gnu





Created attachment 28335

  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28335

Preprocessed source



In the standard publish idiom:



#include 



double d;

std::atomic aflag(0);



void publish() {

   d = 1.0;

   aflag = 1;

}





store to d is reordered (sunk) after store to aflag:



_Z7publishv:

.LFB382:

.cfi_startproc

movabsq$4607182418800017408, %rax

movl$1, aflag(%rip) #<== store to aflag

movq%rax, d(%rip)   #<== store to d

mfence

ret





The reason for it is that in include/c++/4.6/bits/atomic_2.h store() is missing

needed __sync_synchronize() before non-atomic write:



  void

  store(__int_type __i, memory_order __m = memory_order_seq_cst)

  {

__glibcxx_assert(__m != memory_order_acquire);

__glibcxx_assert(__m != memory_order_acq_rel);

__glibcxx_assert(__m != memory_order_consume);



if (__m == memory_order_relaxed)

  _M_i = __i;

else

  {

// write_mem_barrier(); //OZ: Missing __sync_synchronize();  

_M_i = __i;

if (__m == memory_order_seq_cst)

  __sync_synchronize();

  }

  }





Incidentally, load() has unnecessary barrier before non-atomic read:



  __int_type

  load(memory_order __m = memory_order_seq_cst) const

  {

__glibcxx_assert(__m != memory_order_release);

__glibcxx_assert(__m != memory_order_acq_rel);



__sync_synchronize(); //OZ: unnecessary

__int_type __ret = _M_i;

__sync_synchronize();

return __ret;

  }



And by unnecessary, I don't mean just for "publish", but ever.







$ g++-4.6 -v -save-temps --std=c++0x -O2 -c reorder.cpp 

Using built-in specs.

COLLECT_GCC=g++-4.6

COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.6/lto-wrapper

Target: x86_64-linux-gnu

Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro

4.6.3-1ubuntu5' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs

--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr

--program-suffix=-4.6 --enable-shared --enable-linker-build-id

--with-system-zlib --libexecdir=/usr/lib --without-included-gettext

--enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6

--libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu

--enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object

--enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686

--with-tune=generic --enable-checking=release --build=x86_64-linux-gnu

--host=x86_64-linux-gnu --target=x86_64-linux-gnu

Thread model: posix

gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) 

COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++0x' '-O2' '-c' '-shared-libgcc'

'-mtune=generic' '-march=x86-64'

 /usr/lib/gcc/x86_64-linux-gnu/4.6/cc1plus -E -quiet -v -imultilib .

-imultiarch x86_64-linux-gnu -D_GNU_SOURCE reorder.cpp -mtune=generic

-march=x86-64 -std=c++0x -O2 -fpch-preprocess -fstack-protector -o reorder.ii

ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"

ignoring nonexistent directory

"/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../x86_64-linux-gnu/include"

#include "..." search starts here:

#include <...> search starts here:

 /usr/include/c++/4.6

 /usr/include/c++/4.6/x86_64-linux-gnu/.

 /usr/include/c++/4.6/backward

 /usr/lib/gcc/x86_64-linux-gnu/4.6/include

 /usr/local/include

 /usr/lib/gcc/x86_64-linux-gnu/4.6/include-fixed

 /usr/include/x86_64-linux-gnu

 /usr/include

End of search list.

COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++0x' '-O2' '-c' '-shared-libgcc'

'-mtune=generic' '-march=x86-64'

 /usr/lib/gcc/x86_64-linux-gnu/4.6/cc1plus -fpreprocessed reorder.ii -quiet

-dumpbase reorder.cpp -mtune=generic -march=x86-64 -auxbase reorder -O2

-std=c++0x -version -fstack-protector -o reorder.s

GNU C++ (Ubuntu/Linaro 4.6.3-1ubuntu5) version 4.6.3 (x86_64-linux-gnu)

compiled by GNU C version 4.6.3, GMP version 5.0.2, MPFR version 3.1.0-p3,

MPC version 0.9

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072

GNU C++ (Ubuntu/Linaro 4.6.3-1ubuntu5) version 4.6.3 (x86_64-linux-gnu)

compiled by GNU C version 4.6.3, GMP version 5.0.2, MPFR version 3.1.0-p3,

[Bug libstdc++/54786] Missing fence in std::atomic::store() triggers wrong reordering.

2012-10-02 Thread ozabluda at gmail dot com 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54786



--- Comment #3 from Oleg Zabluda  2012-10-02 
22:04:37 UTC ---

I can confirm that it is fixed in 4.7.0, after the move to __atomic builtins,

at least on x86_64. It would be nice to have it fixed in currently hypothetical

4.6.4, especially for those whose distro is stuck on 4.6. But at least it's

documented now and users can act accordingly. For example add

__sync_syncronise() or asm volatile ("":::"memory") before atomic::store().

[Bug c++/54861] New: std::atomic_signal_fence(std::memory_order_seq_cst) issues unnecessary mfence

2012-10-08 Thread ozabluda at gmail dot com 

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54861

 Bug #: 54861
   Summary: std::atomic_signal_fence(std::memory_order_seq_cst)
issues unnecessary mfence
Classification: Unclassified
   Product: gcc
   Version: 4.7.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
AssignedTo: unassig...@gcc.gnu.org
ReportedBy: ozabl...@gmail.com
  Host: x86_64-linux-gnu
Target: x86_64-linux-gnu
 Build: x86_64-linux-gnu


Created attachment 28394
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28394
Preprocessed source

#include 
void f() { std::atomic_signal_fence(std::memory_order_seq_cst); }

f():
.LFB321:
.cfi_startproc
mfence # <== unnecessary mfence
ret
.cfi_endproc

The reason is that std::atomic_signal_fence() (in bits/atomic_base.h) simply
calls std::atomic_thread_fence(), where mfence is indeed necessary.

C++11 29.8.8 says:

extern "C" void atomic_signal_fence(memory_order order) noexcept;

Effects: equivalent to atomic_thread_fence(order), except that the resulting
ordering constraints are established only between a thread and a signal handler
executed in the same thread.

8 Note: compiler optimizations and reorderings of loads and stores are
inhibited in the same way as with atomic_thread_fence, but the hardware fence
instructions that atomic_thread_fence would have inserted are not emitted.

C11 7.17.4.2 has an identical language.

===


$ g++-4.7 -v -save-temps --std=c++0x -O2 -c atomic_fences.cpp  
Using built-in specs.
COLLECT_GCC=g++-4.7
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro
4.7.0-7ubuntu3' --with-bugurl=file:///usr/share/doc/gcc-4.7/README.Bugs
--enable-languages=c,c++,go,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.7 --enable-shared --enable-linker-build-id
--with-system-zlib --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.7
--libdir=/usr/lib --enable-nls --disable-bootstrap --with-sysroot=/
--enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes
--enable-gnu-unique-object --enable-plugin --enable-objc-gc --disable-werror
--with-arch-32=i686 --with-tune=generic --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.7.0 (Ubuntu/Linaro 4.7.0-7ubuntu3) 
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++11' '-O2' '-c' '-shared-libgcc'
'-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/4.7/cc1plus -E -quiet -v -imultiarch
x86_64-linux-gnu -D_GNU_SOURCE atomic_fences.cpp -mtune=generic -march=x86-64
-std=c++11 -O2 -fpch-preprocess -fstack-protector -o atomic_fences.ii
ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"
ignoring nonexistent directory
"/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../x86_64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/include/c++/4.7
 /usr/include/c++/4.7/x86_64-linux-gnu
 /usr/include/c++/4.7/backward
 /usr/lib/gcc/x86_64-linux-gnu/4.7/include
 /usr/local/include
 /usr/lib/gcc/x86_64-linux-gnu/4.7/include-fixed
 /usr/include/x86_64-linux-gnu
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++11' '-O2' '-c' '-shared-libgcc'
'-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/4.7/cc1plus -fpreprocessed atomic_fences.ii
-quiet -dumpbase atomic_fences.cpp -mtune=generic -march=x86-64 -auxbase
atomic_fences -O2 -std=c++11 -version -fstack-protector -o atomic_fences.s
GNU C++ (Ubuntu/Linaro 4.7.0-7ubuntu3) version 4.7.0 (x86_64-linux-gnu)
compiled by GNU C version 4.6.3, GMP version 5.0.2, MPFR version 3.1.0-p3,
MPC version 0.9
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C++ (Ubuntu/Linaro 4.7.0-7ubuntu3) version 4.7.0 (x86_64-linux-gnu)
compiled by GNU C version 4.6.3, GMP version 5.0.2, MPFR version 3.1.0-p3,
MPC version 0.9
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 33e4f0c75e4f7f1ac5eae22180dee261
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++11' '-O2' '-c' '-shared-libgcc'
'-mtune=generic' '-march=x86-64'
 as --64 -o atomic_fences.o atomic_fences.s
COMPILER_PATH=/usr/lib/gcc/x86_64-linux-gnu/4.7/:/usr/lib/gcc/x86_64-linux-gnu/4.7/:/usr/lib/gcc/x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/4.7/:/usr/lib/gcc/x86_64-linux-gnu/
LIBRARY_PATH=/usr/lib/gcc/x86_64-linux-gnu/4.7/:/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../../lib/:/lib/x86_64-linux-gnu/:/lib/../lib/:/usr/lib/x86_64-linux-gnu/:/usr/lib/../lib/:/usr/lib/gcc/x86_64-linux-gnu/4.7/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-std=c++11' '-O2' '-c' '-shared-libgcc'
'-

[Bug libstdc++/54861] std::atomic_signal_fence(std::memory_order_seq_cst) issues unnecessary mfence

2012-10-09 Thread ozabluda at gmail dot com 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54861



--- Comment #3 from Oleg Zabluda  2012-10-09 
18:48:56 UTC ---

I can confirm that this fixes it in gcc 4.7.0.