From 3642ab247caed9a70a31b53d2a931172cd81f015 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Fri, 13 Sep 2019 11:35:33 +0000 Subject: [PATCH] [Orc] Address the remaining move-capture FIXMEs This required spreading unique_function a bit more, which I think is a good thing. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371843 91177308-0d34-0410-b5e6-96231b3b80d8 --- examples/SpeculativeJIT/SpeculativeJIT.cpp | 4 +-- include/llvm/ExecutionEngine/JITSymbol.h | 5 +-- include/llvm/ExecutionEngine/Orc/Core.h | 3 +- .../ExecutionEngine/Orc/LazyEmittingLayer.h | 36 +++++++++---------- .../ExecutionEngine/Orc/RPCSerialization.h | 4 +-- include/llvm/ExecutionEngine/Orc/RPCUtils.h | 13 +++---- .../ExecutionEngine/Orc/RemoteObjectLayer.h | 19 ++++------ include/llvm/ExecutionEngine/RuntimeDyld.h | 23 ++++++------ include/llvm/Support/ThreadPool.h | 3 +- lib/ExecutionEngine/Orc/LLJIT.cpp | 4 +-- lib/ExecutionEngine/Orc/Legacy.cpp | 5 +-- .../Orc/RTDyldObjectLinkingLayer.cpp | 13 +++---- .../RuntimeDyld/RuntimeDyld.cpp | 18 +++++----- .../RuntimeDyld/RuntimeDyldImpl.h | 2 +- .../ExecutionEngine/Orc/CoreAPIsTest.cpp | 3 +- 15 files changed, 69 insertions(+), 86 deletions(-) diff --git a/examples/SpeculativeJIT/SpeculativeJIT.cpp b/examples/SpeculativeJIT/SpeculativeJIT.cpp index ff0c18c9584..647413cc9c2 100644 --- a/examples/SpeculativeJIT/SpeculativeJIT.cpp +++ b/examples/SpeculativeJIT/SpeculativeJIT.cpp @@ -114,9 +114,7 @@ private: this->ES->setDispatchMaterialization( [this](JITDylib &JD, std::unique_ptr MU) { - // FIXME: Switch to move capture once we have c 14. - auto SharedMU = std::shared_ptr(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)); diff --git a/include/llvm/ExecutionEngine/JITSymbol.h b/include/llvm/ExecutionEngine/JITSymbol.h index b14154c5b5e..c0f1ca4b987 100644 --- a/include/llvm/ExecutionEngine/JITSymbol.h +++ b/include/llvm/ExecutionEngine/JITSymbol.h @@ -23,6 +23,7 @@ #include #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()>; + using GetAddressFtor = unique_function()>; /// 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; using LookupResult = std::map; - using OnResolvedFunction = std::function)>; + using OnResolvedFunction = unique_function)>; virtual ~JITSymbolResolver() = default; diff --git a/include/llvm/ExecutionEngine/Orc/Core.h b/include/llvm/ExecutionEngine/Orc/Core.h index 2f06deef0c9..4f22a4c3879 100644 --- a/include/llvm/ExecutionEngine/Orc/Core.h +++ b/include/llvm/ExecutionEngine/Orc/Core.h @@ -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)>; +using SymbolsResolvedCallback = unique_function)>; /// Callback to register the dependencies for a given query. using RegisterDependenciesFunction = diff --git a/include/llvm/ExecutionEngine/Orc/LazyEmittingLayer.h b/include/llvm/ExecutionEngine/Orc/LazyEmittingLayer.h index 3f6ddba3d79..b67a9feed52 100644 --- a/include/llvm/ExecutionEngine/Orc/LazyEmittingLayer.h +++ b/include/llvm/ExecutionEngine/Orc/LazyEmittingLayer.h @@ -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 { - 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 { + 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 diff --git a/include/llvm/ExecutionEngine/Orc/RPCSerialization.h b/include/llvm/ExecutionEngine/Orc/RPCSerialization.h index 1892bf48825..752a0a34e0a 100644 --- a/include/llvm/ExecutionEngine/Orc/RPCSerialization.h +++ b/include/llvm/ExecutionEngine/Orc/RPCSerialization.h @@ -359,9 +359,9 @@ public: { assert(KeyName != nullptr && "No keyname pointer"); std::lock_guard 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)) diff --git a/include/llvm/ExecutionEngine/Orc/RPCUtils.h b/include/llvm/ExecutionEngine/Orc/RPCUtils.h index 6e08581f2dc..caea3bea997 100644 --- a/include/llvm/ExecutionEngine/Orc/RPCUtils.h +++ b/include/llvm/ExecutionEngine/Orc/RPCUtils.h @@ -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(); - auto FutureResult = Promise->get_future(); + ErrorReturnPromise Promise; + auto FutureResult = Promise.get_future(); if (auto Err = this->template appendCallAsync( - [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::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 Lock(M); --NumOutstandingCalls; diff --git a/include/llvm/ExecutionEngine/Orc/RemoteObjectLayer.h b/include/llvm/ExecutionEngine/Orc/RemoteObjectLayer.h index 9b1e0f0640d..d7304cfcf93 100644 --- a/include/llvm/ExecutionEngine/Orc/RemoteObjectLayer.h +++ b/include/llvm/ExecutionEngine/Orc/RemoteObjectLayer.h @@ -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(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(); diff --git a/include/llvm/ExecutionEngine/RuntimeDyld.h b/include/llvm/ExecutionEngine/RuntimeDyld.h index b2b4eba4707..ce7024a7f19 100644 --- a/include/llvm/ExecutionEngine/RuntimeDyld.h +++ b/include/llvm/ExecutionEngine/RuntimeDyld.h @@ -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 UnderlyingBuffer, RuntimeDyld::MemoryManager &MemMgr, JITSymbolResolver &Resolver, bool ProcessAllSections, - std::function, - std::map)> + unique_function, + std::map)> OnLoaded, - std::function OnEmitted); + unique_function 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 UnderlyingBuffer, - RuntimeDyld::MemoryManager &MemMgr, - JITSymbolResolver &Resolver, bool ProcessAllSections, - std::function, - std::map)> - OnLoaded, - std::function OnEmitted); +void jitLinkForORC( + object::ObjectFile &Obj, std::unique_ptr UnderlyingBuffer, + RuntimeDyld::MemoryManager &MemMgr, JITSymbolResolver &Resolver, + bool ProcessAllSections, + unique_function, + std::map)> + OnLoaded, + unique_function OnEmitted); } // end namespace llvm diff --git a/include/llvm/Support/ThreadPool.h b/include/llvm/Support/ThreadPool.h index 4bcbaa3142f..32f88124ee5 100644 --- a/include/llvm/Support/ThreadPool.h +++ b/include/llvm/Support/ThreadPool.h @@ -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; + using TaskTy = unique_function; using PackagedTaskTy = std::packaged_task; /// Construct a pool with the number of threads found by diff --git a/lib/ExecutionEngine/Orc/LLJIT.cpp b/lib/ExecutionEngine/Orc/LLJIT.cpp index a80f78afe80..c6532c60fb8 100644 --- a/lib/ExecutionEngine/Orc/LLJIT.cpp +++ b/lib/ExecutionEngine/Orc/LLJIT.cpp @@ -132,9 +132,7 @@ LLJIT::LLJIT(LLJITBuilderState &S, Error &Err) CompileThreads = std::make_unique(S.NumCompileThreads); ES->setDispatchMaterialization( [this](JITDylib &JD, std::unique_ptr MU) { - // FIXME: Switch to move capture once we have c++14. - auto SharedMU = std::shared_ptr(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)); }); } diff --git a/lib/ExecutionEngine/Orc/Legacy.cpp b/lib/ExecutionEngine/Orc/Legacy.cpp index ce6368b57a8..9f9a6730b2c 100644 --- a/lib/ExecutionEngine/Orc/Legacy.cpp +++ b/lib/ExecutionEngine/Orc/Legacy.cpp @@ -23,7 +23,8 @@ void JITSymbolResolverAdapter::lookup(const LookupSet &Symbols, for (auto &S : Symbols) InternedSymbols.insert(ES.intern(S)); - auto OnResolvedWithUnwrap = [OnResolved](Expected InternedResult) { + auto OnResolvedWithUnwrap = [OnResolved = std::move(OnResolved)]( + Expected InternedResult) mutable { if (!InternedResult) { OnResolved(InternedResult.takeError()); return; @@ -36,7 +37,7 @@ void JITSymbolResolverAdapter::lookup(const LookupSet &Symbols, }; auto Q = std::make_shared( - InternedSymbols, SymbolState::Resolved, OnResolvedWithUnwrap); + InternedSymbols, SymbolState::Resolved, std::move(OnResolvedWithUnwrap)); auto Unresolved = R.lookup(Q, InternedSymbols); if (Unresolved.empty()) { diff --git a/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp b/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp index 7ea1351af8f..939cd539d1f 100644 --- a/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp +++ b/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp @@ -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 InternedResult) { + [OnResolved = std::move(OnResolved)]( + Expected 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 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 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)); }); } diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp index e000b7b227e..18c74172a07 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp @@ -1180,17 +1180,15 @@ Error RuntimeDyldImpl::resolveExternalSymbols() { } void RuntimeDyldImpl::finalizeAsync( - std::unique_ptr This, std::function OnEmitted, + std::unique_ptr This, + unique_function OnEmitted, std::unique_ptr UnderlyingBuffer) { - // FIXME: Move-capture OnRelocsApplied and UnderlyingBuffer once we have - // c++14. - auto SharedUnderlyingBuffer = - std::shared_ptr(std::move(UnderlyingBuffer)); auto SharedThis = std::shared_ptr(std::move(This)); auto PostResolveContinuation = - [SharedThis, OnEmitted, SharedUnderlyingBuffer]( - Expected Result) { + [SharedThis, OnEmitted = std::move(OnEmitted), + UnderlyingBuffer = std::move(UnderlyingBuffer)]( + Expected 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()); } @@ -1400,11 +1398,11 @@ void jitLinkForORC(object::ObjectFile &Obj, std::unique_ptr UnderlyingBuffer, RuntimeDyld::MemoryManager &MemMgr, JITSymbolResolver &Resolver, bool ProcessAllSections, - std::function LoadedObj, std::map)> OnLoaded, - std::function OnEmitted) { + unique_function OnEmitted) { RuntimeDyld RTDyld(MemMgr, Resolver); RTDyld.setProcessAllSections(ProcessAllSections); diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h index 68b3468fbc9..cec7b92b8c4 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h @@ -549,7 +549,7 @@ public: void resolveLocalRelocations(); static void finalizeAsync(std::unique_ptr This, - std::function OnEmitted, + unique_function OnEmitted, std::unique_ptr UnderlyingBuffer); void reassignSectionAddress(unsigned SectionID, uint64_t Addr); diff --git a/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp index 6bb0175fd74..3e16a50d07b 100644 --- a/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp +++ b/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp @@ -1102,9 +1102,8 @@ TEST_F(CoreAPIsStandardTest, TestLookupWithThreadedMaterialization) { std::thread MaterializationThread; ES.setDispatchMaterialization( [&](JITDylib &JD, std::unique_ptr MU) { - auto SharedMU = std::shared_ptr(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}}))); -- 2.40.0