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