[llvm] r312429 - [ORC] Add an Error return to the JITCompileCallbackManager::grow method.
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 4 09:22:24 PDT 2017
On Sat, Sep 2, 2017 at 5:51 PM Lang Hames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: lhames
> Date: Sat Sep 2 17:50:42 2017
> New Revision: 312429
>
> URL: http://llvm.org/viewvc/llvm-project?rev=312429&view=rev
> Log:
> [ORC] Add an Error return to the JITCompileCallbackManager::grow method.
>
> Calling grow may result in an error if, for example, this is a callback
> manager for a remote target. We need to be able to return this error to the
> callee.
>
>
> Modified:
>
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter4/KaleidoscopeJIT.h
>
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter5/KaleidoscopeJIT.h
> llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
> llvm/trunk/include/llvm/ExecutionEngine/Orc/IndirectionUtils.h
> llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
> llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h
> llvm/trunk/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp
>
> Modified:
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter4/KaleidoscopeJIT.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter4/KaleidoscopeJIT.h?rev=312429&r1=312428&r2=312429&view=diff
>
> ==============================================================================
> ---
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter4/KaleidoscopeJIT.h
> (original)
> +++
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter4/KaleidoscopeJIT.h
> Sat Sep 2 17:50:42 2017
> @@ -135,7 +135,7 @@ public:
> Error addFunctionAST(std::unique_ptr<FunctionAST> FnAST) {
> // Create a CompileCallback - this is the re-entry point into the
> compiler
> // for functions that haven't been compiled yet.
> - auto CCInfo = CompileCallbackMgr->getCompileCallback();
> + auto CCInfo = cantFail(CompileCallbackMgr->getCompileCallback());
>
> // Create an indirect stub. This serves as the functions "canonical
> // definition" - an unchanging (constant address) entry point to the
>
> Modified:
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter5/KaleidoscopeJIT.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter5/KaleidoscopeJIT.h?rev=312429&r1=312428&r2=312429&view=diff
>
> ==============================================================================
> ---
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter5/KaleidoscopeJIT.h
> (original)
> +++
> llvm/trunk/examples/Kaleidoscope/BuildingAJIT/Chapter5/KaleidoscopeJIT.h
> Sat Sep 2 17:50:42 2017
> @@ -164,7 +164,7 @@ public:
> Error addFunctionAST(std::unique_ptr<FunctionAST> FnAST) {
> // Create a CompileCallback - this is the re-entry point into the
> compiler
> // for functions that haven't been compiled yet.
> - auto CCInfo = CompileCallbackMgr->getCompileCallback();
> + auto CCInfo = cantFail(CompileCallbackMgr->getCompileCallback());
>
> // Create an indirect stub. This serves as the functions "canonical
> // definition" - an unchanging (constant address) entry point to the
>
> Modified:
> llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h?rev=312429&r1=312428&r2=312429&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
> (original)
> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h Sat
> Sep 2 17:50:42 2017
> @@ -349,19 +349,22 @@ private:
> // Create a callback, associate it with the stub for the function,
> // and set the compile action to compile the partition containing
> the
> // function.
> - auto CCInfo = CompileCallbackMgr.getCompileCallback();
> - StubInits[MangledName] =
> - std::make_pair(CCInfo.getAddress(),
> - JITSymbolFlags::fromGlobalValue(F));
> - CCInfo.setCompileAction([this, &LD, LMId, &F]() ->
> JITTargetAddress {
> - if (auto FnImplAddrOrErr = this->extractAndCompile(LD, LMId,
> F))
> - return *FnImplAddrOrErr;
> - else {
> - // FIXME: Report error, return to 'abort' or something
> similar.
> - consumeError(FnImplAddrOrErr.takeError());
> - return 0;
> - }
> - });
> + if (auto CCInfoOrErr = CompileCallbackMgr.getCompileCallback()) {
>
As much as I like the scoping of declaring things in conditions, the
indentation this causes makes this code a bit harder to follow compared to:
auto CCInfoOrErr = ...
if (!CCInfoOrErr)
return CCInfoOrErr.takeError();
auto &CCInfo = *CCInfoOrErr;
...
& then similarly in the FnIMplAddrOrErr codepath and TrampolineAddrOrErr
below.
Just a thought - up to you though.
> + auto &CCInfo = *CCInfoOrErr;
> + StubInits[MangledName] =
> + std::make_pair(CCInfo.getAddress(),
> + JITSymbolFlags::fromGlobalValue(F));
> + CCInfo.setCompileAction([this, &LD, LMId, &F]() ->
> JITTargetAddress {
> + if (auto FnImplAddrOrErr = this->extractAndCompile(LD,
> LMId, F))
> + return *FnImplAddrOrErr;
> + else {
> + // FIXME: Report error, return to 'abort' or something
> similar.
> + consumeError(FnImplAddrOrErr.takeError());
> + return 0;
> + }
> + });
> + } else
> + return CCInfoOrErr.takeError();
> }
>
> if (auto Err = LD.StubsMgr->createStubs(StubInits))
>
> Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/IndirectionUtils.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/IndirectionUtils.h?rev=312429&r1=312428&r2=312429&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/IndirectionUtils.h
> (original)
> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/IndirectionUtils.h Sat
> Sep 2 17:50:42 2017
> @@ -105,10 +105,13 @@ public:
> }
>
> /// @brief Reserve a compile callback.
> - CompileCallbackInfo getCompileCallback() {
> - JITTargetAddress TrampolineAddr = getAvailableTrampolineAddr();
> - auto &Compile = this->ActiveTrampolines[TrampolineAddr];
> - return CompileCallbackInfo(TrampolineAddr, Compile);
> + Expected<CompileCallbackInfo> getCompileCallback() {
> + if (auto TrampolineAddrOrErr = getAvailableTrampolineAddr()) {
> + const auto &TrampolineAddr = *TrampolineAddrOrErr;
> + auto &Compile = this->ActiveTrampolines[TrampolineAddr];
> + return CompileCallbackInfo(TrampolineAddr, Compile);
> + } else
> + return TrampolineAddrOrErr.takeError();
> }
>
> /// @brief Get a CompileCallbackInfo for an existing callback.
> @@ -138,9 +141,10 @@ protected:
> std::vector<JITTargetAddress> AvailableTrampolines;
>
> private:
> - JITTargetAddress getAvailableTrampolineAddr() {
> + Expected<JITTargetAddress> getAvailableTrampolineAddr() {
> if (this->AvailableTrampolines.empty())
> - grow();
> + if (auto Err = grow())
> + return std::move(Err);
> assert(!this->AvailableTrampolines.empty() &&
> "Failed to grow available trampolines.");
> JITTargetAddress TrampolineAddr = this->AvailableTrampolines.back();
> @@ -149,7 +153,7 @@ private:
> }
>
> // Create new trampolines - to be implemented in subclasses.
> - virtual void grow() = 0;
> + virtual Error grow() = 0;
>
> virtual void anchor();
> };
> @@ -188,7 +192,7 @@ private:
> reinterpret_cast<uintptr_t>(TrampolineId)));
> }
>
> - void grow() override {
> + Error grow() override {
> assert(this->AvailableTrampolines.empty() && "Growing prematurely?");
>
> std::error_code EC;
> @@ -196,7 +200,8 @@ private:
> sys::OwningMemoryBlock(sys::Memory::allocateMappedMemory(
> sys::Process::getPageSize(), nullptr,
> sys::Memory::MF_READ | sys::Memory::MF_WRITE, EC));
> - assert(!EC && "Failed to allocate trampoline block");
> + if (EC)
> + return errorCodeToError(EC);
>
> unsigned NumTrampolines =
> (sys::Process::getPageSize() - TargetT::PointerSize) /
> @@ -211,12 +216,13 @@ private:
> static_cast<JITTargetAddress>(reinterpret_cast<uintptr_t>(
> TrampolineMem + (I * TargetT::TrampolineSize))));
>
> - EC =
> sys::Memory::protectMappedMemory(TrampolineBlock.getMemoryBlock(),
> - sys::Memory::MF_READ |
> - sys::Memory::MF_EXEC);
> - assert(!EC && "Failed to mprotect trampoline block");
> + if (auto EC = sys::Memory::protectMappedMemory(
> + TrampolineBlock.getMemoryBlock(),
> + sys::Memory::MF_READ | sys::Memory::MF_EXEC))
> + return errorCodeToError(EC);
>
> TrampolineBlocks.push_back(std::move(TrampolineBlock));
> + return Error::success();
> }
>
> sys::OwningMemoryBlock ResolverBlock;
>
> Modified:
> llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h?rev=312429&r1=312428&r2=312429&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
> (original)
> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
> Sat Sep 2 17:50:42 2017
> @@ -543,19 +543,19 @@ public:
> : JITCompileCallbackManager(ErrorHandlerAddress), Remote(Remote)
> {}
>
> private:
> - void grow() override {
> + Error grow() override {
> JITTargetAddress BlockAddr = 0;
> uint32_t NumTrampolines = 0;
> if (auto TrampolineInfoOrErr = Remote.emitTrampolineBlock())
> std::tie(BlockAddr, NumTrampolines) = *TrampolineInfoOrErr;
> - else {
> - // FIXME: Return error.
> - llvm_unreachable("Failed to create trampolines");
> - }
> + else
> + return TrampolineInfoOrErr.takeError();
>
> uint32_t TrampolineSize = Remote.getTrampolineSize();
> for (unsigned I = 0; I < NumTrampolines; ++I)
> this->AvailableTrampolines.push_back(BlockAddr + (I *
> TrampolineSize));
> +
> + return Error::success();
> }
>
> OrcRemoteTargetClient &Remote;
>
> Modified: llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h?rev=312429&r1=312428&r2=312429&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h (original)
> +++ llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h Sat Sep 2
> 17:50:42 2017
> @@ -145,12 +145,15 @@ public:
> createLazyCompileCallback(JITTargetAddress &RetAddr,
> LLVMOrcLazyCompileCallbackFn Callback,
> void *CallbackCtx) {
> - auto CCInfo = CCMgr->getCompileCallback();
> - CCInfo.setCompileAction([=]() -> JITTargetAddress {
> - return Callback(wrap(this), CallbackCtx);
> - });
> - RetAddr = CCInfo.getAddress();
> - return LLVMOrcErrSuccess;
> + if (auto CCInfoOrErr = CCMgr->getCompileCallback()) {
> + auto &CCInfo = *CCInfoOrErr;
> + CCInfo.setCompileAction([=]() -> JITTargetAddress {
> + return Callback(wrap(this), CallbackCtx);
> + });
> + RetAddr = CCInfo.getAddress();
> + return LLVMOrcErrSuccess;
> + } else
> + return mapError(CCInfoOrErr.takeError());
> }
>
> LLVMOrcErrorCode createIndirectStub(StringRef StubName,
>
> Modified:
> llvm/trunk/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp?rev=312429&r1=312428&r2=312429&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp
> (original)
> +++ llvm/trunk/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp
> Sat Sep 2 17:50:42 2017
> @@ -21,7 +21,7 @@ public:
> DummyCallbackManager() : JITCompileCallbackManager(0) {}
>
> public:
> - void grow() override { llvm_unreachable("not implemented"); }
> + Error grow() override { llvm_unreachable("not implemented"); }
> };
>
> class DummyStubsManager : public orc::IndirectStubsManager {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170904/fe19b6b5/attachment.html>
More information about the llvm-commits
mailing list