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

Chris Bieneman beanz at apple.com
Mon Oct 27 13:52:02 PDT 2014


A few notes.

According to Chandler's related RFC (http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-September/077081.html) we do not have a requirement that all platforms support <atomic>.

>>! In D5922#11, @majnemer wrote:
> I'm afraid not.  Because your implementation uses operations which are not atomic, it has undefined behavior in the eyes of C++11.
> 
> C++11 [intro.multithread]p21:
> The execution of a program contains a data race if it contains two conflicting actions in different threads, at least one of which is not atomic, and neither happens before the other. Any such data race results in undefined behavior.

I think you're misconstruing what this means. Your statement would seem to imply that until C++11 nobody ever managed to write thread safe code-- which is absurd. There are certainly undefined behaviors in the implementation I've provided, but they are known, and appropriately handled.

Both the MemFences, and the tmp in the else branch of the original patch can be safely removed, and the code can be re-written into a while loop for performance. An example implementation would be:

```
enum InitStatus {
  Done = -1,
  Uninitialized = 0,
  Wait = 1
};

void llvm::call_once(once_flag& flag, void (*UserFn)(void)) {
  while (flag != Done) {
    if (flag == Wait) {
      ::Sleep(1);
      continue;
    }

    sys::cas_flag old_val = sys::CompareAndSwap(&flag, Wait, Uninitialized);
    if (old_val == Uninitialized) {
      UserFn();
      sys::MemoryFence();
      flag = Done;
    }
  }
}
```

I (and some of my colleagues) believe that this is perfectly safe code, and it eliminates unnecessary MemFences (which you raised concerns about). Do you see a problem with this implementation (other than the fact that it doesn't use std::atomic)?

I don't have any particular opposition to using std::atomic, however since we don't have an enforced requirement I'm not sure we can assume we have it. Please keep in mind I think we still support deploying back to Windows XP.

http://reviews.llvm.org/D5922






More information about the llvm-commits mailing list