Commit 7d6b5913 by Nicolas Capens Committed by Nicolas Capens

Avoid implicitly destructing thread-locals

Chrome supports Linux versions which don't come with a glibc library which supports __cxa_thread_atexit_impl. This function is required for destroying C++11 thread_local objects at thread exit. The 'jit' and 'unmaterializedVariables' thread-local objects used by Reactor are only needed between Function<> creation and Routine acquisition, so replace them by explicitly managed raw pointers. Bug: chromium:1074222 Bug: b/153803432 Change-Id: I0a92bdd30940048b9ce0c208d3a2c1a952091283 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/44428 Presubmit-Ready: Nicolas Capens <nicolascapens@google.com> Kokoro-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarBen Clayton <bclayton@google.com> Reviewed-by: 's avatarAntonio Maiorano <amaiorano@google.com> Tested-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent 941293d5
...@@ -65,7 +65,9 @@ extern "C" void X86CompilationCallback() ...@@ -65,7 +65,9 @@ extern "C" void X86CompilationCallback()
namespace { namespace {
thread_local std::unique_ptr<rr::JITBuilder> jit; // This has to be a raw pointer because glibc 2.17 doesn't support __cxa_thread_atexit_impl
// for destructing objects at exit. See crbug.com/1074222
thread_local rr::JITBuilder *jit = nullptr;
// Default configuration settings. Must be accessed under mutex lock. // Default configuration settings. Must be accessed under mutex lock.
std::mutex defaultConfigLock; std::mutex defaultConfigLock;
...@@ -603,12 +605,19 @@ static ::llvm::Function *createFunction(const char *name, ::llvm::Type *retTy, c ...@@ -603,12 +605,19 @@ static ::llvm::Function *createFunction(const char *name, ::llvm::Type *retTy, c
Nucleus::Nucleus() Nucleus::Nucleus()
{ {
ASSERT(jit == nullptr); ASSERT(jit == nullptr);
jit.reset(new JITBuilder(Nucleus::getDefaultConfig())); jit = new JITBuilder(Nucleus::getDefaultConfig());
ASSERT(Variable::unmaterializedVariables == nullptr);
Variable::unmaterializedVariables = new std::unordered_set<Variable *>();
} }
Nucleus::~Nucleus() Nucleus::~Nucleus()
{ {
jit.reset(); delete Variable::unmaterializedVariables;
Variable::unmaterializedVariables = nullptr;
delete jit;
jit = nullptr;
} }
void Nucleus::setDefaultConfig(const Config &cfg) void Nucleus::setDefaultConfig(const Config &cfg)
...@@ -634,10 +643,10 @@ std::shared_ptr<Routine> Nucleus::acquireRoutine(const char *name, const Config: ...@@ -634,10 +643,10 @@ std::shared_ptr<Routine> Nucleus::acquireRoutine(const char *name, const Config:
{ {
std::shared_ptr<Routine> routine; std::shared_ptr<Routine> routine;
auto acquire = [&](std::unique_ptr<rr::JITBuilder> jitBuilder) { auto acquire = [&](rr::JITBuilder *jitBuilder) {
// ::jit is thread-local, so when this is executed on a separate thread (see JIT_IN_SEPARATE_THREAD) // ::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. // it needs to be assigned the value from the parent thread.
jit = std::move(jitBuilder); jit = jitBuilder;
auto cfg = cfgEdit.apply(jit->config); auto cfg = cfgEdit.apply(jit->config);
...@@ -687,7 +696,8 @@ std::shared_ptr<Routine> Nucleus::acquireRoutine(const char *name, const Config: ...@@ -687,7 +696,8 @@ std::shared_ptr<Routine> Nucleus::acquireRoutine(const char *name, const Config:
} }
routine = jit->acquireRoutine(&jit->function, 1, cfg); routine = jit->acquireRoutine(&jit->function, 1, cfg);
jit.reset(); delete jit;
jit = nullptr;
}; };
#ifdef JIT_IN_SEPARATE_THREAD #ifdef JIT_IN_SEPARATE_THREAD
...@@ -4288,7 +4298,8 @@ std::shared_ptr<Routine> Nucleus::acquireCoroutine(const char *name, const Confi ...@@ -4288,7 +4298,8 @@ std::shared_ptr<Routine> Nucleus::acquireCoroutine(const char *name, const Confi
funcs[Nucleus::CoroutineEntryAwait] = jit->coroutine.await; funcs[Nucleus::CoroutineEntryAwait] = jit->coroutine.await;
funcs[Nucleus::CoroutineEntryDestroy] = jit->coroutine.destroy; funcs[Nucleus::CoroutineEntryDestroy] = jit->coroutine.destroy;
auto routine = jit->acquireRoutine(funcs, Nucleus::CoroutineEntryCount, cfg); auto routine = jit->acquireRoutine(funcs, Nucleus::CoroutineEntryCount, cfg);
jit.reset(); delete jit;
jit = nullptr;
return routine; return routine;
} }
......
...@@ -64,7 +64,7 @@ void rr::Config::Edit::apply(const std::vector<std::pair<ListEdit, T>> &edits, s ...@@ -64,7 +64,7 @@ void rr::Config::Edit::apply(const std::vector<std::pair<ListEdit, T>> &edits, s
} }
// Set of variables that do not have a stack location yet. // Set of variables that do not have a stack location yet.
thread_local std::unordered_set<Variable *> Variable::unmaterializedVariables; thread_local std::unordered_set<Variable *> *Variable::unmaterializedVariables = nullptr;
Variable::Variable(Type *type, int arraySize) Variable::Variable(Type *type, int arraySize)
: arraySize(arraySize) : arraySize(arraySize)
...@@ -73,28 +73,28 @@ Variable::Variable(Type *type, int arraySize) ...@@ -73,28 +73,28 @@ Variable::Variable(Type *type, int arraySize)
#if REACTOR_MATERIALIZE_LVALUES_ON_DEFINITION #if REACTOR_MATERIALIZE_LVALUES_ON_DEFINITION
materialize(); materialize();
#else #else
unmaterializedVariables.emplace(this); unmaterializedVariables->emplace(this);
#endif #endif
} }
Variable::~Variable() Variable::~Variable()
{ {
unmaterializedVariables.erase(this); unmaterializedVariables->erase(this);
} }
void Variable::materializeAll() void Variable::materializeAll()
{ {
for(auto *var : unmaterializedVariables) for(auto *var : *unmaterializedVariables)
{ {
var->materialize(); var->materialize();
} }
unmaterializedVariables.clear(); unmaterializedVariables->clear();
} }
void Variable::killUnmaterialized() void Variable::killUnmaterialized()
{ {
unmaterializedVariables.clear(); unmaterializedVariables->clear();
} }
// NOTE: Only 12 bits out of 16 of the |select| value are used. // NOTE: Only 12 bits out of 16 of the |select| value are used.
......
...@@ -133,7 +133,9 @@ private: ...@@ -133,7 +133,9 @@ private:
static void materializeAll(); static void materializeAll();
static void killUnmaterialized(); static void killUnmaterialized();
static thread_local std::unordered_set<Variable *> unmaterializedVariables; // This has to be a raw pointer because glibc 2.17 doesn't support __cxa_thread_atexit_impl
// for destructing objects at exit. See crbug.com/1074222
static thread_local std::unordered_set<Variable *> *unmaterializedVariables;
Type *const type; Type *const type;
mutable Value *rvalue = nullptr; mutable Value *rvalue = nullptr;
......
...@@ -855,7 +855,7 @@ void VPrintf(const std::vector<Value *> &vals) ...@@ -855,7 +855,7 @@ void VPrintf(const std::vector<Value *> &vals)
Nucleus::Nucleus() Nucleus::Nucleus()
{ {
::codegenMutex.lock(); // Reactor is currently not thread safe ::codegenMutex.lock(); // SubzeroReactor is currently not thread safe
Ice::ClFlags &Flags = Ice::ClFlags::Flags; Ice::ClFlags &Flags = Ice::ClFlags::Flags;
Ice::ClFlags::getParsedClFlags(Flags); Ice::ClFlags::getParsedClFlags(Flags);
...@@ -901,10 +901,16 @@ Nucleus::Nucleus() ...@@ -901,10 +901,16 @@ Nucleus::Nucleus()
::context = new Ice::GlobalContext(&cout, &cout, &cerr, elfMemory); ::context = new Ice::GlobalContext(&cout, &cout, &cerr, elfMemory);
::routine = elfMemory; ::routine = elfMemory;
} }
ASSERT(Variable::unmaterializedVariables == nullptr);
Variable::unmaterializedVariables = new std::unordered_set<Variable *>();
} }
Nucleus::~Nucleus() Nucleus::~Nucleus()
{ {
delete Variable::unmaterializedVariables;
Variable::unmaterializedVariables = nullptr;
delete ::routine; delete ::routine;
::routine = nullptr; ::routine = nullptr;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment