Optimizing MCJIT module state tracking

Patch co-developed with Yaron Keren.




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

include/llvm/ExecutionEngine/ExecutionEngine.h

@@ -215,7 +215,7 @@ public:
   /// FindFunctionNamed - Search all of the active modules to find the one that
   /// defines FnName.  This is very slow operation and shouldn't be used for
   /// general code.
-  Function *FindFunctionNamed(const char *FnName);
+  virtual Function *FindFunctionNamed(const char *FnName);
 
   /// runFunction - Execute the specified function with the specified arguments,
   /// and return the result.
@@ -232,6 +232,9 @@ public:
   ///
   /// This function is deprecated for the MCJIT execution engine.
   ///
+  /// FIXME: the JIT and MCJIT interfaces should be disentangled or united 
+  /// again, if possible.
+  ///
   virtual void *getPointerToNamedFunction(const std::string &Name,
                                           bool AbortOnFailure = true) = 0;
 
@@ -275,7 +278,7 @@ public:
   /// the static constructors or destructors for a program.
   ///
   /// \param isDtors - Run the destructors instead of constructors.
-  void runStaticConstructorsDestructors(bool isDtors);
+  virtual void runStaticConstructorsDestructors(bool isDtors);
 
   /// runStaticConstructorsDestructors - This method is used to execute all of
   /// the static constructors or destructors for a particular module.

lib/ExecutionEngine/MCJIT/MCJIT.cpp

@@ -14,6 +14,7 @@
 #include "llvm/ExecutionEngine/MCJIT.h"
 #include "llvm/ExecutionEngine/ObjectBuffer.h"
 #include "llvm/ExecutionEngine/ObjectImage.h"
+#include "llvm/PassManager.h"
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
@@ -57,32 +58,40 @@ MCJIT::MCJIT(Module *m, TargetMachine *tm, RTDyldMemoryManager *MM,
   : ExecutionEngine(m), TM(tm), Ctx(0), MemMgr(this, MM), Dyld(&MemMgr),
     ObjCache(0) {
 
-  ModuleStates[m] = ModuleAdded;
+  OwnedModules.addModule(m);
   setDataLayout(TM->getDataLayout());
 }
 
 MCJIT::~MCJIT() {
   MutexGuard locked(lock);
+  // FIXME: We are managing our modules, so we do not want the base class
+  // ExecutionEngine to manage them as well. To avoid double destruction
+  // of the first (and only) module added in ExecutionEngine constructor
+  // we remove it from EE and will destruct it ourselves.
+  //
+  // It may make sense to move our module manager (based on SmallStPtr) back
+  // into EE if the JIT and Interpreter can live with it.
+  // If so, additional functions: addModule, removeModule, FindFunctionNamed,
+  // runStaticConstructorsDestructors could be moved back to EE as well.
+  //
+  Modules.clear();
   Dyld.deregisterEHFrames();
-
-  LoadedObjectMap::iterator it, end = LoadedObjects.end();
-  for (it = LoadedObjects.begin(); it != end; ++it) {
-    ObjectImage *Obj = it->second;
-    if (Obj) {
-      NotifyFreeingObject(*Obj);
-      delete Obj;
-    }
-  }
   LoadedObjects.clear();
   delete TM;
 }
 
 void MCJIT::addModule(Module *M) {
   MutexGuard locked(lock);
-  Modules.push_back(M);
+  OwnedModules.addModule(M);
-  ModuleStates[M] = MCJITModuleState();
 }
 
+bool MCJIT::removeModule(Module *M) {
+  MutexGuard locked(lock);
+  return OwnedModules.removeModule(M);
+}
+
+
+
 void MCJIT::setObjectCache(ObjectCache* NewCache) {
   MutexGuard locked(lock);
   ObjCache = NewCache;
@@ -91,12 +100,9 @@ void MCJIT::setObjectCache(ObjectCache* NewCache) {
 ObjectBufferStream* MCJIT::emitObject(Module *M) {
   MutexGuard locked(lock);
 
-  // This must be a module which has already been added to this MCJIT instance.
+  // This must be a module which has already been added but not loaded to this
-  assert(std::find(Modules.begin(), Modules.end(), M) != Modules.end());
+  // MCJIT instance, since these conditions are tested by our caller,
-  assert(ModuleStates.find(M) != ModuleStates.end());
+  // generateCodeForModule.
-
-  // Re-compilation is not supported
-  assert(!ModuleStates[M].hasBeenEmitted());
 
   PassManager PM;
 
@@ -133,11 +139,11 @@ void MCJIT::generateCodeForModule(Module *M) {
   MutexGuard locked(lock);
 
   // This must be a module which has already been added to this MCJIT instance.
-  assert(std::find(Modules.begin(), Modules.end(), M) != Modules.end());
+  assert(OwnedModules.ownsModule(M) &&
-  assert(ModuleStates.find(M) != ModuleStates.end());
+         "MCJIT::generateCodeForModule: Unknown module.");
 
   // Re-compilation is not supported
-  if (ModuleStates[M].hasBeenLoaded())
+  if (OwnedModules.hasModuleBeenLoaded(M))
     return;
 
   OwningPtr<ObjectBuffer> ObjectToLoad;
@@ -166,7 +172,7 @@ void MCJIT::generateCodeForModule(Module *M) {
 
   NotifyObjectEmitted(*LoadedObject);
 
-  ModuleStates[M] = ModuleLoaded;
+  OwnedModules.markModuleAsLoaded(M);
 }
 
 void MCJIT::finalizeLoadedModules() {
@@ -175,19 +181,9 @@ void MCJIT::finalizeLoadedModules() {
   // Resolve any outstanding relocations.
   Dyld.resolveRelocations();
 
+  OwnedModules.markAllLoadedModulesAsFinalized();
+
   // Register EH frame data for any module we own which has been loaded
-  SmallVector<Module *, 1>::iterator end = Modules.end();
-  SmallVector<Module *, 1>::iterator it;
-  for (it = Modules.begin(); it != end; ++it) {
-    Module *M = *it;
-    assert(ModuleStates.find(M) != ModuleStates.end());
-
-    if (ModuleStates[M].hasBeenLoaded() &&
-        !ModuleStates[M].hasBeenFinalized()) {
-      ModuleStates[M] = ModuleFinalized;
-    }
-  }
-
   Dyld.registerEHFrames();
 
   // Set page permissions.
@@ -198,68 +194,27 @@ void MCJIT::finalizeLoadedModules() {
 void MCJIT::finalizeObject() {
   MutexGuard locked(lock);
 
-  // FIXME: This is a temporary hack to get around problems with calling
+  for (ModulePtrSet::iterator I = OwnedModules.begin_added(),
-  // finalize multiple times.
+                              E = OwnedModules.end_added();
-  bool finalizeNeeded = false;
+       I != E; ++I) {
-  SmallVector<Module *, 1>::iterator end = Modules.end();
+    Module *M = *I;
-  SmallVector<Module *, 1>::iterator it;
+    generateCodeForModule(M);
-  for (it = Modules.begin(); it != end; ++it) {
-    Module *M = *it;
-    assert(ModuleStates.find(M) != ModuleStates.end());
-    if (!ModuleStates[M].hasBeenFinalized())
-      finalizeNeeded = true;
-
-    // I don't really like this, but the C API depends on this behavior.
-    // I suppose it's OK for a deprecated function.
-    if (!ModuleStates[M].hasBeenLoaded())
-      generateCodeForModule(M);
-  }
-  if (!finalizeNeeded)
-    return;
-
-  // Resolve any outstanding relocations.
-  Dyld.resolveRelocations();
-
-  // Register EH frame data for any module we own which has been loaded
-  for (it = Modules.begin(); it != end; ++it) {
-    Module *M = *it;
-    assert(ModuleStates.find(M) != ModuleStates.end());
-
-    if (ModuleStates[M].hasBeenLoaded() &&
-        !ModuleStates[M].hasBeenFinalized()) {
-      ModuleStates[M] = ModuleFinalized;
-    }
   }
 
-  Dyld.registerEHFrames();
+  finalizeLoadedModules();
-
-  // Set page permissions.
-  MemMgr.finalizeMemory();
 }
 
 void MCJIT::finalizeModule(Module *M) {
   MutexGuard locked(lock);
 
   // This must be a module which has already been added to this MCJIT instance.
-  assert(std::find(Modules.begin(), Modules.end(), M) != Modules.end());
+  assert(OwnedModules.ownsModule(M) && "MCJIT::finalizeModule: Unknown module.");
-  assert(ModuleStates.find(M) != ModuleStates.end());
-
-  if (ModuleStates[M].hasBeenFinalized())
-    return;
 
   // If the module hasn't been compiled, just do that.
-  if (!ModuleStates[M].hasBeenLoaded())
+  if (!OwnedModules.hasModuleBeenLoaded(M))
     generateCodeForModule(M);
 
-  // Resolve any outstanding relocations.
+  finalizeLoadedModules();
-  Dyld.resolveRelocations();
-
-  Dyld.registerEHFrames();
-
-  // Set page permissions.
-  MemMgr.finalizeMemory();
-
-  ModuleStates[M] = ModuleFinalized;
 }
 
 void *MCJIT::getPointerToBasicBlock(BasicBlock *BB) {
@@ -279,10 +234,10 @@ Module *MCJIT::findModuleForSymbol(const std::string &Name,
   MutexGuard locked(lock);
 
   // If it hasn't already been generated, see if it's in one of our modules.
-  SmallVector<Module *, 1>::iterator end = Modules.end();
+  for (ModulePtrSet::iterator I = OwnedModules.begin_added(),
-  SmallVector<Module *, 1>::iterator it;
+                              E = OwnedModules.end_added();
-  for (it = Modules.begin(); it != end; ++it) {
+       I != E; ++I) {
-    Module *M = *it;
+    Module *M = *I;
     Function *F = M->getFunction(Name);
     if (F && !F->empty())
       return M;
@@ -312,12 +267,6 @@ uint64_t MCJIT::getSymbolAddress(const std::string &Name,
   if (!M)
     return 0;
 
-  // If this is in one of our modules, generate code for that module.
-  assert(ModuleStates.find(M) != ModuleStates.end());
-  // If the module code has already been generated, we won't find the symbol.
-  if (ModuleStates[M].hasBeenLoaded())
-    return 0;
-
   generateCodeForModule(M);
 
   // Check the RuntimeDyld table again, it should be there now.
@@ -351,16 +300,15 @@ void *MCJIT::getPointerToFunction(Function *F) {
     return Addr;
   }
 
-  // If this function doesn't belong to one of our modules, we're done.
   Module *M = F->getParent();
-  if (std::find(Modules.begin(), Modules.end(), M) == Modules.end())
+  bool HasBeenAddedButNotLoaded = OwnedModules.hasModuleBeenAddedButNotLoaded(M);
-    return NULL;
-
-  assert(ModuleStates.find(M) != ModuleStates.end());
 
   // Make sure the relevant module has been compiled and loaded.
-  if (!ModuleStates[M].hasBeenLoaded())
+  if (HasBeenAddedButNotLoaded)
     generateCodeForModule(M);
+  else if (!OwnedModules.hasModuleBeenLoaded(M))
+    // If this function doesn't belong to one of our modules, we're done.
+    return NULL;
 
   // FIXME: Should the Dyld be retaining module information? Probably not.
   // FIXME: Should we be using the mangler for this? Probably.
@@ -382,6 +330,45 @@ void MCJIT::freeMachineCodeForFunction(Function *F) {
   report_fatal_error("not yet implemented");
 }
 
+void MCJIT::runStaticConstructorsDestructorsInModulePtrSet(
+    bool isDtors, ModulePtrSet::iterator I, ModulePtrSet::iterator E) {
+  for (; I != E; ++I) {
+    ExecutionEngine::runStaticConstructorsDestructors(*I, isDtors);
+  }
+}
+
+void MCJIT::runStaticConstructorsDestructors(bool isDtors) {
+  // Execute global ctors/dtors for each module in the program.
+  runStaticConstructorsDestructorsInModulePtrSet(
+      isDtors, OwnedModules.begin_added(), OwnedModules.end_added());
+  runStaticConstructorsDestructorsInModulePtrSet(
+      isDtors, OwnedModules.begin_loaded(), OwnedModules.end_loaded());
+  runStaticConstructorsDestructorsInModulePtrSet(
+      isDtors, OwnedModules.begin_finalized(), OwnedModules.end_finalized());
+}
+
+Function *MCJIT::FindFunctionNamedInModulePtrSet(const char *FnName,
+                                                 ModulePtrSet::iterator I,
+                                                 ModulePtrSet::iterator E) {
+  for (; I != E; ++I) {
+    if (Function *F = (*I)->getFunction(FnName))
+      return F;
+  }
+  return 0;
+}
+
+Function *MCJIT::FindFunctionNamed(const char *FnName) {
+  Function *F = FindFunctionNamedInModulePtrSet(
+      FnName, OwnedModules.begin_added(), OwnedModules.end_added());
+  if (!F)
+    F = FindFunctionNamedInModulePtrSet(FnName, OwnedModules.begin_loaded(),
+                                        OwnedModules.end_loaded());
+  if (!F)
+    F = FindFunctionNamedInModulePtrSet(FnName, OwnedModules.begin_finalized(),
+                                        OwnedModules.end_finalized());
+  return F;
+}
+
 GenericValue MCJIT::runFunction(Function *F,
                                 const std::vector<GenericValue> &ArgValues) {
   assert(F && "Function *F was null at entry to run()");

lib/ExecutionEngine/MCJIT/MCJIT.h

@@ -11,15 +11,15 @@
 #define LLVM_LIB_EXECUTIONENGINE_MCJIT_H
 
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ExecutionEngine/ExecutionEngine.h"
 #include "llvm/ExecutionEngine/ObjectCache.h"
 #include "llvm/ExecutionEngine/ObjectImage.h"
 #include "llvm/ExecutionEngine/RuntimeDyld.h"
-#include "llvm/PassManager.h"
+#include "llvm/IR/Module.h"
 
 namespace llvm {
-
 class MCJIT;
 
 // This is a helper class that the MCJIT execution engine uses for linking
@@ -70,7 +70,7 @@ private:
   OwningPtr<RTDyldMemoryManager> ClientMM;
 };
 
-// About Module states:
+// About Module states: added->loaded->finalized.
 //
 // The purpose of the "added" state is having modules in standby. (added=known
 // but not compiled). The idea is that you can add a module to provide function
@@ -94,27 +94,108 @@ class MCJIT : public ExecutionEngine {
   MCJIT(Module *M, TargetMachine *tm, RTDyldMemoryManager *MemMgr,
         bool AllocateGVsWithCode);
 
-  enum ModuleState {
+  typedef llvm::SmallPtrSet<Module *, 4> ModulePtrSet;
-    ModuleAdded,
-    ModuleEmitted,
-    ModuleLoading,
-    ModuleLoaded,
-    ModuleFinalizing,
-    ModuleFinalized
-  };
 
-  class MCJITModuleState {
+  class OwningModuleContainer {
   public:
-    MCJITModuleState() : State(ModuleAdded) {}
+    OwningModuleContainer() {
+    }
+    ~OwningModuleContainer() {
+      freeModulePtrSet(AddedModules);
+      freeModulePtrSet(LoadedModules);
+      freeModulePtrSet(FinalizedModules);
+    }
 
-    MCJITModuleState & operator=(ModuleState s) { State = s; return *this; }
+    ModulePtrSet::iterator begin_added() { return AddedModules.begin(); }
-    bool hasBeenEmitted() { return State != ModuleAdded; }
+    ModulePtrSet::iterator end_added() { return AddedModules.end(); }
-    bool hasBeenLoaded() { return State != ModuleAdded &&
+
-                                  State != ModuleEmitted; }
+    ModulePtrSet::iterator begin_loaded() { return LoadedModules.begin(); }
-    bool hasBeenFinalized() { return State == ModuleFinalized; }
+    ModulePtrSet::iterator end_loaded() { return LoadedModules.end(); }
+
+    ModulePtrSet::iterator begin_finalized() { return FinalizedModules.begin(); }
+    ModulePtrSet::iterator end_finalized() { return FinalizedModules.end(); }
+
+    void addModule(Module *M) {
+      AddedModules.insert(M);
+    }
+
+    bool removeModule(Module *M) {
+      return AddedModules.erase(M) || LoadedModules.erase(M) ||
+             FinalizedModules.erase(M);
+    }
+
+    bool hasModuleBeenAddedButNotLoaded(Module *M) {
+      return AddedModules.count(M) != 0;
+    }
+
+    bool hasModuleBeenLoaded(Module *M) {
+      // If the module is in either the "loaded" or "finalized" sections it
+      // has been loaded.
+      return (LoadedModules.count(M) != 0 ) || (FinalizedModules.count(M) != 0);
+    }
+
+    bool hasModuleBeenFinalized(Module *M) {
+      return FinalizedModules.count(M) != 0;
+    }
+
+    bool ownsModule(Module* M) {
+      return (AddedModules.count(M) != 0) || (LoadedModules.count(M) != 0) ||
+             (FinalizedModules.count(M) != 0);
+    }
+
+    void markModuleAsLoaded(Module *M) {
+      // This checks against logic errors in the MCJIT implementation.
+      // This function should never be called with either a Module that MCJIT
+      // does not own or a Module that has already been loaded and/or finalized.
+      assert(AddedModules.count(M) &&
+             "markModuleAsLoaded: Module not found in AddedModules");
+
+      // Remove the module from the "Added" set.
+      AddedModules.erase(M);
+
+      // Add the Module to the "Loaded" set.
+      LoadedModules.insert(M);
+    }
+
+    void markModuleAsFinalized(Module *M) {
+      // This checks against logic errors in the MCJIT implementation.
+      // This function should never be called with either a Module that MCJIT
+      // does not own, a Module that has not been loaded or a Module that has
+      // already been finalized.
+      assert(LoadedModules.count(M) &&
+             "markModuleAsFinalized: Module not found in LoadedModules");
+
+      // Remove the module from the "Loaded" section of the list.
+      LoadedModules.erase(M);
+
+      // Add the Module to the "Finalized" section of the list by inserting it
+      // before the 'end' iterator.
+      FinalizedModules.insert(M);
+    }
+
+    void markAllLoadedModulesAsFinalized() {
+      for (ModulePtrSet::iterator I = LoadedModules.begin(),
+                                  E = LoadedModules.end();
+           I != E; ++I) {
+        Module *M = *I;
+        FinalizedModules.insert(M);
+      }
+      LoadedModules.clear();
+    }
 
   private:
-    ModuleState State;
+    ModulePtrSet AddedModules;
+    ModulePtrSet LoadedModules;
+    ModulePtrSet FinalizedModules;
+
+    void freeModulePtrSet(ModulePtrSet& MPS) {
+      // Go through the module set and delete everything.
+      for (ModulePtrSet::iterator I = MPS.begin(), E = MPS.end(); I != E; ++I) {
+        Module *M = *I;
+        delete M;
+      }
+      MPS.clear();
+    }
   };
 
   TargetMachine *TM;
@@ -123,8 +204,7 @@ class MCJIT : public ExecutionEngine {
   RuntimeDyld Dyld;
   SmallVector<JITEventListener*, 2> EventListeners;
 
-  typedef DenseMap<Module *, MCJITModuleState> ModuleStateMap;
+  OwningModuleContainer OwnedModules;
-  ModuleStateMap  ModuleStates;
 
   typedef DenseMap<Module *, ObjectImage *> LoadedObjectMap;
   LoadedObjectMap  LoadedObjects;
@@ -133,12 +213,26 @@ class MCJIT : public ExecutionEngine {
   // perform lookup of pre-compiled code to avoid re-compilation.
   ObjectCache *ObjCache;
 
+  Function *FindFunctionNamedInModulePtrSet(const char *FnName,
+                                            ModulePtrSet::iterator I,
+                                            ModulePtrSet::iterator E);
+
+  void runStaticConstructorsDestructorsInModulePtrSet(bool isDtors,
+                                                      ModulePtrSet::iterator I,
+                                                      ModulePtrSet::iterator E);
+
 public:
   ~MCJIT();
 
   /// @name ExecutionEngine interface implementation
   /// @{
   virtual void addModule(Module *M);
+  virtual bool removeModule(Module *M);
+
+  /// FindFunctionNamed - Search all of the active modules to find the one that
+  /// defines FnName.  This is very slow operation and shouldn't be used for
+  /// general code.
+  virtual Function *FindFunctionNamed(const char *FnName);
 
   /// Sets the object manager that MCJIT should use to avoid compilation.
   virtual void setObjectCache(ObjectCache *manager);
@@ -158,6 +252,12 @@ public:
   virtual void finalizeModule(Module *);
   void finalizeLoadedModules();
 
+  /// runStaticConstructorsDestructors - This method is used to execute all of
+  /// the static constructors or destructors for a program.
+  ///
+  /// \param isDtors - Run the destructors instead of constructors.
+  void runStaticConstructorsDestructors(bool isDtors);
+
   virtual void *getPointerToBasicBlock(BasicBlock *BB);
 
   virtual void *getPointerToFunction(Function *F);