From 07fbc5c1c66670ea2b325c303f52f3e82d69eb93 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Fri, 16 Jan 2015 20:07:33 +0000 Subject: [PATCH] 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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@226311 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/GCMetadata.h | 26 +++----------- include/llvm/CodeGen/GCMetadataPrinter.h | 2 +- include/llvm/IR/Function.h | 5 +++ include/llvm/{CodeGen => IR}/GCStrategy.h | 32 ++++++++++++----- include/llvm/Support/Registry.h | 4 +++ lib/CodeGen/CMakeLists.txt | 1 - lib/CodeGen/ErlangGC.cpp | 2 +- lib/CodeGen/GCMetadata.cpp | 34 ++++++------------- lib/CodeGen/GCRootLowering.cpp | 3 +- lib/CodeGen/OcamlGC.cpp | 2 +- lib/CodeGen/Passes.cpp | 1 - .../SelectionDAG/SelectionDAGBuilder.cpp | 2 +- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 2 +- .../SelectionDAG/StatepointLowering.cpp | 3 +- lib/CodeGen/ShadowStackGC.cpp | 2 +- lib/CodeGen/StatepointExampleGC.cpp | 2 +- lib/IR/CMakeLists.txt | 1 + lib/IR/Function.cpp | 6 ++++ lib/{CodeGen => IR}/GCStrategy.cpp | 2 +- lib/IR/LLVMContextImpl.cpp | 24 +++++++++++++ lib/IR/LLVMContextImpl.h | 12 +++++++ 21 files changed, 102 insertions(+), 66 deletions(-) rename include/llvm/{CodeGen => IR}/GCStrategy.h (90%) rename lib/{CodeGen => IR}/GCStrategy.cpp (95%) diff --git a/include/llvm/CodeGen/GCMetadata.h b/include/llvm/CodeGen/GCMetadata.h index c7f1ab87fcb..6922b70bd9a 100644 --- a/include/llvm/CodeGen/GCMetadata.h +++ b/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 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 strategy_map_type; - typedef std::vector> 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 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::const_iterator iterator; static char ID; diff --git a/include/llvm/CodeGen/GCMetadataPrinter.h b/include/llvm/CodeGen/GCMetadataPrinter.h index 25fafba93f8..6010cb8a4f3 100644 --- a/include/llvm/CodeGen/GCMetadataPrinter.h +++ b/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 { diff --git a/include/llvm/IR/Function.h b/include/llvm/IR/Function.h index 51403281e96..e9d0806eadd 100644 --- a/include/llvm/IR/Function.h +++ b/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); diff --git a/include/llvm/CodeGen/GCStrategy.h b/include/llvm/IR/GCStrategy.h similarity index 90% rename from include/llvm/CodeGen/GCStrategy.h rename to include/llvm/IR/GCStrategy.h index bdd14463e00..1cd107b3bca 100644 --- a/include/llvm/CodeGen/GCStrategy.h +++ b/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 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, diff --git a/include/llvm/Support/Registry.h b/include/llvm/Support/Registry.h index e21269b2f1d..51db8cfb15b 100644 --- a/include/llvm/Support/Registry.h +++ b/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 entries() { + return iterator_range(begin(), end()); + } + /// Abstract base class for registry listeners, which are informed when new /// entries are added to the registry. Simply subclass and instantiate: diff --git a/lib/CodeGen/CMakeLists.txt b/lib/CodeGen/CMakeLists.txt index f0b2dfe365b..7f7f5225180 100644 --- a/lib/CodeGen/CMakeLists.txt +++ b/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 diff --git a/lib/CodeGen/ErlangGC.cpp b/lib/CodeGen/ErlangGC.cpp index 5f2b3a09832..ec2c2e8daf2 100644 --- a/lib/CodeGen/ErlangGC.cpp +++ b/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" diff --git a/lib/CodeGen/GCMetadata.cpp b/lib/CodeGen/GCMetadata.cpp index 6101c679a5d..6f4fa11d6d9 100644 --- a/lib/CodeGen/GCMetadata.cpp +++ b/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 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(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(); } diff --git a/lib/CodeGen/GCRootLowering.cpp b/lib/CodeGen/GCRootLowering.cpp index 9bda7b8493f..8ff08ece76e 100644 --- a/lib/CodeGen/GCRootLowering.cpp +++ b/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" diff --git a/lib/CodeGen/OcamlGC.cpp b/lib/CodeGen/OcamlGC.cpp index 48db200c513..c5c78529ece 100644 --- a/lib/CodeGen/OcamlGC.cpp +++ b/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; diff --git a/lib/CodeGen/Passes.cpp b/lib/CodeGen/Passes.cpp index 28e9f847a9d..88cee03fa16 100644 --- a/lib/CodeGen/Passes.cpp +++ b/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" diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 6b3ad5f69b0..a8db35c3a98 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/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" diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index 7ee06fc153b..6a0a07970a8 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/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" diff --git a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp index 33c20d3f219..94f09bdc2f9 100644 --- a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp +++ b/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" diff --git a/lib/CodeGen/ShadowStackGC.cpp b/lib/CodeGen/ShadowStackGC.cpp index 0be00f0e7ce..181cb8c435c 100644 --- a/lib/CodeGen/ShadowStackGC.cpp +++ b/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" diff --git a/lib/CodeGen/StatepointExampleGC.cpp b/lib/CodeGen/StatepointExampleGC.cpp index 09b74ca1d2d..e20b270f42d 100644 --- a/lib/CodeGen/StatepointExampleGC.cpp +++ b/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" diff --git a/lib/IR/CMakeLists.txt b/lib/IR/CMakeLists.txt index 1a210e05827..621c7ca9224 100644 --- a/lib/IR/CMakeLists.txt +++ b/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 diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp index 070513edef2..ddfbaf7a187 100644 --- a/lib/IR/Function.cpp +++ b/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) { diff --git a/lib/CodeGen/GCStrategy.cpp b/lib/IR/GCStrategy.cpp similarity index 95% rename from lib/CodeGen/GCStrategy.cpp rename to lib/IR/GCStrategy.cpp index 2b687d9dd20..18723a0f514 100644 --- a/lib/CodeGen/GCStrategy.cpp +++ b/lib/IR/GCStrategy.cpp @@ -12,7 +12,7 @@ // //===----------------------------------------------------------------------===// -#include "llvm/CodeGen/GCStrategy.h" +#include "llvm/IR/GCStrategy.h" using namespace llvm; diff --git a/lib/IR/LLVMContextImpl.cpp b/lib/IR/LLVMContextImpl.cpp index 01a0e6c98a6..01a786dbfda 100644 --- a/lib/IR/LLVMContextImpl.cpp +++ b/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 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 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; +} + + diff --git a/lib/IR/LLVMContextImpl.h b/lib/IR/LLVMContextImpl.h index 6ebc567f808..45f86093f52 100644 --- a/lib/IR/LLVMContextImpl.h +++ b/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, 1> GCStrategyList; + /// A helper map to speedup lookups into the above list + StringMap 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(); -- 2.34.1