Fix JIT on separate thread
When JIT_IN_SEPARATE_THREAD is defined we generate Reactor routines on
LLVM in a new thread. With Variable::unmaterializedVariables now a
thread-local pointer, we have to avoid touching it from the separate
thread. This is accomplished by moving the createRet() call out of the
lambda which gets called on the separate thread.
Note that while the 'jit' object is no longer needed after
acquireRoutine(), that method may not be called since one may return
early from Reactor code generation. Its deletion was removed from
acquireRoutine() since Nucleus destruction follows quickly after and
this keeps the object's explicit lifetime management simple.
Note also that 'Variable' objects may still exist after acquireRoutine()
has been called, so the earliest we can delete the set of unmaterialized
variables is still at Nucleus object destruction when Function<> is
destructed.
Bug: b/153803432
Bug: b/149829034
Change-Id: Ic3940e5c52a3009e48fb1e1ff6137db6fd5ca96b
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/44488
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index f09787c..0d0e55b 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -641,29 +641,28 @@
std::shared_ptr<Routine> Nucleus::acquireRoutine(const char *name, const Config::Edit &cfgEdit /* = Config::Edit::None */)
{
+ if(jit->builder->GetInsertBlock()->empty() || !jit->builder->GetInsertBlock()->back().isTerminator())
+ {
+ llvm::Type *type = jit->function->getReturnType();
+
+ if(type->isVoidTy())
+ {
+ createRetVoid();
+ }
+ else
+ {
+ createRet(V(llvm::UndefValue::get(type)));
+ }
+ }
+
std::shared_ptr<Routine> routine;
- auto acquire = [&](rr::JITBuilder *jitBuilder) {
+ auto acquire = [&](rr::JITBuilder *jit) {
// ::jit is thread-local, so when this is executed on a separate thread (see JIT_IN_SEPARATE_THREAD)
- // it needs to be assigned the value from the parent thread.
- jit = jitBuilder;
+ // it needs to only use the jit variable passed in as an argument.
auto cfg = cfgEdit.apply(jit->config);
- if(jit->builder->GetInsertBlock()->empty() || !jit->builder->GetInsertBlock()->back().isTerminator())
- {
- llvm::Type *type = jit->function->getReturnType();
-
- if(type->isVoidTy())
- {
- createRetVoid();
- }
- else
- {
- createRet(V(llvm::UndefValue::get(type)));
- }
- }
-
#ifdef ENABLE_RR_DEBUG_INFO
if(jit->debugInfo != nullptr)
{
@@ -696,8 +695,6 @@
}
routine = jit->acquireRoutine(&jit->function, 1, cfg);
- delete jit;
- jit = nullptr;
};
#ifdef JIT_IN_SEPARATE_THREAD
@@ -705,10 +702,10 @@
// FIXME(b/149829034): This is not a long-term solution. Reactor has no control
// over the threading and stack sizes of its users, so this should be addressed
// at a higher level instead.
- std::thread thread(acquire, std::move(jit));
+ std::thread thread(acquire, jit);
thread.join();
#else
- acquire(std::move(jit));
+ acquire(jit);
#endif
return routine;
@@ -4297,7 +4294,9 @@
funcs[Nucleus::CoroutineEntryBegin] = jit->function;
funcs[Nucleus::CoroutineEntryAwait] = jit->coroutine.await;
funcs[Nucleus::CoroutineEntryDestroy] = jit->coroutine.destroy;
+
auto routine = jit->acquireRoutine(funcs, Nucleus::CoroutineEntryCount, cfg);
+
delete jit;
jit = nullptr;
