[PATCH] Removing the static destructor from ManagedStatic.cpp by controlling the allocation and de-allocaation of the mutex.

Chris Bieneman beanz at apple.com
Wed Oct 8 16:52:05 PDT 2014


I'm working on patch revisions, but I do have some comments inline below.

================
Comment at: configure:9628
@@ -9627,1 +9627,3 @@
 
+  { echo "$as_me:$LINENO: checking for library containing pthread_once" >&5
+echo $ECHO_N "checking for library containing pthread_once... $ECHO_C" >&6; }
----------------
rnk wrote:
> Did you modify autoconf/configure.ac to produce this, or write it out yourself based on the pthread_mutex_init code above? We should keep our changes in the .ac file and regenerate this with the appropriate autoconf. Eric can do that if you don't have the right bits.
I'll have Eric review these patches. My autoconf foo is severely lacking.

================
Comment at: include/llvm/Support/Threading.h:18
@@ -17,1 +17,3 @@
 
+#include "llvm/Config/config.h"
+
----------------
rnk wrote:
> We can only include llvm/Config/config.h from .cpp files, not .h files.
Really? We do include it in headers like Unix.h and WindowsSupport.h. I'm not really sure how to implement this without access to the HAVE_PTHREAD_ONCE define.

================
Comment at: include/llvm/Support/Threading.h:24
@@ +23,3 @@
+
+#define LLVM_CALL_ONCE(function)                                               \
+  {                                                                            \
----------------
rnk wrote:
> We take a function pointer as a template parameter instead of using macros. We'll get different static data members / static locals between different instantiations that way, which is what the macros appear to be solving.
This would absolutely be a cleaner way to do this.

================
Comment at: include/llvm/Support/Threading.h:47
@@ +46,3 @@
+  {                                                                            \
+    static volatile sys::cas_flag initialized = 0;                             \
+    sys::cas_flag old_val = sys::CompareAndSwap(&initialized, 1, 0);           \
----------------
rnk wrote:
> I don't think we want to commit this double checked locking. I think we can simplify this to:
> 
>   if Windows:
>     use InitOnceExecuteOnce in lib/Support/Windows/Threading.inc
>   else:
>     use a C++11 thread-safe static local in lib/Support/Unix/Threading.inc
I'm not sure this is the right breakdown. I believe MSVC has std::call_once, so I was hoping to do a breakdown based on the availability of <mutex>, and pthread_once.

http://reviews.llvm.org/D5473






More information about the llvm-commits mailing list