[llvm] 76c59a6 - [ORC] Decompose LazyCallThroughManager::callThroughToSymbol()

[Stefan Gränitz via llvm-commits]

Author: Stefan Gränitz
Date: 2020-03-05T00:24:23+01:00
New Revision: 76c59a63bc7f19f264f44a1a44f0a67193ad79de

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

LOG: [ORC] Decompose LazyCallThroughManager::callThroughToSymbol()

Summary: Decompose callThroughToSymbol() into findReexport(), resolveSymbol(), notifyResolved() and reportCallThroughError(). This allows derived classes to reuse the functionality while adding their own code in between.

Reviewers: lhames

Reviewed By: lhames

Subscribers: hiraditya, steven_wu, dexonsmith, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D75084

Added: 
    

Modified: 
    llvm/include/llvm/ExecutionEngine/Orc/LazyReexports.h
    llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/LazyReexports.h b/llvm/include/llvm/ExecutionEngine/Orc/LazyReexports.h
index fd34556ff959..407dc6b7e34f 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/LazyReexports.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/LazyReexports.h
@@ -51,15 +51,28 @@ class LazyCallThroughManager {
                          JITTargetAddress ErrorHandlerAddr,
                          std::unique_ptr<TrampolinePool> TP);
 
-  JITTargetAddress callThroughToSymbol(JITTargetAddress TrampolineAddr);
+  struct ReexportsEntry {
+    JITDylib *SourceJD;
+    SymbolStringPtr SymbolName;
+  };
+
+  Expected<ReexportsEntry> findReexport(JITTargetAddress TrampolineAddr);
+  Expected<JITTargetAddress> resolveSymbol(const ReexportsEntry &RE);
+
+  Error notifyResolved(JITTargetAddress TrampolineAddr,
+                       JITTargetAddress ResolvedAddr);
+
+  JITTargetAddress reportCallThroughError(Error Err) {
+    ES.reportError(std::move(Err));
+    return ErrorHandlerAddr;
+  }
 
   void setTrampolinePool(std::unique_ptr<TrampolinePool> TP) {
     this->TP = std::move(TP);
   }
 
 private:
-  using ReexportsMap =
-      std::map<JITTargetAddress, std::pair<JITDylib *, SymbolStringPtr>>;
+  using ReexportsMap = std::map<JITTargetAddress, ReexportsEntry>;
 
   using NotifiersMap = std::map<JITTargetAddress, NotifyResolvedFunction>;
 
@@ -91,6 +104,21 @@ class LocalLazyCallThroughManager : public LazyCallThroughManager {
     return Error::success();
   }
 
+  JITTargetAddress callThroughToSymbol(JITTargetAddress TrampolineAddr) {
+    auto Entry = findReexport(TrampolineAddr);
+    if (!Entry)
+      return reportCallThroughError(Entry.takeError());
+
+    auto ResolvedAddr = resolveSymbol(std::move(*Entry));
+    if (!ResolvedAddr)
+      return reportCallThroughError(ResolvedAddr.takeError());
+
+    if (Error Err = notifyResolved(TrampolineAddr, *ResolvedAddr))
+      return reportCallThroughError(std::move(Err));
+
+    return *ResolvedAddr;
+  }
+
 public:
   /// Create a LocalLazyCallThroughManager using the given ABI. See
   /// createLocalLazyCallThroughManager.

diff  --git a/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp b/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
index 27d08e18270f..2812159b0076 100644
--- a/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
@@ -30,36 +30,37 @@ Expected<JITTargetAddress> LazyCallThroughManager::getCallThroughTrampoline(
   if (!Trampoline)
     return Trampoline.takeError();
 
-  Reexports[*Trampoline] = std::make_pair(&SourceJD, std::move(SymbolName));
+  Reexports[*Trampoline] = ReexportsEntry{&SourceJD, std::move(SymbolName)};
   Notifiers[*Trampoline] = std::move(NotifyResolved);
   return *Trampoline;
 }
 
-JITTargetAddress
-LazyCallThroughManager::callThroughToSymbol(JITTargetAddress TrampolineAddr) {
-  JITDylib *SourceJD = nullptr;
-  SymbolStringPtr SymbolName;
+Expected<LazyCallThroughManager::ReexportsEntry>
+LazyCallThroughManager::findReexport(JITTargetAddress TrampolineAddr) {
+  std::lock_guard<std::mutex> Lock(LCTMMutex);
+  auto I = Reexports.find(TrampolineAddr);
+  if (I == Reexports.end())
+    return createStringError(inconvertibleErrorCode(),
+                             "Missing reexport for trampoline address %p",
+                             TrampolineAddr);
+  return I->second;
+}
 
-  {
-    std::lock_guard<std::mutex> Lock(LCTMMutex);
-    auto I = Reexports.find(TrampolineAddr);
-    if (I == Reexports.end())
-      return ErrorHandlerAddr;
-    SourceJD = I->second.first;
-    SymbolName = I->second.second;
-  }
+Expected<JITTargetAddress>
+LazyCallThroughManager::resolveSymbol(const ReexportsEntry &RE) {
+  auto LookupResult =
+      ES.lookup(makeJITDylibSearchOrder(RE.SourceJD,
+                                        JITDylibLookupFlags::MatchAllSymbols),
+                RE.SymbolName, SymbolState::Ready);
 
-  auto LookupResult = ES.lookup(
-      makeJITDylibSearchOrder(SourceJD, JITDylibLookupFlags::MatchAllSymbols),
-      SymbolName, SymbolState::Ready);
+  if (!LookupResult)
+    return LookupResult.takeError();
 
-  if (!LookupResult) {
-    ES.reportError(LookupResult.takeError());
-    return ErrorHandlerAddr;
-  }
-
-  auto ResolvedAddr = LookupResult->getAddress();
+  return LookupResult->getAddress();
+}
 
+Error LazyCallThroughManager::notifyResolved(JITTargetAddress TrampolineAddr,
+                                             JITTargetAddress ResolvedAddr) {
   NotifyResolvedFunction NotifyResolved;
   {
     std::lock_guard<std::mutex> Lock(LCTMMutex);
@@ -70,14 +71,7 @@ LazyCallThroughManager::callThroughToSymbol(JITTargetAddress TrampolineAddr) {
     }
   }
 
-  if (NotifyResolved) {
-    if (auto Err = NotifyResolved(ResolvedAddr)) {
-      ES.reportError(std::move(Err));
-      return ErrorHandlerAddr;
-    }
-  }
-
-  return ResolvedAddr;
+  return NotifyResolved ? NotifyResolved(ResolvedAddr) : Error::success();
 }
 
 Expected<std::unique_ptr<LazyCallThroughManager>>