GCStrategy should not own GCFunctionInfo
authorPhilip Reames <listmail@philipreames.com>
Thu, 11 Dec 2014 01:47:23 +0000 (01:47 +0000)
committerPhilip Reames <listmail@philipreames.com>
Thu, 11 Dec 2014 01:47:23 +0000 (01:47 +0000)
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.

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@223994 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/GCMetadata.h
include/llvm/CodeGen/GCMetadataPrinter.h
include/llvm/CodeGen/GCStrategy.h
lib/CodeGen/AsmPrinter/AsmPrinter.cpp
lib/CodeGen/AsmPrinter/ErlangGCPrinter.cpp
lib/CodeGen/AsmPrinter/OcamlGCPrinter.cpp
lib/CodeGen/GCMetadata.cpp
lib/CodeGen/GCMetadataPrinter.cpp
lib/CodeGen/GCStrategy.cpp

index 27e4219ec93a985508105b615b677b03ae247592..6b24dadd584778e929751f1cb1ab2e309b277663 100644 (file)
@@ -160,22 +160,37 @@ namespace llvm {
     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;
@@ -192,8 +207,9 @@ namespace llvm {
     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);
   };
 
index 47f3aff2da662a6745d51440662efe1c2c51d4f2..25fafba93f8b653db12410a9fe6d6038076f8ef7 100644 (file)
@@ -32,16 +32,11 @@ namespace llvm {
   /// 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:
@@ -56,13 +51,14 @@ namespace llvm {
   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();
   };
index 3bc2069ca1e0724ad0f5731fa0683c2d9205c2a5..61ed2d2095bf34a507f26b8abebed2680785a826 100644 (file)
@@ -55,16 +55,10 @@ namespace llvm {
   /// 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.
@@ -126,15 +120,6 @@ namespace llvm {
     ///                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
index 9d5b78a8cf582f1d6d4899ce235c56573a95044a..6863b78a0a2ec2d9715776c78dad83e2263ed5cd 100644 (file)
@@ -210,7 +210,7 @@ bool AsmPrinter::doInitialization(Module &M) {
   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()) {
@@ -985,7 +985,7 @@ bool AsmPrinter::doFinalization(Module &M) {
   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);
index a1e9c4c358c905d9dc069a8869b70bf70cfe129d..e293acd43a88eaed83d5597c23931c086c2f66b5 100644 (file)
@@ -36,8 +36,8 @@ 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;
   };
 
 }
@@ -47,9 +47,8 @@ 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();
@@ -60,9 +59,13 @@ void ErlangGCPrinter::finishAssembly(Module &M, AsmPrinter &AP) {
                    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 {
index 850419c9ee44d46cc7b5342658af4af2fa7532ce..ddb14a05736358eef6d3754f90e1a3723896044b 100644 (file)
@@ -34,8 +34,10 @@ 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;
   };
 
 }
@@ -67,7 +69,8 @@ static void EmitCamlGlobal(const Module &M, AsmPrinter &AP, const char *Id) {
   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");
 
@@ -91,7 +94,8 @@ void OcamlGCMetadataPrinter::beginAssembly(Module &M, AsmPrinter &AP) {
 /// (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();
 
@@ -108,8 +112,12 @@ void OcamlGCMetadataPrinter::finishAssembly(Module &M, AsmPrinter &AP) {
   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++;
     }
@@ -122,8 +130,12 @@ void OcamlGCMetadataPrinter::finishAssembly(Module &M, AsmPrinter &AP) {
   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) {
index 5b03d6bb5db86f7bb3e91e29c05a4c1531b29445..6101c679a5d33655bb5f00c21a1ef0779a958d51 100644 (file)
@@ -91,12 +91,14 @@ GCFunctionInfo &GCModuleInfo::getFunctionInfo(const Function &F) {
     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();
index ad238cceec07ce04c69870e96c054ccff76aa464..fdff4a78e07942bfa4efd7ef83061fae24a9246d 100644 (file)
@@ -17,11 +17,3 @@ 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.
-}
index b346657dd96398e9de37898fc2663a982ef35dc6..044169e04f803b22bb66760927009e686052e282 100644 (file)
@@ -115,12 +115,6 @@ bool GCStrategy::findCustomSafePoints(GCFunctionInfo& FI, MachineFunction &F) {
   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",