[llvm] r258157 - [GC] Registry initialization and linkage interactions

[Philip Reames via llvm-commits]

Author: reames
Date: Tue Jan 19 12:34:27 2016
New Revision: 258157

URL: http://llvm.org/viewvc/llvm-project?rev=258157&view=rev
Log:
[GC] Registry initialization and linkage interactions

The Registry class constructs a linked list of nodes whose storage is inside static variables and nodes are added via static initializers. The trick is that those static initializers are in both the LLVM code base, and some random plugin that might get loaded in at runtime. The existing code tries to use C++ templates and their ODR rules to get a single definition of the registry for each type, but, experimentally, this doesn't quite work as designed. (Well, the entire structure doesn't. It might not actually be an ODR problem.)

Previously, when I tried moving the GCStrategy class (along with it's registry) from CodeGen to IR, I ran into a problem where asking the GCStrategyRegistry a question would return inconsistent results depending on whether you asked from CodeGen (where the static initializers still were) or Transforms. My best guess is that this is a result of either a) an order of initialization error, or b) we ended up with two copies of the registry being created. I remember at the time having convinced myself it was probably (b), but I don't have any of my notes around from that investigation any more.

See http://reviews.llvm.org/rL226311 for the original patch in question.

This patch tries to remove the possibility of (b) above. (a) was already fixed in change 258109.

Differential Revision: http://reviews.llvm.org/D16170



Modified:
    llvm/trunk/include/llvm/Support/Registry.h
    llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
    llvm/trunk/lib/CodeGen/GCStrategy.cpp

Modified: llvm/trunk/include/llvm/Support/Registry.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Registry.h?rev=258157&r1=258156&r2=258157&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Registry.h (original)
+++ llvm/trunk/include/llvm/Support/Registry.h Tue Jan 19 12:34:27 2016
@@ -116,14 +116,15 @@ namespace llvm {
     };
   };
 
-  
-  // Since these are defined in a header file, plugins must be sure to export
-  // these symbols.
-  template <typename T>
-  typename Registry<T>::node *Registry<T>::Head;
-
-  template <typename T>
-  typename Registry<T>::node *Registry<T>::Tail;
+  // Use this macro to stamp out definitions for required static symbols in an
+  // appropriate source file.  This is required to avoid getting multiple
+  // definitions of the Registry's fields in different translation or linkage
+  // units.   
+#define DEFINE_REGISTRY(T)                      \
+  template <>                                    \
+  typename Registry<T>::node *Registry<T>::Head = nullptr; \
+  template <>                          \
+  typename Registry<T>::node *Registry<T>::Tail = nullptr;
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_REGISTRY_H

Modified: llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp?rev=258157&r1=258156&r2=258157&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp (original)
+++ llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp Tue Jan 19 12:34:27 2016
@@ -17,3 +17,7 @@ using namespace llvm;
 GCMetadataPrinter::GCMetadataPrinter() {}
 
 GCMetadataPrinter::~GCMetadataPrinter() {}
+
+// Stamp out the registry of GCMetadataPrinter objects so that pulgins can load
+// new Strategies.  
+DEFINE_REGISTRY(GCMetadataPrinter)

Modified: llvm/trunk/lib/CodeGen/GCStrategy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GCStrategy.cpp?rev=258157&r1=258156&r2=258157&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GCStrategy.cpp (original)
+++ llvm/trunk/lib/CodeGen/GCStrategy.cpp Tue Jan 19 12:34:27 2016
@@ -20,3 +20,7 @@ GCStrategy::GCStrategy()
     : UseStatepoints(false), NeededSafePoints(0), CustomReadBarriers(false),
       CustomWriteBarriers(false), CustomRoots(false), InitRoots(true),
       UsesMetadata(false) {}
+
+// Stamp out the registry of GCStrategy objects so that pulgins can load new
+// Strategies.  
+DEFINE_REGISTRY(GCStrategy)