[PATCH] Removing the static initializer in ManagedStatic.cpp by using llvm_call_once to initialize the ManagedStatic mutex.

[Chris Bieneman]

It sounds like David's complaints are performance related not correctness. Unfortunately his suggested implementation doesn't really cover the needs for this. The hand-rolled solution is basically filling in on Windows where we aren't guaranteed to have <mutex>, <thread>, or <atomic>.

There are definitely more performant ways to implement the hand rolled solution, but it should be safe as implemented. That said, I will make adjustments to the implementation and submit updated patches for review.

================
Comment at: include/llvm/Support/Threading.h:70
@@ +69,3 @@
+  // Visual Studio bug id #811192.
+  static volatile sys::cas_flag initialized = 0;
+  sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0);
----------------
majnemer wrote:
> `volatile` doesn't really make sense here.
Why not? Having this be volatile actually alleviates the possibility of the compiler optimizing away loads.

================
Comment at: include/llvm/Support/Threading.h:71
@@ +70,3 @@
+  static volatile sys::cas_flag initialized = 0;
+  sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0);
+  if (old_val == 0) {
----------------
majnemer wrote:
> This is very expensive, you are performing a RMW operation even if nothing has changed.
Sure. For performance this should only execute if not already initialized.

================
Comment at: include/llvm/Support/Threading.h:77-82
@@ +76,8 @@
+  } else {
+    sys::cas_flag tmp = initialized;
+    sys::MemoryFence();
+    while (tmp != 2) {
+      tmp = initialized;
+      sys::MemoryFence();
+    }
+  }
----------------
majnemer wrote:
> Let's assume that loading `initialized` will somehow turn into a relaxed load.  Why bother with the fence? You could just fence after:
>   while (initialized != 2);
>   sys::MemoryFence();
> 
> This will result in far, far fewer fences should this ever be contended.
The point of making initialized volatile is so that the compiler will emit a load for the while loop. If you're loading with each iteration of the loop is the fence really needed at all?

It shouldn't be for correctness.

http://reviews.llvm.org/D5922