[llvm] 54aec17 - [ORC] Don't use a platform mutex for LLJIT's GenericLLVMIRPlatformSupport class.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2020-03-19T11:03:34-07:00
New Revision: 54aec178dac9ba0bf543cf854c8f78d237fd0771

URL: https://github.com/llvm/llvm-project/commit/54aec178dac9ba0bf543cf854c8f78d237fd0771
DIFF: https://github.com/llvm/llvm-project/commit/54aec178dac9ba0bf543cf854c8f78d237fd0771.diff

LOG: [ORC] Don't use a platform mutex for LLJIT's GenericLLVMIRPlatformSupport class.

Along the same lines as eb918d8daf1: This code also had to acquire the session
mutex, and this could cause a deadlock under the wrong circumstances. This
patch updates GenericLLVMIRPlatformSupport to just use the session lock for
everything.

Added: 
    

Modified: 
    llvm/lib/ExecutionEngine/Orc/LLJIT.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
index 63a5b1f09c82..9451a5725493 100644
--- a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
@@ -175,7 +175,6 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
   }
 
   Error notifyAdding(JITDylib &JD, const MaterializationUnit &MU) {
-    std::lock_guard<std::mutex> Lock(PlatformSupportMutex);
     if (auto &InitSym = MU.getInitializerSymbol())
       InitSymbols[&JD].add(InitSym);
     else {
@@ -236,11 +235,13 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
   }
 
   void registerInitFunc(JITDylib &JD, SymbolStringPtr InitName) {
-    std::lock_guard<std::mutex> Lock(PlatformSupportMutex);
-    InitFunctions[&JD].add(InitName);
+    getExecutionSession().runSessionLocked([&]() {
+        InitFunctions[&JD].add(InitName);
+      });
   }
 
 private:
+
   Expected<std::vector<JITTargetAddress>> getInitializers(JITDylib &JD) {
     if (auto Err = issueInitLookups(JD))
       return std::move(Err);
@@ -248,18 +249,17 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
     DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
     std::vector<JITDylib *> DFSLinkOrder;
 
-    {
-      std::lock_guard<std::mutex> Lock(PlatformSupportMutex);
-      DFSLinkOrder = getDFSLinkOrder(JD);
+    getExecutionSession().runSessionLocked([&]() {
+        DFSLinkOrder = getDFSLinkOrder(JD);
 
-      for (auto *NextJD : DFSLinkOrder) {
-        auto IFItr = InitFunctions.find(NextJD);
-        if (IFItr != InitFunctions.end()) {
-          LookupSymbols[NextJD] = std::move(IFItr->second);
-          InitFunctions.erase(IFItr);
+        for (auto *NextJD : DFSLinkOrder) {
+          auto IFItr = InitFunctions.find(NextJD);
+          if (IFItr != InitFunctions.end()) {
+            LookupSymbols[NextJD] = std::move(IFItr->second);
+            InitFunctions.erase(IFItr);
+          }
         }
-      }
-    }
+      });
 
     LLVM_DEBUG({
       dbgs() << "JITDylib init order is [ ";
@@ -300,21 +300,20 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
     DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
     std::vector<JITDylib *> DFSLinkOrder;
 
-    {
-      std::lock_guard<std::mutex> Lock(PlatformSupportMutex);
-      DFSLinkOrder = getDFSLinkOrder(JD);
-
-      for (auto *NextJD : DFSLinkOrder) {
-        auto &JDLookupSymbols = LookupSymbols[NextJD];
-        auto DIFItr = DeInitFunctions.find(NextJD);
-        if (DIFItr != DeInitFunctions.end()) {
-          LookupSymbols[NextJD] = std::move(DIFItr->second);
-          DeInitFunctions.erase(DIFItr);
-        }
-        JDLookupSymbols.add(LLJITRunAtExits,
+    ES.runSessionLocked([&]() {
+        DFSLinkOrder = getDFSLinkOrder(JD);
+
+        for (auto *NextJD : DFSLinkOrder) {
+          auto &JDLookupSymbols = LookupSymbols[NextJD];
+          auto DIFItr = DeInitFunctions.find(NextJD);
+          if (DIFItr != DeInitFunctions.end()) {
+            LookupSymbols[NextJD] = std::move(DIFItr->second);
+            DeInitFunctions.erase(DIFItr);
+          }
+          JDLookupSymbols.add(LLJITRunAtExits,
                             SymbolLookupFlags::WeaklyReferencedSymbol);
       }
-    }
+    });
 
     auto LookupResult = Platform::lookupInitSymbols(ES, LookupSymbols);
 
@@ -366,20 +365,19 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
   /// JITDylibs that it depends on).
   Error issueInitLookups(JITDylib &JD) {
     DenseMap<JITDylib *, SymbolLookupSet> RequiredInitSymbols;
+    std::vector<JITDylib *> DFSLinkOrder;
 
-    {
-      std::lock_guard<std::mutex> Lock(PlatformSupportMutex);
-
-      auto DFSLinkOrder = getDFSLinkOrder(JD);
+    getExecutionSession().runSessionLocked([&]() {
+        DFSLinkOrder = getDFSLinkOrder(JD);
 
-      for (auto *NextJD : DFSLinkOrder) {
-        auto ISItr = InitSymbols.find(NextJD);
-        if (ISItr != InitSymbols.end()) {
-          RequiredInitSymbols[NextJD] = std::move(ISItr->second);
-          InitSymbols.erase(ISItr);
+        for (auto *NextJD : DFSLinkOrder) {
+          auto ISItr = InitSymbols.find(NextJD);
+          if (ISItr != InitSymbols.end()) {
+            RequiredInitSymbols[NextJD] = std::move(ISItr->second);
+            InitSymbols.erase(ISItr);
+          }
         }
-      }
-    }
+      });
 
     return Platform::lookupInitSymbols(getExecutionSession(),
                                        RequiredInitSymbols)
@@ -435,7 +433,6 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
     return ThreadSafeModule(std::move(M), std::move(Ctx));
   }
 
-  std::mutex PlatformSupportMutex;
   LLJIT &J;
   SymbolStringPtr InitFunctionPrefix;
   DenseMap<JITDylib *, SymbolLookupSet> InitSymbols;