Move ownership of GCStrategy objects to LLVMContext

Note: This change ended up being slightly more controversial than expected.  Chandler has tentatively okayed this for the moment, but I may be revisiting this in the near future after we settle some high level questions.

Rather than have the GCStrategy object owned by the GCModuleInfo - which is an immutable analysis pass used mainly by gc.root - have it be owned by the LLVMContext. This simplifies the ownership logic (i.e. can you have two instances of the same strategy at once?), but more importantly, allows us to access the GCStrategy in the middle end optimizer. To this end, I add an accessor through Function which becomes the canonical way to get at a GCStrategy instance.

In the near future, this will allows me to move some of the checks from http://reviews.llvm.org/D6808 into the Verifier itself, and to introduce optimization legality predicates for some of the recent additions to InstCombine. (These will follow as separate changes.)

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

llvm-svn: 226311

llvm/include/llvm/CodeGen/GCMetadata.h

@@ -36,26 +36,15 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/DebugLoc.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/Pass.h"
 #include <memory>
 
 namespace llvm {
   class AsmPrinter;
-  class GCStrategy;
   class Constant;
   class MCSymbol;
 
-  namespace GC {
-    /// PointKind - The type of a collector-safe point.
-    ///
-    enum PointKind {
-      Loop,    ///< Instr is a loop (backwards branch).
-      Return,  ///< Instr is a return instruction.
-      PreCall, ///< Instr is a call instruction.
-      PostCall ///< Instr is the return address of a call.
-    };
-  }
-
   /// GCPoint - Metadata for a collector-safe point in machine code.
   ///
   struct GCPoint {
@@ -163,14 +152,9 @@ namespace llvm {
   /// Records both the function level information used by GCRoots and a
   /// cache of the 'active' gc strategy objects for the current Module.
   class GCModuleInfo : public ImmutablePass {
-    typedef StringMap<GCStrategy*> strategy_map_type;
-    typedef std::vector<std::unique_ptr<GCStrategy>> list_type;
-
-    strategy_map_type StrategyMap;
-    list_type StrategyList;
-
-    GCStrategy *getOrCreateStrategy(const Module *M, const std::string &Name);
-
+    /// A list of GCStrategies which are active in this Module.  These are
+    /// not owning pointers.
+    std::vector<GCStrategy*> StrategyList;
   public:
     /// List of per function info objects.  In theory, Each of these
     /// may be associated with a different GC.
@@ -190,7 +174,7 @@ namespace llvm {
     finfo_map_type FInfoMap;
   public:
 
-    typedef list_type::const_iterator iterator;
+    typedef std::vector<GCStrategy*>::const_iterator iterator;
 
     static char ID;
 

llvm/include/llvm/CodeGen/GCMetadataPrinter.h

@@ -21,7 +21,7 @@
 #define LLVM_CODEGEN_GCMETADATAPRINTER_H
 
 #include "llvm/CodeGen/GCMetadata.h"
-#include "llvm/CodeGen/GCStrategy.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/Support/Registry.h"
 
 namespace llvm {

llvm/include/llvm/IR/Function.h

@@ -29,6 +29,7 @@
 namespace llvm {
 
 class FunctionType;
+class GCStrategy;  
 class LLVMContext;
 
 // Traits for intrusive list of basic blocks...
@@ -225,6 +226,10 @@ public:
   void setGC(const char *Str);
   void clearGC();
 
+  /// Returns the GCStrategy associated with the specified garbage collector
+  /// algorithm or nullptr if one is not set.
+  GCStrategy *getGCStrategy() const;
+
   /// @brief adds the attribute to the list of attributes.
   void addAttribute(unsigned i, Attribute::AttrKind attr);
 

llvm/include/llvm/IR/GCStrategy.h

@@ -1,4 +1,4 @@
-//===-- llvm/CodeGen/GCStrategy.h - Garbage collection ----------*- C++ -*-===//
+//===-- llvm/IR/GCStrategy.h - Garbage collection ----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -47,25 +47,39 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_CODEGEN_GCSTRATEGY_H
-#define LLVM_CODEGEN_GCSTRATEGY_H
+#ifndef LLVM_IR_GCSTRATEGY_H
+#define LLVM_IR_GCSTRATEGY_H
 
 #include "llvm/ADT/Optional.h"
-#include "llvm/CodeGen/GCMetadata.h"
-#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/IR/Value.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Registry.h"
 #include <string>
 
 namespace llvm {
+  namespace GC {
+    /// PointKind - The type of a collector-safe point.
+    ///
+    enum PointKind {
+      Loop,    ///< Instr is a loop (backwards branch).
+      Return,  ///< Instr is a return instruction.
+      PreCall, ///< Instr is a call instruction.
+      PostCall ///< Instr is the return address of a call.
+    };
+  }
+
+  
   /// GCStrategy describes a garbage collector algorithm's code generation
   /// requirements, and provides overridable hooks for those needs which cannot
-  /// be abstractly described.  GCStrategy objects currently must be looked up
-  /// through the GCModuleInfo analysis pass.  They are owned by the analysis
-  /// pass and recreated every time that pass is invalidated.
+  /// be abstractly described.  GCStrategy objects must be looked up through
+  /// the Function.  The objects themselves are owned by the Context and must
+  /// be immutable.
   class GCStrategy {
   private:
     std::string Name;
-    friend class GCModuleInfo;
+    friend class LLVMContextImpl;
     
   protected:
     bool UseStatepoints;       /// Uses gc.statepoints as opposed to gc.roots,

llvm/include/llvm/Support/Registry.h

@@ -120,6 +120,10 @@ namespace llvm {
     static iterator begin() { return iterator(Head); }
     static iterator end()   { return iterator(nullptr); }
 
+    static iterator_range<iterator> entries() {
+      return iterator_range<iterator>(begin(), end());
+    }
+
 
     /// Abstract base class for registry listeners, which are informed when new
     /// entries are added to the registry. Simply subclass and instantiate:

llvm/lib/CodeGen/CMakeLists.txt

@@ -23,7 +23,6 @@ add_llvm_library(LLVMCodeGen
   GCMetadata.cpp
   GCMetadataPrinter.cpp
   GCRootLowering.cpp
-  GCStrategy.cpp
   GlobalMerge.cpp
   IfConversion.cpp
   InlineSpiller.cpp

llvm/lib/CodeGen/ErlangGC.cpp

@@ -15,8 +15,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GCs.h"
-#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Target/TargetInstrInfo.h"

llvm/lib/CodeGen/GCMetadata.cpp

@@ -12,10 +12,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GCMetadata.h"
-#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Debug.h"
@@ -61,27 +61,6 @@ GCModuleInfo::GCModuleInfo()
   initializeGCModuleInfoPass(*PassRegistry::getPassRegistry());
 }
 
-GCStrategy *GCModuleInfo::getOrCreateStrategy(const Module *M,
-                                              const std::string &Name) {
-  strategy_map_type::iterator NMI = StrategyMap.find(Name);
-  if (NMI != StrategyMap.end())
-    return NMI->getValue();
-  
-  for (GCRegistry::iterator I = GCRegistry::begin(),
-                            E = GCRegistry::end(); I != E; ++I) {
-    if (Name == I->getName()) {
-      std::unique_ptr<GCStrategy> S = I->instantiate();
-      S->Name = Name;
-      StrategyMap[Name] = S.get();
-      StrategyList.push_back(std::move(S));
-      return StrategyList.back().get();
-    }
-  }
- 
-  dbgs() << "unsupported GC: " << Name << "\n";
-  llvm_unreachable(nullptr);
-}
-
 GCFunctionInfo &GCModuleInfo::getFunctionInfo(const Function &F) {
   assert(!F.isDeclaration() && "Can only get GCFunctionInfo for a definition!");
   assert(F.hasGC());
@@ -90,7 +69,15 @@ GCFunctionInfo &GCModuleInfo::getFunctionInfo(const Function &F) {
   if (I != FInfoMap.end())
     return *I->second;
   
-  GCStrategy *S = getOrCreateStrategy(F.getParent(), F.getGC());
+  GCStrategy *S = F.getGCStrategy();
+  if (!S) {
+    std::string error = std::string("unsupported GC: ") + F.getGC();
+    report_fatal_error(error);
+  }
+  // Save the fact this strategy is associated with this module.  Note that
+  // these are non-owning references, the GCStrategy remains owned by the
+  // Context. 
+  StrategyList.push_back(S);
   Functions.push_back(make_unique<GCFunctionInfo>(F, *S));
   GCFunctionInfo *GFI = Functions.back().get();
   FInfoMap[&F] = GFI;
@@ -100,7 +87,6 @@ GCFunctionInfo &GCModuleInfo::getFunctionInfo(const Function &F) {
 void GCModuleInfo::clear() {
   Functions.clear();
   FInfoMap.clear();
-  StrategyMap.clear();
   StrategyList.clear();
 }
 

llvm/lib/CodeGen/GCRootLowering.cpp

@@ -11,13 +11,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/CodeGen/GCStrategy.h"
+#include "llvm/CodeGen/GCMetadata.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/Debug.h"

llvm/lib/CodeGen/OcamlGC.cpp

@@ -15,7 +15,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GCs.h"
-#include "llvm/CodeGen/GCStrategy.h"
+#include "llvm/IR/GCStrategy.h"
 
 using namespace llvm;
 

llvm/lib/CodeGen/Passes.cpp

@@ -14,7 +14,6 @@
 
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/Analysis/Passes.h"
-#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/IR/IRPrintingPasses.h"

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

@@ -26,7 +26,6 @@
 #include "llvm/CodeGen/FastISel.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
 #include "llvm/CodeGen/GCMetadata.h"
-#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
@@ -41,6 +40,7 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instructions.h"

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

@@ -24,7 +24,6 @@
 #include "llvm/CodeGen/FastISel.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
 #include "llvm/CodeGen/GCMetadata.h"
-#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
@@ -36,6 +35,7 @@
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"

llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp

@@ -16,11 +16,12 @@
 #include "SelectionDAGBuilder.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/CodeGen/GCMetadata.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
-#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/CodeGen/SelectionDAG.h"
 #include "llvm/CodeGen/StackMaps.h"
 #include "llvm/IR/CallingConv.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"

llvm/lib/CodeGen/ShadowStackGC.cpp

@@ -27,8 +27,8 @@
 
 #include "llvm/CodeGen/GCs.h"
 #include "llvm/ADT/StringExtras.h"
-#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/IR/CallSite.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"

llvm/lib/CodeGen/StatepointExampleGC.cpp

@@ -16,7 +16,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/CodeGen/GCStrategy.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Value.h"
 

llvm/lib/IR/CMakeLists.txt

@@ -17,6 +17,7 @@ add_llvm_library(LLVMCore
   Dominators.cpp
   Function.cpp
   GCOV.cpp
+  GCStrategy.cpp
   GVMaterializer.cpp
   Globals.cpp
   IRBuilder.cpp

llvm/lib/IR/Function.cpp

@@ -386,6 +386,12 @@ void Function::clearGC() {
   }
 }
 
+GCStrategy *Function::getGCStrategy() const {
+  // Lookup the GCStrategy (which is owned by the Context), given the name of
+  // the GC in question.
+  return getContext().pImpl->getGCStrategy(getGC());
+}
+
 /// copyAttributesFrom - copy all additional attributes (those not needed to
 /// create a Function) from the Function Src to this one.
 void Function::copyAttributesFrom(const GlobalValue *Src) {

llvm/lib/IR/GCStrategy.cpp

@@ -12,7 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/CodeGen/GCStrategy.h"
+#include "llvm/IR/GCStrategy.h"
 
 using namespace llvm;
 

llvm/lib/IR/LLVMContextImpl.cpp

@@ -15,6 +15,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/GCStrategy.h"
 #include "llvm/IR/Module.h"
 #include <algorithm>
 using namespace llvm;
@@ -182,3 +183,26 @@ void InsertValueConstantExpr::anchor() { }
 void GetElementPtrConstantExpr::anchor() { }
 
 void CompareConstantExpr::anchor() { }
+
+GCStrategy *LLVMContextImpl::getGCStrategy(const StringRef Name) {
+  // TODO: Arguably, just doing a linear search would be faster for small N
+  auto NMI = GCStrategyMap.find(Name);
+  if (NMI != GCStrategyMap.end())
+    return NMI->getValue();
+  
+  for (auto& Entry : GCRegistry::entries()) {
+    if (Name == Entry.getName()) {
+      std::unique_ptr<GCStrategy> S = Entry.instantiate();
+      S->Name = Name;
+      GCStrategyMap[Name] = S.get();
+      GCStrategyList.push_back(std::move(S));
+      return GCStrategyList.back().get();
+    }
+  }
+
+  // No GCStrategy found for that name, error reporting is the job of our
+  // callers. 
+  return nullptr;
+}
+
+

llvm/lib/IR/LLVMContextImpl.h

@@ -41,6 +41,7 @@ class ConstantFP;
 class DiagnosticInfoOptimizationRemark;
 class DiagnosticInfoOptimizationRemarkMissed;
 class DiagnosticInfoOptimizationRemarkAnalysis;
+class GCStrategy;
 class LLVMContext;
 class Type;
 class Value;
@@ -389,6 +390,17 @@ public:
 
   int getOrAddScopeRecordIdxEntry(MDNode *N, int ExistingIdx);
   int getOrAddScopeInlinedAtIdxEntry(MDNode *Scope, MDNode *IA,int ExistingIdx);
+
+  /// An owning list of all GCStrategies which have been created
+  SmallVector<std::unique_ptr<GCStrategy>, 1> GCStrategyList;
+  /// A helper map to speedup lookups into the above list
+  StringMap<GCStrategy*> GCStrategyMap;
+
+  /// Lookup the GCStrategy object associated with the given gc name.  If one
+  /// can't be found, returns nullptr.  The lifetime of the returned objects
+  /// is dictated by the lifetime of the associated context.  No caller should
+  /// attempt to delete the returned objects.
+  GCStrategy *getGCStrategy(const StringRef Name);
   
   LLVMContextImpl(LLVMContext &C);
   ~LLVMContextImpl();