tcmalloc: Use C++11 atomics where appropriate.

third_party/tcmalloc/chromium/src/base/atomicops_internals_portable.h

 // Copyright (c) 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file is an internal atomic implementation, use atomicops.h instead.
 //
 // This implementation uses C++11 atomics' member functions. The code base is
 // currently written assuming atomicity revolves around accesses instead of
 // C++11's memory locations. The burden is on the programmer to ensure that all
 // memory locations accessed atomically are never accessed non-atomically (tsan
@@ skipping 15 matching lines @@
 //    implemented as sequentially consistent fence followed by a relaxed load.
 //  * Atomic increment is expected to return the post-incremented value, whereas
 //    C11 fetch add returns the previous value. The implementation therefore
 //    needs to increment twice (which the compiler should be able to detect and
 //    optimize).
 
 #ifndef BASE_ATOMICOPS_INTERNALS_PORTABLE_H_
 #define BASE_ATOMICOPS_INTERNALS_PORTABLE_H_
 
 #include <atomic>
+#include <stdint.h>
+
+typedef int32_t Atomic32;
+#define BASE_HAS_ATOMIC64 1  // Use only in tests and base/atomic*

[JF, 2015/12/26 00:27:24]

I'm not sure I understand the comment: isn't this force-defining the 64-bit
versions of atomic functions for all users?

[Nico, 2015/12/26 04:13:03]

It's like this in the x86 version this patch deleted, and it's used in
atomicops.h. I understood the comment to mean that you shouldn't check for this
define except in comments and (tcmalloc's) base/.

[JF, 2015/12/26 15:56:15]

Is it required though? If so, could we only support 64-bit atomics on x86-64?

I'd wary of supporting 64-bit atomics on non-64-bit platforms: older MIPS don't
have lock-free 64-bit atomics, and ARM has fairly expensive ones (they're
useful, but perf really needs to be measured when used).

[Nico, 2015/12/26 16:35:05]

In practice, there are three non-test uses of atomicops in tcmalloc
(https://code.google.com/p/chromium/codesearch#search/&q=atomicops.h%5C%22%20f...):

1. spinlock_internal uses an atomic32:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/tcmall...

2. vdso_support (only used on 32-bit linux and can probably be removed there
too) uses a call to MemoryBarrier:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/tcmall...

3. AtomicPtr uses an AtomicWord:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/tcmall...

So 64-bit atomics are only used on 64-bit platforms, via the third use (if that
code is life at all). tcmalloc doesn't change much, so I don't think this list
of uses will fluctuate a lot.

The old code used to define BASE_HAS_ATOMIC64 for 32-bit x86 too, so I figured
I'd do this too, and if this is defined then atomicops defines functions that
forward from the global namespace to base::subtle (with a TODO to remove these
forwarders). I could delete those, but I'd prefer if we didn't diverge more from
upstream tcmalloc than necessary.

[JF, 2015/12/26 16:58:37]

OK thanks for looking into it!

Yes, I think 2 can also be removed, but agree with you that less divergence is
good.

 
 namespace base {
 namespace subtle {
 
 // This implementation is transitional and maintains the original API for
 // atomicops.h. This requires casting memory locations to the atomic types, and
 // assumes that the API and the C++11 implementation are layout-compatible,
 // which isn't true for all implementations or hardware platforms. The static
 // assertion should detect this issue, were it to fire then this header
 // shouldn't be used.
@@ skipping 84 matching lines @@
 
 inline Atomic32 Acquire_Load(volatile const Atomic32* ptr) {
   return ((AtomicLocation32)ptr)->load(std::memory_order_acquire);
 }
 
 inline Atomic32 Release_Load(volatile const Atomic32* ptr) {
   MemoryBarrier();
   return ((AtomicLocation32)ptr)->load(std::memory_order_relaxed);
 }
 
-#if defined(ARCH_CPU_64_BITS)
+#if defined(BASE_HAS_ATOMIC64)
+typedef int64_t Atomic64;
 
 typedef volatile std::atomic<Atomic64>* AtomicLocation64;
 static_assert(sizeof(*(AtomicLocation64) nullptr) == sizeof(Atomic64),
               "incompatible 64-bit atomic layout");
 
 inline Atomic64 NoBarrier_CompareAndSwap(volatile Atomic64* ptr,
                                          Atomic64 old_value,
                                          Atomic64 new_value) {
   ((AtomicLocation64)ptr)
       ->compare_exchange_strong(old_value,
@@ skipping 61 matching lines @@
 }
 
 inline Atomic64 Acquire_Load(volatile const Atomic64* ptr) {
   return ((AtomicLocation64)ptr)->load(std::memory_order_acquire);
 }
 
 inline Atomic64 Release_Load(volatile const Atomic64* ptr) {
   MemoryBarrier();
   return ((AtomicLocation64)ptr)->load(std::memory_order_relaxed);
 }
-
-#endif  // defined(ARCH_CPU_64_BITS)
+#endif  // defined(BASE_HAS_ATOMIC64)
 }  // namespace subtle
 }  // namespace base
 
 #endif  // BASE_ATOMICOPS_INTERNALS_PORTABLE_H_