[llvm] r274131 - [ManagedStatic] Reimplement double-checked locking with std::atomic.

[Sean Silva via llvm-commits]

Nice! IIRC when Davide was profiling LTO these fences were actually showing
up measurably in the profile!

-- Sean Silva

On Wed, Jun 29, 2016 at 8:04 AM, Benjamin Kramer via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: d0k
> Date: Wed Jun 29 10:04:07 2016
> New Revision: 274131
>
> URL: http://llvm.org/viewvc/llvm-project?rev=274131&view=rev
> Log:
> [ManagedStatic] Reimplement double-checked locking with std::atomic.
>
> This gets rid of the memory fence in the hot path (dereferencing the
> ManagedStatic), trading for an extra mutex lock in the cold path (when
> the ManagedStatic was uninitialized). Since this only happens on the
> first accesses it shouldn't matter much. On strict architectures like
> x86 this removes any atomic instructions from the hot path.
>
> Also remove the tsan annotations, tsan knows how standard atomics work
> so they should be unnecessary now.
>
> Modified:
>     llvm/trunk/include/llvm/Support/ManagedStatic.h
>     llvm/trunk/lib/Support/ManagedStatic.cpp
>
> Modified: llvm/trunk/include/llvm/Support/ManagedStatic.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ManagedStatic.h?rev=274131&r1=274130&r2=274131&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/ManagedStatic.h (original)
> +++ llvm/trunk/include/llvm/Support/ManagedStatic.h Wed Jun 29 10:04:07
> 2016
> @@ -14,9 +14,9 @@
>  #ifndef LLVM_SUPPORT_MANAGEDSTATIC_H
>  #define LLVM_SUPPORT_MANAGEDSTATIC_H
>
> -#include "llvm/Support/Atomic.h"
>  #include "llvm/Support/Compiler.h"
> -#include "llvm/Support/Threading.h"
> +#include <atomic>
> +#include <cstddef>
>
>  namespace llvm {
>
> @@ -41,7 +41,7 @@ class ManagedStaticBase {
>  protected:
>    // This should only be used as a static variable, which guarantees that
> this
>    // will be zero initialized.
> -  mutable void *Ptr;
> +  mutable std::atomic<void *> Ptr;
>    mutable void (*DeleterFn)(void*);
>    mutable const ManagedStaticBase *Next;
>
> @@ -61,40 +61,26 @@ public:
>  template<class C>
>  class ManagedStatic : public ManagedStaticBase {
>  public:
> -
>    // Accessors.
>    C &operator*() {
> -    void* tmp = Ptr;
> -    if (llvm_is_multithreaded()) sys::MemoryFence();
> -    if (!tmp) RegisterManagedStatic(object_creator<C>,
> object_deleter<C>::call);
> -    TsanHappensAfter(this);
> +    void *Tmp = Ptr.load(std::memory_order_acquire);
> +    if (!Tmp)
> +      RegisterManagedStatic(object_creator<C>, object_deleter<C>::call);
>
> -    return *static_cast<C*>(Ptr);
> +    return *static_cast<C *>(Ptr.load(std::memory_order_relaxed));
>    }
> -  C *operator->() {
> -    void* tmp = Ptr;
> -    if (llvm_is_multithreaded()) sys::MemoryFence();
> -    if (!tmp) RegisterManagedStatic(object_creator<C>,
> object_deleter<C>::call);
> -    TsanHappensAfter(this);
>
> -    return static_cast<C*>(Ptr);
> -  }
> +  C *operator->() { return &**this; }
> +
>    const C &operator*() const {
> -    void* tmp = Ptr;
> -    if (llvm_is_multithreaded()) sys::MemoryFence();
> -    if (!tmp) RegisterManagedStatic(object_creator<C>,
> object_deleter<C>::call);
> -    TsanHappensAfter(this);
> +    void *Tmp = Ptr.load(std::memory_order_acquire);
> +    if (!Tmp)
> +      RegisterManagedStatic(object_creator<C>, object_deleter<C>::call);
>
> -    return *static_cast<C*>(Ptr);
> +    return *static_cast<C *>(Ptr.load(std::memory_order_relaxed));
>    }
> -  const C *operator->() const {
> -    void* tmp = Ptr;
> -    if (llvm_is_multithreaded()) sys::MemoryFence();
> -    if (!tmp) RegisterManagedStatic(object_creator<C>,
> object_deleter<C>::call);
> -    TsanHappensAfter(this);
>
> -    return static_cast<C*>(Ptr);
> -  }
> +  const C *operator->() const { return &**this; }
>  };
>
>  /// llvm_shutdown - Deallocate and destroy all ManagedStatic variables.
>
> Modified: llvm/trunk/lib/Support/ManagedStatic.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/ManagedStatic.cpp?rev=274131&r1=274130&r2=274131&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/ManagedStatic.cpp (original)
> +++ llvm/trunk/lib/Support/ManagedStatic.cpp Wed Jun 29 10:04:07 2016
> @@ -13,7 +13,6 @@
>
>  #include "llvm/Support/ManagedStatic.h"
>  #include "llvm/Config/config.h"
> -#include "llvm/Support/Atomic.h"
>  #include "llvm/Support/Mutex.h"
>  #include "llvm/Support/MutexGuard.h"
>  #include "llvm/Support/Threading.h"
> @@ -42,18 +41,10 @@ void ManagedStaticBase::RegisterManagedS
>    if (llvm_is_multithreaded()) {
>      MutexGuard Lock(*getManagedStaticMutex());
>
> -    if (!Ptr) {
> -      void* tmp = Creator();
> +    if (!Ptr.load(std::memory_order_relaxed)) {
> +      void *Tmp = Creator();
>
> -      TsanHappensBefore(this);
> -      sys::MemoryFence();
> -
> -      // This write is racy against the first read in the ManagedStatic
> -      // accessors. The race is benign because it does a second read
> after a
> -      // memory fence, at which point it isn't possible to get a partial
> value.
> -      TsanIgnoreWritesBegin();
> -      Ptr = tmp;
> -      TsanIgnoreWritesEnd();
> +      Ptr.store(Tmp, std::memory_order_release);
>        DeleterFn = Deleter;
>
>        // Add to list of managed statics.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160629/a56b04f3/attachment.html>