[libcxx-commits] [libcxxabi] r357944 - Make reads and writes of the guard variable atomic.

[Eric Fiselier via libcxx-commits]

Author: ericwf
Date: Mon Apr  8 14:26:25 2019
New Revision: 357944

URL: http://llvm.org/viewvc/llvm-project?rev=357944&view=rev
Log:
Make reads and writes of the guard variable atomic.

The read of the guard variable by the caller is atomic,
and doesn't happen under a mutex.

Our internal reads and writes were non-atomic, because they happened
under a mutex.

The writes should always be atomic since they can be observed outside
of the lock.

Making the reads atomic is not strictly necessary under the current
global mutex approach, but will be under implementations that use a
futex (which I plan to land shortly). However, they should add little
additional cost.

Modified:
    libcxxabi/trunk/src/cxa_guard.cpp

Modified: libcxxabi/trunk/src/cxa_guard.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_guard.cpp?rev=357944&r1=357943&r2=357944&view=diff
==============================================================================
--- libcxxabi/trunk/src/cxa_guard.cpp (original)
+++ libcxxabi/trunk/src/cxa_guard.cpp Mon Apr  8 14:26:25 2019
@@ -9,6 +9,7 @@
 #include "__cxxabi_config.h"
 
 #include "abort_message.h"
+#include "include/atomic_support.h"
 #include <__threading_support>
 
 #include <stdint.h>
@@ -155,13 +156,18 @@ private:
 struct GuardObject {
   explicit GuardObject(guard_type *g) : guard(g) {}
 
-  // Read the current value of the guard object.
-  // TODO: Make this read atomic.
-  GuardValue read() const;
-
-  // Write the specified value to the guard object.
-  // TODO: Make this atomic
-  void write(GuardValue new_val);
+  /// Load the current value from the guard object.
+  GuardValue load() const;
+
+  /// Store the specified value in the guard object.
+  void store(GuardValue new_val);
+
+  /// Store the specified value in the guard object and return the previous value.
+  GuardValue exchange(GuardValue new_val);
+
+  /// Perform a atomic compare and exchange operation. Return true if
+  // desired is written to the guard object.
+  bool compare_exchange(GuardValue *expected, GuardValue desired);
 
 private:
   GuardObject(const GuardObject&) = delete;
@@ -178,7 +184,7 @@ extern "C"
 _LIBCXXABI_FUNC_VIS int __cxa_guard_acquire(guard_type* raw_guard_object) {
   GlobalMutexGuard gmutex("__cxa_guard_acquire", OnRelease::UNLOCK);
   GuardObject guard(raw_guard_object);
-  GuardValue current_value = guard.read();
+  GuardValue current_value = guard.load();
 
   if (current_value.is_initialization_complete())
     return INIT_COMPLETE;
@@ -192,12 +198,12 @@ _LIBCXXABI_FUNC_VIS int __cxa_guard_acqu
 #endif
   while (current_value.is_initialization_pending()) {
       gmutex.wait_for_signal();
-      current_value = guard.read();
+      current_value = guard.load();
   }
   if (current_value.is_initialization_complete())
     return INIT_COMPLETE;
 
-  guard.write(LOCK_ID);
+  guard.store(LOCK_ID);
   return INIT_NOT_COMPLETE;
 }
 
@@ -205,14 +211,14 @@ _LIBCXXABI_FUNC_VIS void __cxa_guard_rel
   GlobalMutexGuard gmutex("__cxa_guard_release",
                           OnRelease::UNLOCK_AND_BROADCAST);
   GuardObject guard(raw_guard_object);
-  guard.write(GuardValue::ZERO());
-  guard.write(GuardValue::INIT_COMPLETE());
+  guard.store(GuardValue::ZERO());
+  guard.store(GuardValue::INIT_COMPLETE());
 }
 
 _LIBCXXABI_FUNC_VIS void __cxa_guard_abort(guard_type *raw_guard_object) {
   GlobalMutexGuard gmutex("__cxa_guard_abort", OnRelease::UNLOCK);
   GuardObject guard(raw_guard_object);
-  guard.write(GuardValue::ZERO());
+  guard.store(GuardValue::ZERO());
 }
 }  // extern "C"
 
@@ -220,17 +226,26 @@ _LIBCXXABI_FUNC_VIS void __cxa_guard_abo
 //                        GuardObject Definitions
 //===----------------------------------------------------------------------===//
 
-GuardValue GuardObject::read() const {
-  // FIXME: Make this atomic
-  guard_type val = *guard;
-  return GuardValue(val);
+GuardValue GuardObject::load() const {
+  return GuardValue(std::__libcpp_atomic_load(guard));
+}
+
+void GuardObject::store(GuardValue new_val) {
+  std::__libcpp_atomic_store(guard, new_val.value);
+}
+
+GuardValue GuardObject::exchange(GuardValue new_val) {
+  return GuardValue(
+      std::__libcpp_atomic_exchange(guard, new_val.value, std::_AO_Acq_Rel));
 }
 
-void GuardObject::write(GuardValue new_val) {
-  // FIXME: make this atomic
-  *guard = new_val.value;
+bool GuardObject::compare_exchange(GuardValue* expected,
+                                   GuardValue desired) {
+  return std::__libcpp_atomic_compare_exchange(guard, &expected->value, desired.value,
+                                               std::_AO_Acq_Rel, std::_AO_Acquire);
 }
 
+
 //===----------------------------------------------------------------------===//
 //                        GuardValue Definitions
 //===----------------------------------------------------------------------===//