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

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 29 11:25:52 PDT 2016


On Wed, Jun 29, 2016 at 11:21 AM, Sean Silva <chisophugis at gmail.com> wrote:
> Nice! IIRC when Davide was profiling LTO these fences were actually showing
> up measurably in the profile!
>

Definitely, thanks!

> -- 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
>
>


More information about the llvm-commits mailing list