[Orc] Address the remaining move-capture FIXMEs
authorBenjamin Kramer <benny.kra@googlemail.com>
Fri, 13 Sep 2019 11:35:33 +0000 (11:35 +0000)
committerBenjamin Kramer <benny.kra@googlemail.com>
Fri, 13 Sep 2019 11:35:33 +0000 (11:35 +0000)
This required spreading unique_function a bit more, which I think is a
good thing.

llvm-svn: 371843

15 files changed:
llvm/examples/SpeculativeJIT/SpeculativeJIT.cpp
llvm/include/llvm/ExecutionEngine/JITSymbol.h
llvm/include/llvm/ExecutionEngine/Orc/Core.h
llvm/include/llvm/ExecutionEngine/Orc/LazyEmittingLayer.h
llvm/include/llvm/ExecutionEngine/Orc/RPCSerialization.h
llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h
llvm/include/llvm/ExecutionEngine/Orc/RemoteObjectLayer.h
llvm/include/llvm/ExecutionEngine/RuntimeDyld.h
llvm/include/llvm/Support/ThreadPool.h
llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
llvm/lib/ExecutionEngine/Orc/Legacy.cpp
llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp

index ff0c18c..647413c 100644 (file)
@@ -114,9 +114,7 @@ private:
     this->ES->setDispatchMaterialization(
 
         [this](JITDylib &JD, std::unique_ptr<MaterializationUnit> MU) {
-          // FIXME: Switch to move capture once we have c  14.
-          auto SharedMU = std::shared_ptr<MaterializationUnit>(std::move(MU));
-          auto Work = [SharedMU, &JD]() { SharedMU->doMaterialize(JD); };
+          auto Work = [MU = std::move(MU), &JD] { MU->doMaterialize(JD); };
           CompileThreads.async(std::move(Work));
         });
     ExitOnErr(S.addSpeculationRuntime(this->ES->getMainJITDylib(), Mangle));
index b14154c..c0f1ca4 100644 (file)
@@ -23,6 +23,7 @@
 #include <string>
 
 #include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
@@ -217,7 +218,7 @@ private:
 /// Represents a symbol in the JIT.
 class JITSymbol {
 public:
-  using GetAddressFtor = std::function<Expected<JITTargetAddress>()>;
+  using GetAddressFtor = unique_function<Expected<JITTargetAddress>()>;
 
   /// Create a 'null' symbol, used to represent a "symbol not found"
   ///        result from a successful (non-erroneous) lookup.
@@ -325,7 +326,7 @@ class JITSymbolResolver {
 public:
   using LookupSet = std::set<StringRef>;
   using LookupResult = std::map<StringRef, JITEvaluatedSymbol>;
-  using OnResolvedFunction = std::function<void(Expected<LookupResult>)>;
+  using OnResolvedFunction = unique_function<void(Expected<LookupResult>)>;
 
   virtual ~JITSymbolResolver() = default;
 
index 2f06dee..4f22a4c 100644 (file)
@@ -14,6 +14,7 @@
 #define LLVM_EXECUTIONENGINE_ORC_CORE_H
 
 #include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ExecutionEngine/JITSymbol.h"
 #include "llvm/ExecutionEngine/Orc/SymbolStringPool.h"
 #include "llvm/ExecutionEngine/OrcV1Deprecation.h"
@@ -107,7 +108,7 @@ raw_ostream &operator<<(raw_ostream &OS, const SymbolAliasMap &Aliases);
 raw_ostream &operator<<(raw_ostream &OS, const SymbolState &S);
 
 /// Callback to notify client that symbols have been resolved.
-using SymbolsResolvedCallback = std::function<void(Expected<SymbolMap>)>;
+using SymbolsResolvedCallback = unique_function<void(Expected<SymbolMap>)>;
 
 /// Callback to register the dependencies for a given query.
 using RegisterDependenciesFunction =
index 3f6ddba..b67a9fe 100644 (file)
@@ -49,28 +49,24 @@ private:
       switch (EmitState) {
       case NotEmitted:
         if (auto GV = searchGVs(Name, ExportedSymbolsOnly)) {
-          // Create a std::string version of Name to capture here - the argument
-          // (a StringRef) may go away before the lambda is executed.
-          // FIXME: Use capture-init when we move to C++14.
-          std::string PName = Name;
           JITSymbolFlags Flags = JITSymbolFlags::fromGlobalValue(*GV);
-          auto GetAddress =
-            [this, ExportedSymbolsOnly, PName, &B]() -> Expected<JITTargetAddress> {
-              if (this->EmitState == Emitting)
-                return 0;
-              else if (this->EmitState == NotEmitted) {
-                this->EmitState = Emitting;
-                if (auto Err = this->emitToBaseLayer(B))
-                  return std::move(Err);
-                this->EmitState = Emitted;
-              }
-              if (auto Sym = B.findSymbolIn(K, PName, ExportedSymbolsOnly))
-                return Sym.getAddress();
-              else if (auto Err = Sym.takeError())
+          auto GetAddress = [this, ExportedSymbolsOnly, Name = Name.str(),
+                             &B]() -> Expected<JITTargetAddress> {
+            if (this->EmitState == Emitting)
+              return 0;
+            else if (this->EmitState == NotEmitted) {
+              this->EmitState = Emitting;
+              if (auto Err = this->emitToBaseLayer(B))
                 return std::move(Err);
-              else
-                llvm_unreachable("Successful symbol lookup should return "
-                                 "definition address here");
+              this->EmitState = Emitted;
+            }
+            if (auto Sym = B.findSymbolIn(K, Name, ExportedSymbolsOnly))
+              return Sym.getAddress();
+            else if (auto Err = Sym.takeError())
+              return std::move(Err);
+            else
+              llvm_unreachable("Successful symbol lookup should return "
+                               "definition address here");
           };
           return JITSymbol(std::move(GetAddress), Flags);
         } else
index 1892bf4..752a0a3 100644 (file)
@@ -359,9 +359,9 @@ public:
     {
       assert(KeyName != nullptr && "No keyname pointer");
       std::lock_guard<std::recursive_mutex> Lock(SerializersMutex);
-      // FIXME: Move capture Serialize once we have C++14.
       Serializers[ErrorInfoT::classID()] =
-          [KeyName, Serialize](ChannelT &C, const ErrorInfoBase &EIB) -> Error {
+          [KeyName, Serialize = std::move(Serialize)](
+              ChannelT &C, const ErrorInfoBase &EIB) -> Error {
         assert(EIB.dynamicClassID() == ErrorInfoT::classID() &&
                "Serializer called for wrong error type");
         if (auto Err = serializeSeq(C, *KeyName))
index 6e08581..caea3be 100644 (file)
@@ -1413,14 +1413,12 @@ public:
     using ErrorReturn = typename RTraits::ErrorReturnType;
     using ErrorReturnPromise = typename RTraits::ReturnPromiseType;
 
-    // FIXME: Stack allocate and move this into the handler once LLVM builds
-    //        with C++14.
-    auto Promise = std::make_shared<ErrorReturnPromise>();
-    auto FutureResult = Promise->get_future();
+    ErrorReturnPromise Promise;
+    auto FutureResult = Promise.get_future();
 
     if (auto Err = this->template appendCallAsync<Func>(
-            [Promise](ErrorReturn RetOrErr) {
-              Promise->set_value(std::move(RetOrErr));
+            [Promise = std::move(Promise)](ErrorReturn RetOrErr) {
+              Promise.set_value(std::move(RetOrErr));
               return Error::success();
             },
             Args...)) {
@@ -1598,8 +1596,7 @@ public:
     // outstanding calls count, then poke the condition variable.
     using ArgType = typename detail::ResponseHandlerArg<
         typename detail::HandlerTraits<HandlerT>::Type>::ArgType;
-    // FIXME: Move handler into wrapped handler once we have C++14.
-    auto WrappedHandler = [this, Handler](ArgType Arg) {
+    auto WrappedHandler = [this, Handler = std::move(Handler)](ArgType Arg) {
       auto Err = Handler(std::move(Arg));
       std::unique_lock<std::mutex> Lock(M);
       --NumOutstandingCalls;
index 9b1e0f0..d7304cf 100644 (file)
@@ -137,17 +137,12 @@ protected:
                              RemoteSymbolId Id)
       : C(C), Id(Id) {}
 
-    RemoteSymbolMaterializer(const RemoteSymbolMaterializer &Other)
-      : C(Other.C), Id(Other.Id) {
-      // FIXME: This is a horrible, auto_ptr-style, copy-as-move operation.
-      //        It should be removed as soon as LLVM has C++14's generalized
-      //        lambda capture (at which point the materializer can be moved
-      //        into the lambda in remoteToJITSymbol below).
-      const_cast<RemoteSymbolMaterializer&>(Other).Id = 0;
+    RemoteSymbolMaterializer(RemoteSymbolMaterializer &&Other)
+        : C(Other.C), Id(Other.Id) {
+      Other.Id = 0;
     }
 
-    RemoteSymbolMaterializer&
-    operator=(const RemoteSymbolMaterializer&) = delete;
+    RemoteSymbolMaterializer &operator=(RemoteSymbolMaterializer &&) = delete;
 
     /// Release the remote symbol.
     ~RemoteSymbolMaterializer() {
@@ -218,9 +213,9 @@ protected:
         return nullptr;
       // else...
       RemoteSymbolMaterializer RSM(*this, RemoteSym.first);
-      auto Sym =
-        JITSymbol([RSM]() mutable { return RSM.materialize(); },
-                  RemoteSym.second);
+      auto Sym = JITSymbol(
+          [RSM = std::move(RSM)]() mutable { return RSM.materialize(); },
+          RemoteSym.second);
       return Sym;
     } else
       return RemoteSymOrErr.takeError();
index b2b4eba..ce7024a 100644 (file)
@@ -13,6 +13,7 @@
 #ifndef LLVM_EXECUTIONENGINE_RUNTIMEDYLD_H
 #define LLVM_EXECUTIONENGINE_RUNTIMEDYLD_H
 
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/DebugInfo/DIContext.h"
@@ -271,10 +272,10 @@ private:
                 std::unique_ptr<MemoryBuffer> UnderlyingBuffer,
                 RuntimeDyld::MemoryManager &MemMgr, JITSymbolResolver &Resolver,
                 bool ProcessAllSections,
-                std::function<Error(std::unique_ptr<LoadedObjectInfo>,
-                                    std::map<StringRef, JITEvaluatedSymbol>)>
+                unique_function<Error(std::unique_ptr<LoadedObjectInfo>,
+                                      std::map<StringRef, JITEvaluatedSymbol>)>
                     OnLoaded,
-                std::function<void(Error)> OnEmitted);
+                unique_function<void(Error)> OnEmitted);
 
   // RuntimeDyldImpl is the actual class. RuntimeDyld is just the public
   // interface.
@@ -291,14 +292,14 @@ private:
 // but ORC's RTDyldObjectLinkingLayer2. Internally it constructs a RuntimeDyld
 // instance and uses continuation passing to perform the fix-up and finalize
 // steps asynchronously.
-void jitLinkForORC(object::ObjectFile &Obj,
-                   std::unique_ptr<MemoryBuffer> UnderlyingBuffer,
-                   RuntimeDyld::MemoryManager &MemMgr,
-                   JITSymbolResolver &Resolver, bool ProcessAllSections,
-                   std::function<Error(std::unique_ptr<LoadedObjectInfo>,
-                                       std::map<StringRef, JITEvaluatedSymbol>)>
-                       OnLoaded,
-                   std::function<void(Error)> OnEmitted);
+void jitLinkForORC(
+    object::ObjectFile &Obj, std::unique_ptr<MemoryBuffer> UnderlyingBuffer,
+    RuntimeDyld::MemoryManager &MemMgr, JITSymbolResolver &Resolver,
+    bool ProcessAllSections,
+    unique_function<Error(std::unique_ptr<RuntimeDyld::LoadedObjectInfo>,
+                          std::map<StringRef, JITEvaluatedSymbol>)>
+        OnLoaded,
+    unique_function<void(Error)> OnEmitted);
 
 } // end namespace llvm
 
index 4bcbaa3..32f8812 100644 (file)
@@ -13,6 +13,7 @@
 #ifndef LLVM_SUPPORT_THREAD_POOL_H
 #define LLVM_SUPPORT_THREAD_POOL_H
 
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/thread.h"
 
@@ -35,7 +36,7 @@ namespace llvm {
 /// for some work to become available.
 class ThreadPool {
 public:
-  using TaskTy = std::function<void()>;
+  using TaskTy = unique_function<void()>;
   using PackagedTaskTy = std::packaged_task<void()>;
 
   /// Construct a pool with the number of threads found by
index a80f78a..c6532c6 100644 (file)
@@ -132,9 +132,7 @@ LLJIT::LLJIT(LLJITBuilderState &S, Error &Err)
     CompileThreads = std::make_unique<ThreadPool>(S.NumCompileThreads);
     ES->setDispatchMaterialization(
         [this](JITDylib &JD, std::unique_ptr<MaterializationUnit> MU) {
-          // FIXME: Switch to move capture once we have c++14.
-          auto SharedMU = std::shared_ptr<MaterializationUnit>(std::move(MU));
-          auto Work = [SharedMU, &JD]() { SharedMU->doMaterialize(JD); };
+          auto Work = [MU = std::move(MU), &JD] { MU->doMaterialize(JD); };
           CompileThreads->async(std::move(Work));
         });
   }
index ce6368b..9f9a673 100644 (file)
@@ -23,7 +23,8 @@ void JITSymbolResolverAdapter::lookup(const LookupSet &Symbols,
   for (auto &S : Symbols)
     InternedSymbols.insert(ES.intern(S));
 
-  auto OnResolvedWithUnwrap = [OnResolved](Expected<SymbolMap> InternedResult) {
+  auto OnResolvedWithUnwrap = [OnResolved = std::move(OnResolved)](
+                                  Expected<SymbolMap> InternedResult) mutable {
     if (!InternedResult) {
       OnResolved(InternedResult.takeError());
       return;
@@ -36,7 +37,7 @@ void JITSymbolResolverAdapter::lookup(const LookupSet &Symbols,
   };
 
   auto Q = std::make_shared<AsynchronousSymbolQuery>(
-      InternedSymbols, SymbolState::Resolved, OnResolvedWithUnwrap);
+      InternedSymbols, SymbolState::Resolved, std::move(OnResolvedWithUnwrap));
 
   auto Unresolved = R.lookup(Q, InternedSymbols);
   if (Unresolved.empty()) {
index 7ea1351..939cd53 100644 (file)
@@ -27,9 +27,9 @@ public:
 
     // Build an OnResolve callback to unwrap the interned strings and pass them
     // to the OnResolved callback.
-    // FIXME: Switch to move capture of OnResolved once we have c++14.
     auto OnResolvedWithUnwrap =
-        [OnResolved](Expected<SymbolMap> InternedResult) {
+        [OnResolved = std::move(OnResolved)](
+            Expected<SymbolMap> InternedResult) mutable {
           if (!InternedResult) {
             OnResolved(InternedResult.takeError());
             return;
@@ -50,7 +50,7 @@ public:
     MR.getTargetJITDylib().withSearchOrderDo(
         [&](const JITDylibSearchList &JDs) { SearchOrder = JDs; });
     ES.lookup(SearchOrder, InternedSymbols, SymbolState::Resolved,
-              OnResolvedWithUnwrap, RegisterDependencies);
+              std::move(OnResolvedWithUnwrap), RegisterDependencies);
   }
 
   Expected<LookupSet> getResponsibilitySet(const LookupSet &Symbols) {
@@ -133,8 +133,6 @@ void RTDyldObjectLinkingLayer::emit(MaterializationResponsibility R,
 
   JITDylibSearchOrderResolver Resolver(*SharedR);
 
-  // FIXME: Switch to move-capture for the 'O' buffer once we have c++14.
-  MemoryBuffer *UnownedObjBuffer = O.release();
   jitLinkForORC(
       **Obj, std::move(O), *MemMgr, Resolver, ProcessAllSections,
       [this, K, SharedR, &Obj, InternalSymbols](
@@ -143,9 +141,8 @@ void RTDyldObjectLinkingLayer::emit(MaterializationResponsibility R,
         return onObjLoad(K, *SharedR, **Obj, std::move(LoadedObjInfo),
                          ResolvedSymbols, *InternalSymbols);
       },
-      [this, K, SharedR, UnownedObjBuffer](Error Err) {
-        std::unique_ptr<MemoryBuffer> ObjBuffer(UnownedObjBuffer);
-        onObjEmit(K, std::move(ObjBuffer), *SharedR, std::move(Err));
+      [this, K, SharedR, O = std::move(O)](Error Err) mutable {
+        onObjEmit(K, std::move(O), *SharedR, std::move(Err));
       });
 }
 
index e000b7b..18c7417 100644 (file)
@@ -1180,17 +1180,15 @@ Error RuntimeDyldImpl::resolveExternalSymbols() {
 }
 
 void RuntimeDyldImpl::finalizeAsync(
-    std::unique_ptr<RuntimeDyldImpl> This, std::function<void(Error)> OnEmitted,
+    std::unique_ptr<RuntimeDyldImpl> This,
+    unique_function<void(Error)> OnEmitted,
     std::unique_ptr<MemoryBuffer> UnderlyingBuffer) {
 
-  // FIXME: Move-capture OnRelocsApplied and UnderlyingBuffer once we have
-  // c++14.
-  auto SharedUnderlyingBuffer =
-      std::shared_ptr<MemoryBuffer>(std::move(UnderlyingBuffer));
   auto SharedThis = std::shared_ptr<RuntimeDyldImpl>(std::move(This));
   auto PostResolveContinuation =
-      [SharedThis, OnEmitted, SharedUnderlyingBuffer](
-          Expected<JITSymbolResolver::LookupResult> Result) {
+      [SharedThis, OnEmitted = std::move(OnEmitted),
+       UnderlyingBuffer = std::move(UnderlyingBuffer)](
+          Expected<JITSymbolResolver::LookupResult> Result) mutable {
         if (!Result) {
           OnEmitted(Result.takeError());
           return;
@@ -1224,7 +1222,7 @@ void RuntimeDyldImpl::finalizeAsync(
   }
 
   if (!Symbols.empty()) {
-    SharedThis->Resolver.lookup(Symbols, PostResolveContinuation);
+    SharedThis->Resolver.lookup(Symbols, std::move(PostResolveContinuation));
   } else
     PostResolveContinuation(std::map<StringRef, JITEvaluatedSymbol>());
 }
@@ -1400,11 +1398,11 @@ void jitLinkForORC(object::ObjectFile &Obj,
                    std::unique_ptr<MemoryBuffer> UnderlyingBuffer,
                    RuntimeDyld::MemoryManager &MemMgr,
                    JITSymbolResolver &Resolver, bool ProcessAllSections,
-                   std::function<Error(
+                   unique_function<Error(
                        std::unique_ptr<RuntimeDyld::LoadedObjectInfo> LoadedObj,
                        std::map<StringRef, JITEvaluatedSymbol>)>
                        OnLoaded,
-                   std::function<void(Error)> OnEmitted) {
+                   unique_function<void(Error)> OnEmitted) {
 
   RuntimeDyld RTDyld(MemMgr, Resolver);
   RTDyld.setProcessAllSections(ProcessAllSections);
index 68b3468..cec7b92 100644 (file)
@@ -549,7 +549,7 @@ public:
   void resolveLocalRelocations();
 
   static void finalizeAsync(std::unique_ptr<RuntimeDyldImpl> This,
-                            std::function<void(Error)> OnEmitted,
+                            unique_function<void(Error)> OnEmitted,
                             std::unique_ptr<MemoryBuffer> UnderlyingBuffer);
 
   void reassignSectionAddress(unsigned SectionID, uint64_t Addr);
index 6bb0175..3e16a50 100644 (file)
@@ -1102,9 +1102,8 @@ TEST_F(CoreAPIsStandardTest, TestLookupWithThreadedMaterialization) {
   std::thread MaterializationThread;
   ES.setDispatchMaterialization(
       [&](JITDylib &JD, std::unique_ptr<MaterializationUnit> MU) {
-        auto SharedMU = std::shared_ptr<MaterializationUnit>(std::move(MU));
         MaterializationThread =
-            std::thread([SharedMU, &JD]() { SharedMU->doMaterialize(JD); });
+            std::thread([MU = std::move(MU), &JD] { MU->doMaterialize(JD); });
       });
 
   cantFail(JD.define(absoluteSymbols({{Foo, FooSym}})));