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