[go: nahoru, domu]

Use C++11 atomics

This CL moves the Chrome code base to C++11 atomics instead of inline
assembly for the toolchains that support C++11's <atomic>. The patch is
constructed to modify as little code as possible, and will be followed-up with
other patches to clean up the rest of the code base.

This change should allow compilers to emit better code when dealing with atomics
(LLVM has recently seen substantial improvements thanks to morisset's work), be
more portable, eventually delete a bunch of code, and in subsequent changes
atomicity will be based on memory location instead of individual accesses (making
the code less error-prone, and easier to analyze).

This patch stems from a fix I recently made to V8 to build under NaCl and
PNaCl. This was gating V8's C++11 migration: they needed to move to the
LLVM-based NaCl toolchain which didn't work with atomicops' inline assembly. V8
currently uses the __sync_* primitives for the NaCl/PNaCl build, which imply
sequential consistency and are therefore suboptimal. Before doing further work I
want to fix Chrome's own headers, and trickle these changes to V8.

Future changes:
 * The atomicops headers are copied in 8 locations [0] in the Chrome code base,
   not all of them are up to date. Those should all be updated in their
   individual repositories (some are in thiry_party).
 * The signature of functions should be updated to take an atomic location
   instead of the pointer argument, and all call sites should be updated (there
   are current 127 such calls [1] in chromium/src in non-implementation and
   non-test files).
 * Atomic operations should be used directly.

A few things worth noting:
 * The atomicops_internals_portable.h file contains the main implementation, and
   has extra notes on implementation choices.
 * The CL removes x86 CPU features detection from the portable implementation:
    - Because the headers are copied in a few places and the x86 CPU feature
      detection uses a static global that parses cpuid, the
      atomicops_internals_x86_gcc.cc file was only built once, but its struct
      interface was declared external and used in protobuf and
      tcmalloc. Removing the struct from Chrome's own base makes the linker sad
      because of the two uses. This has two implications:
       # Don't specialize atomics for SSE2 versus non SSE2. This is a non-issue
         since Chrome requires a minimum of SSE2. The code is therefore faster
         (skip a branch).
       # The code doesn't detect the AMD Opteron Rev E memory barrier bug from
         AMD errata 147 [2]. This bug affects Opterons 1xx/2xx/8xx that shipped
         in 2003-2004. In general compilers should work around this bug, not
         library code. GCC has had this workaround since 2009 [3], but it is
         still an open bug in LLVM [4]. See comment #27 on the code review: this
         CPU is used by 0.02% of Chrome users, for whom the race would have to
         occur and the compiler not have the workaround for the bug to manifest.
         This also makes the code faster by skipping a branch.
 * The CL moves the declaration of the x86 CPU features struct to atomicops.h,
   and matches its fields with the ones duplicated across the code base. This
   is transitional: it fixes a link failure because tcmalloc relied on the
   x86_gcc.{h,cc} declaration and implementation, and it fixes an ODR violation
   where the struct didn't always have 3 members (and the other members were
   sometimes accessed, uninitialized).
 * tsan already rewrites all atomics to its own primitives, its header is
   therefore now unnecessary. The implementation takes care to detect and error
   out if that turns out not to be true.
 * MemoryBarrier works around a libstdc++ bug in older versions [5]. The
   workaround is exactly the non-buggy code for libstdc++ (call out to the
   compiler's builtin).
 * The assembly generated by GCC 4.8 and LLVM 3.6 for x86-32/x86-64/ARM when
   using this portable implementation can be found in [6].

[0]: find . -name "atomicops.h"
  ./base/atomicops.h
  ./v8/src/base/atomicops.h
  ./native_client_sdk/src/libraries/sdk_util/atomicops.h
  ./third_party/webrtc/base/atomicops.h
  ./third_party/tcmalloc/chromium/src/base/atomicops.h
  ./third_party/tcmalloc/vendor/src/base/atomicops.h
  ./third_party/re2/util/atomicops.h
  ./third_party/protobuf/src/google/protobuf/stubs/atomicops.h
[1]: git grep Barrier_ | grep -v atomicops | grep -v unittest | wc -l
[2]: http://support.amd.com/us/Processor_TechDocs/25759.pdf
[3]: https://gcc.gnu.org/ml/gcc-patches/2009-10/txt00046.txt
[4]: http://llvm.org/bugs/show_bug.cgi?id=5934
[5]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51038
[6]: https://code.google.com/p/chromium/issues/detail?id=423074

TEST=ninja -C out/Release base_unittests
TEST=trybots
BUG=246514
BUG=234215
BUG=420970
BUG=423074
R= dvyukov@google.com, thakis@chromium.org, glider@chromium.org, hboehm@google.com, morisset@google.com, willchan@chromium.org

Review URL: https://codereview.chromium.org/636783002

Cr-Commit-Position: refs/heads/master@{#299485}
diff --git a/base/atomicops.h b/base/atomicops.h
index 84be8c0..833e1704 100644
--- a/base/atomicops.h
+++ b/base/atomicops.h
@@ -28,8 +28,11 @@
 #ifndef BASE_ATOMICOPS_H_
 #define BASE_ATOMICOPS_H_
 
+#include <cassert>  // Small C++ header which defines implementation specific
+                    // macros used to identify the STL implementation.
 #include <stdint.h>
 
+#include "base/base_export.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN) && defined(ARCH_CPU_64_BITS)
@@ -137,28 +140,66 @@
 }  // namespace subtle
 }  // namespace base
 
-// Include our platform specific implementation.
-#if defined(THREAD_SANITIZER)
-#include "base/atomicops_internals_tsan.h"
-#elif defined(OS_WIN) && defined(COMPILER_MSVC) && defined(ARCH_CPU_X86_FAMILY)
-#include "base/atomicops_internals_x86_msvc.h"
-#elif defined(OS_MACOSX)
-#include "base/atomicops_internals_mac.h"
-#elif defined(OS_NACL)
-#include "base/atomicops_internals_gcc.h"
-#elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARMEL)
-#include "base/atomicops_internals_arm_gcc.h"
-#elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARM64)
-#include "base/atomicops_internals_arm64_gcc.h"
-#elif defined(COMPILER_GCC) && defined(ARCH_CPU_X86_FAMILY)
-#include "base/atomicops_internals_x86_gcc.h"
-#elif defined(COMPILER_GCC) && \
-      (defined(ARCH_CPU_MIPS_FAMILY) || defined(ARCH_CPU_MIPS64_FAMILY))
-#include "base/atomicops_internals_mips_gcc.h"
-#else
-#error "Atomic operations are not supported on your platform"
+// The following x86 CPU features are used in atomicops_internals_x86_gcc.h, but
+// this file is duplicated inside of Chrome: protobuf and tcmalloc rely on the
+// struct being present at link time. Some parts of Chrome can currently use the
+// portable interface whereas others still use GCC one. The include guards are
+// the same as in atomicops_internals_x86_gcc.cc.
+#if defined(__i386__) || defined(__x86_64__)
+// This struct is not part of the public API of this module; clients may not
+// use it.  (However, it's exported via BASE_EXPORT because clients implicitly
+// do use it at link time by inlining these functions.)
+// Features of this x86.  Values may not be correct before main() is run,
+// but are set conservatively.
+struct AtomicOps_x86CPUFeatureStruct {
+  bool has_amd_lock_mb_bug; // Processor has AMD memory-barrier bug; do lfence
+                            // after acquire compare-and-swap.
+  // The following fields are unused by Chrome's base implementation but are
+  // still used by copies of the same code in other parts of the code base. This
+  // causes an ODR violation, and the other code is likely reading invalid
+  // memory.
+  // TODO(jfb) Delete these fields once the rest of the Chrome code base doesn't
+  //           depend on them.
+  bool has_sse2;            // Processor has SSE2.
+  bool has_cmpxchg16b;      // Processor supports cmpxchg16b instruction.
+};
+BASE_EXPORT extern struct AtomicOps_x86CPUFeatureStruct
+    AtomicOps_Internalx86CPUFeatures;
 #endif
 
+// Try to use a portable implementation based on C++11 atomics.
+//
+// Some toolchains support C++11 language features without supporting library
+// features (recent compiler, older STL). Whitelist libstdc++ and libc++ that we
+// know will have <atomic> when compiling C++11.
+#if ((__cplusplus >= 201103L) &&                            \
+     ((defined(__GLIBCXX__) && (__GLIBCXX__ > 20110216)) || \
+      (defined(_LIBCPP_VERSION) && (_LIBCPP_STD_VER >= 11))))
+#  include "base/atomicops_internals_portable.h"
+#else  // Otherwise use a platform specific implementation.
+#  if defined(THREAD_SANITIZER)
+#    error "Thread sanitizer must use the portable atomic operations"
+#  elif (defined(OS_WIN) && defined(COMPILER_MSVC) && \
+         defined(ARCH_CPU_X86_FAMILY))
+#    include "base/atomicops_internals_x86_msvc.h"
+#  elif defined(OS_MACOSX)
+#    include "base/atomicops_internals_mac.h"
+#  elif defined(OS_NACL)
+#    include "base/atomicops_internals_gcc.h"
+#  elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARMEL)
+#    include "base/atomicops_internals_arm_gcc.h"
+#  elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARM64)
+#    include "base/atomicops_internals_arm64_gcc.h"
+#  elif defined(COMPILER_GCC) && defined(ARCH_CPU_X86_FAMILY)
+#    include "base/atomicops_internals_x86_gcc.h"
+#  elif (defined(COMPILER_GCC) && \
+         (defined(ARCH_CPU_MIPS_FAMILY) || defined(ARCH_CPU_MIPS64_FAMILY)))
+#    include "base/atomicops_internals_mips_gcc.h"
+#  else
+#    error "Atomic operations are not supported on your platform"
+#  endif
+#endif   // Portable / non-portable includes.
+
 // On some platforms we need additional declarations to make
 // AtomicWord compatible with our other Atomic* types.
 #if defined(OS_MACOSX) || defined(OS_OPENBSD)