GCStrategy should not own GCFunctionInfo

[reames]

Summary

This change moves the ownership and access of GCFunctionInfo (the object which describes the safepoints associated with a safepoint under GCRoot) to GCModuleInfo. Previously, this was owned by GCStrategy which was in turned owned by GCModuleInfo. This made GCStrategy module specific which is 'surprising' given it's name and other purposes.

There's a few more changes needed, but we're getting towards the point we can reuse GCStrategy for gc.statepoint as well.

p.s. The style of this code ends up being a mess. I was trying to move code around without otherwise changing much. Once I get the ownership structure rearranged, I will go through and fixup spacing, naming, comments etc.

[whitequark]

LGTM

llvm/trunk/include/llvm/CodeGen/GCMetadata.h

public:
 
     /// live_begin/live_end - Iterators for live roots at a given safe point.
     ///
     live_iterator live_begin(const iterator &p) { return roots_begin(); }
     live_iterator live_end  (const iterator &p) { return roots_end();   }
     size_t live_size(const iterator &p) const { return roots_size(); }
   };
 
-
   /// An analysis pass which caches information about the entire Module.
   /// 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;
-    typedef DenseMap<const Function*,GCFunctionInfo*> finfo_map_type;
 
     strategy_map_type StrategyMap;
     list_type StrategyList;
-    finfo_map_type FInfoMap;
 
     GCStrategy *getOrCreateStrategy(const Module *M, const std::string &Name);
 
   public:
+    /// List of per function info objects.  In theory, Each of these
+    /// may be associated with a different GC.
+    typedef std::vector<std::unique_ptr<GCFunctionInfo>> FuncInfoVec;
+
+    FuncInfoVec::iterator funcinfo_begin() { return Functions.begin(); }
+    FuncInfoVec::iterator funcinfo_end()   { return Functions.end();   }
+    
+
+  private:
+    /// Owning list of all GCFunctionInfos associated with this Module
+    FuncInfoVec Functions;
+
+    /// Non-owning map to bypass linear search when finding the GCFunctionInfo
+    /// associated with a particular Function.
+    typedef DenseMap<const Function*,GCFunctionInfo*> finfo_map_type;
+    finfo_map_type FInfoMap;
+  public:
+
     typedef list_type::const_iterator iterator;
 
     static char ID;
 
     GCModuleInfo();
 
     /// clear - Resets the pass. Any pass, which uses GCModuleInfo, should
     /// call it in doFinalization().
     ///
     void clear();
 
     /// begin/end - Iterators for used strategies.
     ///
     iterator begin() const { return StrategyList.begin(); }
     iterator end()   const { return StrategyList.end();   }
 
-    /// get - Look up function metadata.
-    ///
+    /// get - Look up function metadata.  This is currently assumed
+    /// have the side effect of initializing the associated GCStrategy.  That
+    /// will soon change.
     GCFunctionInfo &getFunctionInfo(const Function &F);
   };
 
 }
 
 #endif

llvm/trunk/include/llvm/CodeGen/GCMetadataPrinter.h

 namespace llvm {
 
   class GCMetadataPrinter;
 
   /// GCMetadataPrinterRegistry - The GC assembly printer registry uses all the
   /// defaults from Registry.
   typedef Registry<GCMetadataPrinter> GCMetadataPrinterRegistry;
 
-  /// GCMetadataPrinter - Emits GC metadata as assembly code.
-  ///
+  /// GCMetadataPrinter - Emits GC metadata as assembly code.  Instances are
+  /// created, managed, and owned by the AsmPrinter.
   class GCMetadataPrinter {
-  public:
-    typedef GCStrategy::list_type list_type;
-    typedef GCStrategy::iterator iterator;
-
   private:
     GCStrategy *S;
-
     friend class AsmPrinter;
 
   protected:
     // May only be subclassed.
     GCMetadataPrinter();
 
   private:
     GCMetadataPrinter(const GCMetadataPrinter &) LLVM_DELETED_FUNCTION;
     GCMetadataPrinter &
       operator=(const GCMetadataPrinter &) LLVM_DELETED_FUNCTION;
 
   public:
     GCStrategy &getStrategy() { return *S; }
 
-    /// begin/end - Iterate over the collected function metadata.
-    iterator begin() { return S->begin(); }
-    iterator end()   { return S->end();   }
-
-    /// beginAssembly/finishAssembly - Emit module metadata as assembly code.
-    virtual void beginAssembly(Module &M, AsmPrinter &AP);
-    virtual void finishAssembly(Module &M, AsmPrinter &AP);
+    /// Called before the assembly for the module is generated by
+    /// the AsmPrinter (but after target specific hooks.)
+    virtual void beginAssembly(Module &M, GCModuleInfo &Info,
+                               AsmPrinter &AP) {}
+    /// Called after the assembly for the module is generated by
+    /// the AsmPrinter (but before target specific hooks)
+    virtual void finishAssembly(Module &M, GCModuleInfo &Info,
+                                AsmPrinter &AP) {}
 
     virtual ~GCMetadataPrinter();
   };
 
 }
 
 #endif

llvm/trunk/include/llvm/CodeGen/GCStrategy.h

namespace llvm {
   typedef Registry<GCStrategy> GCRegistry;
   
   /// 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.
   class GCStrategy {
-  public:
-    typedef std::vector<std::unique_ptr<GCFunctionInfo>> list_type;
-    typedef list_type::iterator iterator;
-    
   private:
     friend class GCModuleInfo;
     std::string Name;
     
-    list_type Functions;
-    
   protected:
     unsigned NeededSafePoints; ///< Bitmask of required safe points.
     bool CustomReadBarriers;   ///< Default is to insert loads.
     bool CustomWriteBarriers;  ///< Default is to insert stores.
     bool CustomRoots;          ///< Default is to pass through to backend.
     bool CustomSafePoints;     ///< Default is to use NeededSafePoints
                                ///< to find safe points.
     bool InitRoots;            ///< If set, roots are nulled during lowering.
public:
     //                    allocas to null before the first use. This is
     //                    necessary for most GCs and is enabled by default.
     bool initializeRoots() const { return InitRoots; }
     
     /// usesMetadata - If set, appropriate metadata tables must be emitted by
     ///                the back-end (assembler, JIT, or otherwise).
     bool usesMetadata() const { return UsesMetadata; }
     
-    /// begin/end - Iterators for function metadata.
-    /// 
-    iterator begin() { return Functions.begin(); }
-    iterator end()   { return Functions.end();   }
-
-    /// insertFunctionMetadata - Creates metadata for a function.
-    /// 
-    GCFunctionInfo *insertFunctionInfo(const Function &F);
-
     /// initializeCustomLowering/performCustomLowering - If any of the actions
     /// are set to custom, performCustomLowering must be overriden to transform
     /// the corresponding actions to LLVM IR. initializeCustomLowering is
     /// optional to override. These are the only GCStrategy methods through
     /// which the LLVM IR can be modified.
     virtual bool initializeCustomLowering(Module &F);
     virtual bool performCustomLowering(Function &F);
     virtual bool findCustomSafePoints(GCFunctionInfo& FI, MachineFunction& MF);
   };
   
 }
 
 #endif

llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

if (MAI->hasSingleParameterDotFile()) {
     // .file "foo.c"
     OutStreamer.EmitFileDirective(M.getModuleIdentifier());
   }
 
   GCModuleInfo *MI = getAnalysisIfAvailable<GCModuleInfo>();
   assert(MI && "AsmPrinter didn't require GCModuleInfo?");
   for (auto &I : *MI)
     if (GCMetadataPrinter *MP = GetOrCreateGCPrinter(*I))
-      MP->beginAssembly(M, *this);
+      MP->beginAssembly(M, *MI, *this);
 
   // Emit module-level inline asm if it exists.
   if (!M.getModuleInlineAsm().empty()) {
     OutStreamer.AddComment("Start of file scope inline assembly");
     OutStreamer.AddBlankLine();
     EmitInlineAsm(M.getModuleInlineAsm()+"\n");
     OutStreamer.AddComment("End of file scope inline assembly");
     OutStreamer.AddBlankLine();
for (const auto &Alias : M.aliases()) {
     // Emit the directives as assignments aka .set:
     OutStreamer.EmitAssignment(Name, lowerConstant(Alias.getAliasee(), *this));
   }
 
   GCModuleInfo *MI = getAnalysisIfAvailable<GCModuleInfo>();
   assert(MI && "AsmPrinter didn't require GCModuleInfo?");
   for (GCModuleInfo::iterator I = MI->end(), E = MI->begin(); I != E; )
     if (GCMetadataPrinter *MP = GetOrCreateGCPrinter(**--I))
-      MP->finishAssembly(M, *this);
+      MP->finishAssembly(M, *MI, *this);
 
   // Emit llvm.ident metadata in an '.ident' directive.
   EmitModuleIdents(M);
 
   // If we don't have any trampolines, then we don't require stack memory
   // to be executable. Some targets have a directive to declare this.
   Function *InitTrampolineIntrinsic = M.getFunction("llvm.init.trampoline");
   if (!InitTrampolineIntrinsic || InitTrampolineIntrinsic->use_empty())

llvm/trunk/lib/CodeGen/AsmPrinter/ErlangGCPrinter.cpp

 #include "llvm/Target/TargetSubtargetInfo.h"
 
 using namespace llvm;
 
 namespace {
 
   class ErlangGCPrinter : public GCMetadataPrinter {
   public:
-    void beginAssembly(Module &M, AsmPrinter &AP) override;
-    void finishAssembly(Module &M, AsmPrinter &AP) override;
+    void finishAssembly(Module &M, GCModuleInfo &Info,
+                        AsmPrinter &AP) override;
   };
 
 }
 
 static GCMetadataPrinterRegistry::Add<ErlangGCPrinter>
 X("erlang", "erlang-compatible garbage collector");
 
 void llvm::linkErlangGCPrinter() { }
 
-void ErlangGCPrinter::beginAssembly(Module &M, AsmPrinter &AP) { }
-
-void ErlangGCPrinter::finishAssembly(Module &M, AsmPrinter &AP) {
+void ErlangGCPrinter::finishAssembly(Module &M, GCModuleInfo &Info,
+                                     AsmPrinter &AP) {
   MCStreamer &OS = AP.OutStreamer;
   unsigned IntPtrSize =
       AP.TM.getSubtargetImpl()->getDataLayout()->getPointerSize();
 
   // Put this in a custom .note section.
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getContext()
     .getELFSection(".note.gc", ELF::SHT_PROGBITS, 0,
                    SectionKind::getDataRel()));
 
   // For each function...
-  for (iterator FI = begin(), FE = end(); FI != FE; ++FI) {
+  for (GCModuleInfo::FuncInfoVec::iterator FI = Info.funcinfo_begin(),
+         IE = Info.funcinfo_end();
+       FI != IE; ++FI) {
     GCFunctionInfo &MD = **FI;
-
+    if (MD.getStrategy().getName() != getStrategy().getName())
+      // this function is managed by some other GC
+      continue;
     /** A compact GC layout. Emit this data structure:
      *
      * struct {
      *   int16_t PointCount;
      *   void *SafePointAddress[PointCount];
      *   int16_t StackFrameSize; (in words)
      *   int16_t StackArity;
      *   int16_t LiveCount;

llvm/trunk/lib/CodeGen/AsmPrinter/OcamlGCPrinter.cpp

 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <cctype>
 using namespace llvm;
 
 namespace {
 
   class OcamlGCMetadataPrinter : public GCMetadataPrinter {
   public:
-    void beginAssembly(Module &M, AsmPrinter &AP) override;
-    void finishAssembly(Module &M, AsmPrinter &AP) override;
+    void beginAssembly(Module &M, GCModuleInfo &Info,
+                       AsmPrinter &AP) override;
+    void finishAssembly(Module &M, GCModuleInfo &Info,
+                        AsmPrinter &AP) override;
   };
 
 }
 
 static GCMetadataPrinterRegistry::Add<OcamlGCMetadataPrinter>
 Y("ocaml", "ocaml 3.10-compatible collector");
 
 void llvm::linkOcamlGCPrinter() { }
static void EmitCamlGlobal(const Module &M, AsmPrinter &AP, const char *Id) {
   AP.Mang->getNameWithPrefix(TmpStr, SymName);
 
   MCSymbol *Sym = AP.OutContext.GetOrCreateSymbol(TmpStr);
 
   AP.OutStreamer.EmitSymbolAttribute(Sym, MCSA_Global);
   AP.OutStreamer.EmitLabel(Sym);
 }
 
-void OcamlGCMetadataPrinter::beginAssembly(Module &M, AsmPrinter &AP) {
+void OcamlGCMetadataPrinter::beginAssembly(Module &M, GCModuleInfo &Info,
+                                           AsmPrinter &AP) {
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getTextSection());
   EmitCamlGlobal(M, AP, "code_begin");
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getDataSection());
   EmitCamlGlobal(M, AP, "data_begin");
 }
 
 /// emitAssembly - Print the frametable. The ocaml frametable format is thus:
 ///
 ///   extern "C" struct align(sizeof(intptr_t)) {
 ///     uint16_t NumDescriptors;
 ///     struct align(sizeof(intptr_t)) {
 ///       void *ReturnAddress;
 ///       uint16_t FrameSize;
 ///       uint16_t NumLiveOffsets;
 ///       uint16_t LiveOffsets[NumLiveOffsets];
 ///     } Descriptors[NumDescriptors];
 ///   } caml${module}__frametable;
 ///
 /// Note that this precludes programs from stack frames larger than 64K
 /// (FrameSize and LiveOffsets would overflow). FrameTablePrinter will abort if
 /// either condition is detected in a function which uses the GC.
 ///
-void OcamlGCMetadataPrinter::finishAssembly(Module &M, AsmPrinter &AP) {
+void OcamlGCMetadataPrinter::finishAssembly(Module &M, GCModuleInfo &Info,
+                                            AsmPrinter &AP) {
   unsigned IntPtrSize =
       AP.TM.getSubtargetImpl()->getDataLayout()->getPointerSize();
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getTextSection());
   EmitCamlGlobal(M, AP, "code_end");
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getDataSection());
   EmitCamlGlobal(M, AP, "data_end");
 
   // FIXME: Why does ocaml emit this??
   AP.OutStreamer.EmitIntValue(0, IntPtrSize);
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getDataSection());
   EmitCamlGlobal(M, AP, "frametable");
 
   int NumDescriptors = 0;
-  for (iterator I = begin(), IE = end(); I != IE; ++I) {
+  for (GCModuleInfo::FuncInfoVec::iterator I = Info.funcinfo_begin(),
+         IE = Info.funcinfo_end(); I != IE; ++I) {
     GCFunctionInfo &FI = **I;
+    if (FI.getStrategy().getName() != getStrategy().getName())
+      // this function is managed by some other GC
+      continue;
     for (GCFunctionInfo::iterator J = FI.begin(), JE = FI.end(); J != JE; ++J) {
       NumDescriptors++;
     }
   }
 
   if (NumDescriptors >= 1<<16) {
     // Very rude!
     report_fatal_error(" Too much descriptor for ocaml GC");
   }
   AP.EmitInt16(NumDescriptors);
   AP.EmitAlignment(IntPtrSize == 4 ? 2 : 3);
 
-  for (iterator I = begin(), IE = end(); I != IE; ++I) {
+  for (GCModuleInfo::FuncInfoVec::iterator I = Info.funcinfo_begin(),
+         IE = Info.funcinfo_end(); I != IE; ++I) {
     GCFunctionInfo &FI = **I;
+    if (FI.getStrategy().getName() != getStrategy().getName())
+      // this function is managed by some other GC
+      continue;
 
     uint64_t FrameSize = FI.getFrameSize();
     if (FrameSize >= 1<<16) {
       // Very rude!
       report_fatal_error("Function '" + FI.getFunction().getName() +
                          "' is too large for the ocaml GC! "
                          "Frame size " + Twine(FrameSize) + ">= 65536.\n"
                          "(" + Twine(uintptr_t(&FI)) + ")");

llvm/trunk/lib/CodeGen/GCMetadata.cpp

GCFunctionInfo &GCModuleInfo::getFunctionInfo(const Function &F) {
   assert(!F.isDeclaration() && "Can only get GCFunctionInfo for a definition!");
   assert(F.hasGC());
   
   finfo_map_type::iterator I = FInfoMap.find(&F);
   if (I != FInfoMap.end())
     return *I->second;
   
   GCStrategy *S = getOrCreateStrategy(F.getParent(), F.getGC());
-  GCFunctionInfo *GFI = S->insertFunctionInfo(F);
+  Functions.push_back(make_unique<GCFunctionInfo>(F, *S));
+  GCFunctionInfo *GFI = Functions.back().get();
   FInfoMap[&F] = GFI;
   return *GFI;
 }
 
 void GCModuleInfo::clear() {
+  Functions.clear();
   FInfoMap.clear();
   StrategyMap.clear();
   StrategyList.clear();
 }
 
 // -----------------------------------------------------------------------------
 
 char Printer::ID = 0;

llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/GCMetadataPrinter.h"
 using namespace llvm;
 
 GCMetadataPrinter::GCMetadataPrinter() { }
 
 GCMetadataPrinter::~GCMetadataPrinter() { }
-
-void GCMetadataPrinter::beginAssembly(Module &M, AsmPrinter &AP) {
-  // Default is no action.
-}
-
-void GCMetadataPrinter::finishAssembly(Module &M, AsmPrinter &AP) {
-  // Default is no action.
-}

llvm/trunk/lib/CodeGen/GCStrategy.cpp

 }
 
 
 bool GCStrategy::findCustomSafePoints(GCFunctionInfo& FI, MachineFunction &F) {
   dbgs() << "gc " << getName() << " must override findCustomSafePoints.\n";
   llvm_unreachable(nullptr);
 }
 
-
-GCFunctionInfo *GCStrategy::insertFunctionInfo(const Function &F) {
-  Functions.push_back(make_unique<GCFunctionInfo>(F, *this));
-  return Functions.back().get();
-}
-
 // -----------------------------------------------------------------------------
 
 INITIALIZE_PASS_BEGIN(LowerIntrinsics, "gc-lowering", "GC Lowering",
                       false, false)
 INITIALIZE_PASS_DEPENDENCY(GCModuleInfo)
 INITIALIZE_PASS_END(LowerIntrinsics, "gc-lowering", "GC Lowering", false, false)
 
 FunctionPass *llvm::createGCLoweringPass() {