Remove the Module pointer from GCStrategy and GCMetadataPrinter

[reames]

Summary

In the current implementation, GCStrategy is a part of the ownership structure for the gc metadata which describes a Module. It also contains a reference to the module in question. As a result, GCStrategy instances are essentially Module specific.

I plan to transition away from this design. Instead, a GCStrategy will be owned by the LLVMContext. It will be a lightweight policy object which contains no information about the Modules or Functions involved, but can be easily reached given a Function.

The first step in this transition is to remove the direct Module reference from GCStrategy. This also requires removing the single user of this reference, the GCMetadataPrinter hierarchy. In theory, this will allow the lifetime of the printers to be scoped to the LLVMContext as well, but in practice, I'm not actually changing that. (Yet?)

An alternate design would have been to move the direct Module reference into the GCMetadataPrinter and change the keying of the owning maps to explicitly key off both GCStrategy and Module. I'm open to doing it that way instead, but didn't see much value in preserving the per Module association for GCMetadataPrinters.

The next change in this sequence will be to start unwinding the intertwined ownership between GCStrategy, GCModuleInfo, and GCFunctionInfo.

[whitequark]

LGTM

[reames]

Closed by commit rL223859 (authored by @reames).

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

namespace llvm {
 
   private:
     GCMetadataPrinter(const GCMetadataPrinter &) LLVM_DELETED_FUNCTION;
     GCMetadataPrinter &
       operator=(const GCMetadataPrinter &) LLVM_DELETED_FUNCTION;
 
   public:
     GCStrategy &getStrategy() { return *S; }
-    const Module &getModule() const { return S->getModule(); }
 
     /// 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(AsmPrinter &AP);
-
-    virtual void finishAssembly(AsmPrinter &AP);
+    virtual void beginAssembly(Module &M, AsmPrinter &AP);
+    virtual void finishAssembly(Module &M, AsmPrinter &AP);
 
     virtual ~GCMetadataPrinter();
   };
 
 }
 
 #endif

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

namespace llvm {
   /// be abstractly described.
   class GCStrategy {
   public:
     typedef std::vector<std::unique_ptr<GCFunctionInfo>> list_type;
     typedef list_type::iterator iterator;
     
   private:
     friend class GCModuleInfo;
-    const Module *M;
     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.
     bool UsesMetadata;         ///< If set, backend must emit metadata tables.
     
   public:
     GCStrategy();
     
     virtual ~GCStrategy() {}
     
     
     /// getName - The name of the GC strategy, for debugging.
     /// 
     const std::string &getName() const { return Name; }
 
-    /// getModule - The module within which the GC strategy is operating.
-    /// 
-    const Module &getModule() const { return *M; }
-
     /// needsSafePoitns - True if safe points of any kind are required. By
     //                    default, none are recorded.
     bool needsSafePoints() const {
       return CustomSafePoints || NeededSafePoints != 0;
     }
     
     /// needsSafePoint(Kind) - True if the given kind of safe point is
     //                          required. By default, none are recorded.

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(*this);
+      MP->beginAssembly(M, *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(*this);
+      MP->finishAssembly(M, *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(AsmPrinter &AP) override;
-    void finishAssembly(AsmPrinter &AP) override;
+    void beginAssembly(Module &M, AsmPrinter &AP) override;
+    void finishAssembly(Module &M, AsmPrinter &AP) override;
   };
 
 }
 
 static GCMetadataPrinterRegistry::Add<ErlangGCPrinter>
 X("erlang", "erlang-compatible garbage collector");
 
 void llvm::linkErlangGCPrinter() { }
 
-void ErlangGCPrinter::beginAssembly(AsmPrinter &AP) { }
+void ErlangGCPrinter::beginAssembly(Module &M, AsmPrinter &AP) { }
 
-void ErlangGCPrinter::finishAssembly(AsmPrinter &AP) {
+void ErlangGCPrinter::finishAssembly(Module &M, 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()));

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(AsmPrinter &AP) override;
-    void finishAssembly(AsmPrinter &AP) override;
+    void beginAssembly(Module &M, AsmPrinter &AP) override;
+    void finishAssembly(Module &M, 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(AsmPrinter &AP) {
+void OcamlGCMetadataPrinter::beginAssembly(Module &M, AsmPrinter &AP) {
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getTextSection());
-  EmitCamlGlobal(getModule(), AP, "code_begin");
+  EmitCamlGlobal(M, AP, "code_begin");
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getDataSection());
-  EmitCamlGlobal(getModule(), AP, "data_begin");
+  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(AsmPrinter &AP) {
+void OcamlGCMetadataPrinter::finishAssembly(Module &M, AsmPrinter &AP) {
   unsigned IntPtrSize =
       AP.TM.getSubtargetImpl()->getDataLayout()->getPointerSize();
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getTextSection());
-  EmitCamlGlobal(getModule(), AP, "code_end");
+  EmitCamlGlobal(M, AP, "code_end");
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getDataSection());
-  EmitCamlGlobal(getModule(), AP, "data_end");
+  EmitCamlGlobal(M, AP, "data_end");
 
   // FIXME: Why does ocaml emit this??
   AP.OutStreamer.EmitIntValue(0, IntPtrSize);
 
   AP.OutStreamer.SwitchSection(AP.getObjFileLowering().getDataSection());
-  EmitCamlGlobal(getModule(), AP, "frametable");
+  EmitCamlGlobal(M, AP, "frametable");
 
   int NumDescriptors = 0;
   for (iterator I = begin(), IE = end(); I != IE; ++I) {
     GCFunctionInfo &FI = **I;
     for (GCFunctionInfo::iterator J = FI.begin(), JE = FI.end(); J != JE; ++J) {
       NumDescriptors++;
     }
   }

llvm/trunk/lib/CodeGen/GCMetadata.cpp

GCStrategy *GCModuleInfo::getOrCreateStrategy(const Module *M,
   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->M = M;
       S->Name = Name;
       StrategyMap[Name] = S.get();
       StrategyList.push_back(std::move(S));
       return StrategyList.back().get();
     }
   }
  
   dbgs() << "unsupported GC: " << Name << "\n";

llvm/trunk/lib/CodeGen/GCMetadataPrinter.cpp

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