[llvm] e1d5f7d - [ORC] Add getDFSLinkOrder / getReverseDFSLinkOrder methods to JITDylib.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 29 16:04:01 PDT 2020


Author: Lang Hames
Date: 2020-08-29T15:17:06-07:00
New Revision: e1d5f7d003748843f9fd1d1b2d46d3f362b1b5cf

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

LOG: [ORC] Add getDFSLinkOrder / getReverseDFSLinkOrder methods to JITDylib.

DFS and Reverse-DFS linkage orders are used to order execution of
deinitializers and initializers respectively.

This patch replaces uses of special purpose DFS order functions in
MachOPlatform and LLJIT with uses of the new methods.

Added: 
    

Modified: 
    llvm/include/llvm/ExecutionEngine/Orc/Core.h
    llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
    llvm/lib/ExecutionEngine/Orc/Core.cpp
    llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
    llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
    llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/Core.h b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
index 101017f89def..6951df3f2d3f 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/Core.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
@@ -921,6 +921,26 @@ class JITDylib : public std::enable_shared_from_this<JITDylib> {
   Expected<SymbolNameSet>
   legacyLookup(std::shared_ptr<AsynchronousSymbolQuery> Q, SymbolNameSet Names);
 
+  /// Returns the given JITDylibs and all of their transitive dependencies in
+  /// DFS order (based on linkage relationships). Each JITDylib will appear
+  /// only once.
+  static std::vector<std::shared_ptr<JITDylib>>
+  getDFSLinkOrder(ArrayRef<std::shared_ptr<JITDylib>> JDs);
+
+  /// Returns the given JITDylibs and all of their transitive dependensies in
+  /// reverse DFS order (based on linkage relationships). Each JITDylib will
+  /// appear only once.
+  static std::vector<std::shared_ptr<JITDylib>>
+  getReverseDFSLinkOrder(ArrayRef<std::shared_ptr<JITDylib>> JDs);
+
+  /// Return this JITDylib and its transitive dependencies in DFS order
+  /// based on linkage relationships.
+  std::vector<std::shared_ptr<JITDylib>> getDFSLinkOrder();
+
+  /// Rteurn this JITDylib and its transitive dependencies in reverse DFS order
+  /// based on linkage relationships.
+  std::vector<std::shared_ptr<JITDylib>> getReverseDFSLinkOrder();
+
 private:
   using AsynchronousSymbolQueryList =
       std::vector<std::shared_ptr<AsynchronousSymbolQuery>>;

diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
index 15fe079eccaf..15cdc6bee3ce 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
@@ -136,8 +136,6 @@ class MachOPlatform : public Platform {
     InitSymbolDepMap InitSymbolDeps;
   };
 
-  static std::vector<JITDylib *> getDFSLinkOrder(JITDylib &JD);
-
   void registerInitInfo(JITDylib &JD, JITTargetAddress ObjCImageInfoAddr,
                         MachOJITDylibInitializers::SectionExtent ModInits,
                         MachOJITDylibInitializers::SectionExtent ObjCSelRefs,

diff  --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
index bad13cfebbc6..f623fc59a0a9 100644
--- a/llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -1884,6 +1884,57 @@ Expected<SymbolMap> ExecutionSession::legacyLookup(
 #endif
 }
 
+std::vector<std::shared_ptr<JITDylib>>
+JITDylib::getDFSLinkOrder(ArrayRef<std::shared_ptr<JITDylib>> JDs) {
+  if (JDs.empty())
+    return {};
+
+  auto &ES = JDs.front()->getExecutionSession();
+  return ES.runSessionLocked([&]() {
+    DenseSet<JITDylib *> Visited;
+    std::vector<std::shared_ptr<JITDylib>> Result;
+
+    for (auto &JD : JDs) {
+
+      if (Visited.count(JD.get()))
+        continue;
+
+      SmallVector<std::shared_ptr<JITDylib>, 64> WorkStack;
+      WorkStack.push_back(JD);
+      Visited.insert(JD.get());
+
+      while (!WorkStack.empty()) {
+        Result.push_back(std::move(WorkStack.back()));
+        WorkStack.pop_back();
+
+        for (auto &KV : llvm::reverse(Result.back()->LinkOrder)) {
+          auto &JD = *KV.first;
+          if (Visited.count(&JD))
+            continue;
+          Visited.insert(&JD);
+          WorkStack.push_back(JD.shared_from_this());
+        }
+      }
+    }
+    return Result;
+  });
+};
+
+std::vector<std::shared_ptr<JITDylib>>
+JITDylib::getReverseDFSLinkOrder(ArrayRef<std::shared_ptr<JITDylib>> JDs) {
+  auto Tmp = getDFSLinkOrder(JDs);
+  std::reverse(Tmp.begin(), Tmp.end());
+  return Tmp;
+}
+
+std::vector<std::shared_ptr<JITDylib>> JITDylib::getDFSLinkOrder() {
+  return getDFSLinkOrder({shared_from_this()});
+}
+
+std::vector<std::shared_ptr<JITDylib>> JITDylib::getReverseDFSLinkOrder() {
+  return getReverseDFSLinkOrder({shared_from_this()});
+}
+
 void ExecutionSession::lookup(
     LookupKind K, const JITDylibSearchOrder &SearchOrder,
     SymbolLookupSet Symbols, SymbolState RequiredState,

diff  --git a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
index 56dec8688441..373d86d92f8d 100644
--- a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
@@ -261,23 +261,23 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
       return std::move(Err);
 
     DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
-    std::vector<JITDylib *> DFSLinkOrder;
+    std::vector<std::shared_ptr<JITDylib>> DFSLinkOrder;
 
     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);
-          }
+      DFSLinkOrder = JD.getDFSLinkOrder();
+
+      for (auto &NextJD : DFSLinkOrder) {
+        auto IFItr = InitFunctions.find(NextJD.get());
+        if (IFItr != InitFunctions.end()) {
+          LookupSymbols[NextJD.get()] = std::move(IFItr->second);
+          InitFunctions.erase(IFItr);
         }
-      });
+      }
+    });
 
     LLVM_DEBUG({
       dbgs() << "JITDylib init order is [ ";
-      for (auto *JD : llvm::reverse(DFSLinkOrder))
+      for (auto &JD : llvm::reverse(DFSLinkOrder))
         dbgs() << "\"" << JD->getName() << "\" ";
       dbgs() << "]\n";
       dbgs() << "Looking up init functions:\n";
@@ -311,26 +311,26 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
     auto LLJITRunAtExits = J.mangleAndIntern("__lljit_run_atexits");
 
     DenseMap<JITDylib *, SymbolLookupSet> LookupSymbols;
-    std::vector<JITDylib *> DFSLinkOrder;
+    std::vector<std::shared_ptr<JITDylib>> DFSLinkOrder;
 
     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,
+      DFSLinkOrder = JD.getDFSLinkOrder();
+
+      for (auto &NextJD : DFSLinkOrder) {
+        auto &JDLookupSymbols = LookupSymbols[NextJD.get()];
+        auto DIFItr = DeInitFunctions.find(NextJD.get());
+        if (DIFItr != DeInitFunctions.end()) {
+          LookupSymbols[NextJD.get()] = std::move(DIFItr->second);
+          DeInitFunctions.erase(DIFItr);
+        }
+        JDLookupSymbols.add(LLJITRunAtExits,
                             SymbolLookupFlags::WeaklyReferencedSymbol);
       }
     });
 
     LLVM_DEBUG({
       dbgs() << "JITDylib deinit order is [ ";
-      for (auto *JD : DFSLinkOrder)
+      for (auto &JD : DFSLinkOrder)
         dbgs() << "\"" << JD->getName() << "\" ";
       dbgs() << "]\n";
       dbgs() << "Looking up deinit functions:\n";
@@ -344,8 +344,8 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
       return LookupResult.takeError();
 
     std::vector<JITTargetAddress> DeInitializers;
-    for (auto *NextJD : DFSLinkOrder) {
-      auto DeInitsItr = LookupResult->find(NextJD);
+    for (auto &NextJD : DFSLinkOrder) {
+      auto DeInitsItr = LookupResult->find(NextJD.get());
       assert(DeInitsItr != LookupResult->end() &&
              "Every JD should have at least __lljit_run_atexits");
 
@@ -361,46 +361,23 @@ class GenericLLVMIRPlatformSupport : public LLJIT::PlatformSupport {
     return DeInitializers;
   }
 
-  // Returns a DFS traversal order of the JITDylibs reachable (via
-  // links-against edges) from JD, starting with JD itself.
-  static std::vector<JITDylib *> getDFSLinkOrder(JITDylib &JD) {
-    std::vector<JITDylib *> DFSLinkOrder;
-    std::vector<JITDylib *> WorkStack({&JD});
-    DenseSet<JITDylib *> Visited;
-
-    while (!WorkStack.empty()) {
-      auto &NextJD = *WorkStack.back();
-      WorkStack.pop_back();
-      if (Visited.count(&NextJD))
-        continue;
-      Visited.insert(&NextJD);
-      DFSLinkOrder.push_back(&NextJD);
-      NextJD.withLinkOrderDo([&](const JITDylibSearchOrder &LinkOrder) {
-        for (auto &KV : LinkOrder)
-          WorkStack.push_back(KV.first);
-      });
-    }
-
-    return DFSLinkOrder;
-  }
-
   /// Issue lookups for all init symbols required to initialize JD (and any
   /// JITDylibs that it depends on).
   Error issueInitLookups(JITDylib &JD) {
     DenseMap<JITDylib *, SymbolLookupSet> RequiredInitSymbols;
-    std::vector<JITDylib *> DFSLinkOrder;
+    std::vector<std::shared_ptr<JITDylib>> DFSLinkOrder;
 
     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);
-          }
+      DFSLinkOrder = JD.getDFSLinkOrder();
+
+      for (auto &NextJD : DFSLinkOrder) {
+        auto ISItr = InitSymbols.find(NextJD.get());
+        if (ISItr != InitSymbols.end()) {
+          RequiredInitSymbols[NextJD.get()] = std::move(ISItr->second);
+          InitSymbols.erase(ISItr);
         }
-      });
+      }
+    });
 
     return Platform::lookupInitSymbols(getExecutionSession(),
                                        RequiredInitSymbols)

diff  --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index 15c3aa79a2a8..2d6435f8dff1 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -185,19 +185,19 @@ MachOPlatform::getInitializerSequence(JITDylib &JD) {
            << JD.getName() << "\n";
   });
 
-  std::vector<JITDylib *> DFSLinkOrder;
+  std::vector<std::shared_ptr<JITDylib>> DFSLinkOrder;
 
   while (true) {
 
     DenseMap<JITDylib *, SymbolLookupSet> NewInitSymbols;
 
     ES.runSessionLocked([&]() {
-      DFSLinkOrder = getDFSLinkOrder(JD);
+      DFSLinkOrder = JD.getDFSLinkOrder();
 
-      for (auto *InitJD : DFSLinkOrder) {
-        auto RISItr = RegisteredInitSymbols.find(InitJD);
+      for (auto &InitJD : DFSLinkOrder) {
+        auto RISItr = RegisteredInitSymbols.find(InitJD.get());
         if (RISItr != RegisteredInitSymbols.end()) {
-          NewInitSymbols[InitJD] = std::move(RISItr->second);
+          NewInitSymbols[InitJD.get()] = std::move(RISItr->second);
           RegisteredInitSymbols.erase(RISItr);
         }
       }
@@ -229,14 +229,14 @@ MachOPlatform::getInitializerSequence(JITDylib &JD) {
   InitializerSequence FullInitSeq;
   {
     std::lock_guard<std::mutex> Lock(InitSeqsMutex);
-    for (auto *InitJD : reverse(DFSLinkOrder)) {
+    for (auto &InitJD : reverse(DFSLinkOrder)) {
       LLVM_DEBUG({
         dbgs() << "MachOPlatform: Appending inits for \"" << InitJD->getName()
                << "\" to sequence\n";
       });
-      auto ISItr = InitSeqs.find(InitJD);
+      auto ISItr = InitSeqs.find(InitJD.get());
       if (ISItr != InitSeqs.end()) {
-        FullInitSeq.emplace_back(InitJD, std::move(ISItr->second));
+        FullInitSeq.emplace_back(InitJD.get(), std::move(ISItr->second));
         InitSeqs.erase(ISItr);
       }
     }
@@ -247,39 +247,19 @@ MachOPlatform::getInitializerSequence(JITDylib &JD) {
 
 Expected<MachOPlatform::DeinitializerSequence>
 MachOPlatform::getDeinitializerSequence(JITDylib &JD) {
-  std::vector<JITDylib *> DFSLinkOrder = getDFSLinkOrder(JD);
+  std::vector<std::shared_ptr<JITDylib>> DFSLinkOrder = JD.getDFSLinkOrder();
 
   DeinitializerSequence FullDeinitSeq;
   {
     std::lock_guard<std::mutex> Lock(InitSeqsMutex);
-    for (auto *DeinitJD : DFSLinkOrder) {
-      FullDeinitSeq.emplace_back(DeinitJD, MachOJITDylibDeinitializers());
+    for (auto &DeinitJD : DFSLinkOrder) {
+      FullDeinitSeq.emplace_back(DeinitJD.get(), MachOJITDylibDeinitializers());
     }
   }
 
   return FullDeinitSeq;
 }
 
-std::vector<JITDylib *> MachOPlatform::getDFSLinkOrder(JITDylib &JD) {
-  std::vector<JITDylib *> Result, WorkStack({&JD});
-  DenseSet<JITDylib *> Visited;
-
-  while (!WorkStack.empty()) {
-    auto *NextJD = WorkStack.back();
-    WorkStack.pop_back();
-    if (Visited.count(NextJD))
-      continue;
-    Visited.insert(NextJD);
-    Result.push_back(NextJD);
-    NextJD->withLinkOrderDo([&](const JITDylibSearchOrder &LO) {
-      for (auto &KV : LO)
-        WorkStack.push_back(KV.first);
-    });
-  }
-
-  return Result;
-}
-
 void MachOPlatform::registerInitInfo(
     JITDylib &JD, JITTargetAddress ObjCImageInfoAddr,
     MachOJITDylibInitializers::SectionExtent ModInits,

diff  --git a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
index 6f6e1d43af93..42d76080fdbf 100644
--- a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
@@ -1325,4 +1325,110 @@ TEST_F(CoreAPIsStandardTest, TestMaterializeWeakSymbol) {
   cantFail(FooResponsibility->notifyEmitted());
 }
 
+static bool linkOrdersEqual(const std::vector<std::shared_ptr<JITDylib>> &LHS,
+                            ArrayRef<JITDylib *> RHS) {
+  if (LHS.size() != RHS.size())
+    return false;
+  auto *RHSE = RHS.begin();
+  for (auto &LHSE : LHS)
+    if (LHSE.get() != *RHSE)
+      return false;
+    else
+      ++RHSE;
+  return true;
+};
+
+TEST(JITDylibTest, GetDFSLinkOrderTree) {
+  // Test that DFS ordering behaves as expected when the linkage relationships
+  // form a tree.
+
+  ExecutionSession ES;
+
+  auto &LibA = ES.createBareJITDylib("A");
+  auto &LibB = ES.createBareJITDylib("B");
+  auto &LibC = ES.createBareJITDylib("C");
+  auto &LibD = ES.createBareJITDylib("D");
+  auto &LibE = ES.createBareJITDylib("E");
+  auto &LibF = ES.createBareJITDylib("F");
+
+  // Linkage relationships:
+  // A --- B -- D
+  //  \      \- E
+  //    \- C -- F
+  LibA.setLinkOrder(makeJITDylibSearchOrder({&LibB, &LibC}));
+  LibB.setLinkOrder(makeJITDylibSearchOrder({&LibD, &LibE}));
+  LibC.setLinkOrder(makeJITDylibSearchOrder({&LibF}));
+
+  auto DFSOrderFromB = JITDylib::getDFSLinkOrder({LibB.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromB, {&LibB, &LibD, &LibE}))
+      << "Incorrect DFS link order for LibB";
+
+  auto DFSOrderFromA = JITDylib::getDFSLinkOrder({LibA.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromA,
+                              {&LibA, &LibB, &LibD, &LibE, &LibC, &LibF}))
+      << "Incorrect DFS link order for libA";
+
+  auto DFSOrderFromAB = JITDylib::getDFSLinkOrder(
+      {LibA.shared_from_this(), LibB.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromAB,
+                              {&LibA, &LibB, &LibD, &LibE, &LibC, &LibF}))
+      << "Incorrect DFS link order for { libA, libB }";
+
+  auto DFSOrderFromBA = JITDylib::getDFSLinkOrder(
+      {LibB.shared_from_this(), LibA.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromBA,
+                              {&LibB, &LibD, &LibE, &LibA, &LibC, &LibF}))
+      << "Incorrect DFS link order for { libB, libA }";
+}
+
+TEST(JITDylibTest, GetDFSLinkOrderDiamond) {
+  // Test that DFS ordering behaves as expected when the linkage relationships
+  // contain a diamond.
+
+  ExecutionSession ES;
+  auto &LibA = ES.createBareJITDylib("A");
+  auto &LibB = ES.createBareJITDylib("B");
+  auto &LibC = ES.createBareJITDylib("C");
+  auto &LibD = ES.createBareJITDylib("D");
+
+  // Linkage relationships:
+  // A -- B --- D
+  //  \-- C --/
+  LibA.setLinkOrder(makeJITDylibSearchOrder({&LibB, &LibC}));
+  LibB.setLinkOrder(makeJITDylibSearchOrder({&LibD}));
+  LibC.setLinkOrder(makeJITDylibSearchOrder({&LibD}));
+
+  auto DFSOrderFromA = JITDylib::getDFSLinkOrder({LibA.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromA, {&LibA, &LibB, &LibD, &LibC}))
+      << "Incorrect DFS link order for libA";
+}
+
+TEST(JITDylibTest, GetDFSLinkOrderCycle) {
+  // Test that DFS ordering behaves as expected when the linkage relationships
+  // contain a cycle.
+
+  ExecutionSession ES;
+  auto &LibA = ES.createBareJITDylib("A");
+  auto &LibB = ES.createBareJITDylib("B");
+  auto &LibC = ES.createBareJITDylib("C");
+
+  // Linkage relationships:
+  // A -- B --- C -- A
+  LibA.setLinkOrder(makeJITDylibSearchOrder({&LibB}));
+  LibB.setLinkOrder(makeJITDylibSearchOrder({&LibC}));
+  LibC.setLinkOrder(makeJITDylibSearchOrder({&LibA}));
+
+  auto DFSOrderFromA = JITDylib::getDFSLinkOrder({LibA.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromA, {&LibA, &LibB, &LibC}))
+      << "Incorrect DFS link order for libA";
+
+  auto DFSOrderFromB = JITDylib::getDFSLinkOrder({LibB.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromB, {&LibB, &LibC, &LibA}))
+      << "Incorrect DFS link order for libB";
+
+  auto DFSOrderFromC = JITDylib::getDFSLinkOrder({LibC.shared_from_this()});
+  EXPECT_TRUE(linkOrdersEqual(DFSOrderFromC, {&LibC, &LibA, &LibB}))
+      << "Incorrect DFS link order for libC";
+}
+
 } // namespace


        


More information about the llvm-commits mailing list