Skip to content

Move pthread TLS state native code using wasm globals #12839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1577,9 +1577,10 @@ def filter_out_duplicate_dynamic_libs(inputs):
# manually export them

shared.Settings.EXPORTED_FUNCTIONS += [
'__emscripten_thread_init',
'_emscripten_get_global_libc',
'___pthread_tsd_run_dtors',
'registerPthreadPtr', '_pthread_self',
'_pthread_self',
'___emscripten_pthread_data_constructor',
'_emscripten_futex_wake',
'_emscripten_stack_set_limits',
Expand Down
2 changes: 2 additions & 0 deletions src/deps_info.json
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@
"emscripten_webgl_get_shader_source_utf8": ["malloc", "free"],
"emscripten_webgl_get_parameter_utf8": ["malloc", "free"],
"emscripten_webgl_create_context": ["malloc", "free"],
"emscripten_start_fetch": ["emscripten_is_main_browser_thread"],
"emscripten_fetch": ["emscripten_is_main_browser_thread"],
"glMapBufferRange": ["malloc", "free"],
"glGetString": ["malloc", "free"],
"glGetStringi": ["malloc", "free"],
Expand Down
52 changes: 6 additions & 46 deletions src/library_pthread.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

var LibraryPThread = {
$PThread__postset: 'if (!ENVIRONMENT_IS_PTHREAD) PThread.initMainThreadBlock();',
$PThread__deps: ['$registerPthreadPtr',
$PThread__deps: ['_emscripten_thread_init',
'$ERRNO_CODES', 'emscripten_futex_wake', '$killThread',
'$cancelThread', '$cleanupThread',
#if USE_ASAN || USE_LSAN
Expand Down Expand Up @@ -81,10 +81,10 @@ var LibraryPThread = {
});
#endif

// Pass the thread address inside the asm.js scope to store it for fast
// access that avoids the need for a FFI out. Global constructors trying
// Pass the thread address to the native code where they stored in wasm
// globals which act as a form of TLS. Global constructors trying
// to access this value will read the wrong value, but that is UB anyway.
registerPthreadPtr(PThread.mainThreadBlock, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
__emscripten_thread_init(PThread.mainThreadBlock, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
_emscripten_register_main_browser_thread_id(PThread.mainThreadBlock);
},
initWorker: function() {
Expand Down Expand Up @@ -218,7 +218,7 @@ var LibraryPThread = {
PThread.runExitHandlers();

_emscripten_futex_wake(tb + {{{ C_STRUCTS.pthread.threadStatus }}}, {{{ cDefine('INT_MAX') }}});
registerPthreadPtr(0, 0, 0); // Unregister the thread block also inside the asm.js scope.
__emscripten_thread_init(0, 0, 0); // Unregister the thread block also inside the asm.js scope.
threadInfoStruct = 0;
if (ENVIRONMENT_IS_PTHREAD) {
// Note: in theory we would like to return any offscreen canvases back to the main thread,
Expand All @@ -234,7 +234,7 @@ var LibraryPThread = {
Atomics.store(HEAPU32, (threadInfoStruct + {{{ C_STRUCTS.pthread.threadStatus }}} ) >> 2, 1); // Mark the thread as no longer running.
_emscripten_futex_wake(threadInfoStruct + {{{ C_STRUCTS.pthread.threadStatus }}}, {{{ cDefine('INT_MAX') }}}); // wake all threads
threadInfoStruct = selfThreadId = 0; // Not hosting a pthread anymore in this worker, reset the info structures to null.
registerPthreadPtr(0, 0, 0); // Unregister the thread block also inside the asm.js scope.
__emscripten_thread_init(0, 0, 0); // Unregister the thread block also inside the asm.js scope.
postMessage({ 'cmd': 'cancelDone' });
},

Expand Down Expand Up @@ -1005,46 +1005,6 @@ var LibraryPThread = {
throw 'unwind';
},

_pthread_ptr: 0,
_pthread_is_main_runtime_thread: 0,
_pthread_is_main_browser_thread: 0,

$registerPthreadPtr__deps: ['_pthread_ptr', '_pthread_is_main_runtime_thread', '_pthread_is_main_browser_thread'],
$registerPthreadPtr__asm: true,
$registerPthreadPtr__sig: 'viii',
$registerPthreadPtr: function(pthreadPtr, isMainBrowserThread, isMainRuntimeThread) {
pthreadPtr = pthreadPtr|0;
isMainBrowserThread = isMainBrowserThread|0;
isMainRuntimeThread = isMainRuntimeThread|0;
__pthread_ptr = pthreadPtr;
__pthread_is_main_browser_thread = isMainBrowserThread;
__pthread_is_main_runtime_thread = isMainRuntimeThread;
},

// Public pthread_self() function which returns a unique ID for the thread.
pthread_self__deps: ['_pthread_ptr'],
pthread_self__asm: true,
pthread_self__sig: 'i',
pthread_self: function() {
return __pthread_ptr|0;
},

emscripten_is_main_runtime_thread__asm: true,
emscripten_is_main_runtime_thread__sig: 'i',
emscripten_is_main_runtime_thread__deps: ['_pthread_is_main_runtime_thread'],
emscripten_is_main_runtime_thread: function() {
// Semantically the same as testing "!ENVIRONMENT_IS_PTHREAD" outside the asm.js scope
return __pthread_is_main_runtime_thread|0;
},

emscripten_is_main_browser_thread__asm: true,
emscripten_is_main_browser_thread__sig: 'i',
emscripten_is_main_browser_thread__deps: ['_pthread_is_main_browser_thread'],
emscripten_is_main_browser_thread: function() {
// Semantically the same as testing "!ENVIRONMENT_IS_WORKER" outside the asm.js scope
return __pthread_is_main_browser_thread|0;
},

pthread_getschedparam: function(thread, policy, schedparam) {
if (!policy && !schedparam) return ERRNO_CODES.EINVAL;

Expand Down
4 changes: 2 additions & 2 deletions src/postamble_minimal.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function initRuntime(asm) {
Module['_emscripten_tls_init'] = _emscripten_tls_init;
Module['HEAPU32'] = HEAPU32;
Module['dynCall'] = dynCall;
Module['registerPthreadPtr'] = registerPthreadPtr;
Module['__emscripten_thread_init'] = __emscripten_thread_init;
Module['_pthread_self'] = _pthread_self;

if (ENVIRONMENT_IS_PTHREAD) {
Expand All @@ -72,7 +72,7 @@ function initRuntime(asm) {

// Pass the thread address inside the asm.js scope to store it for fast access
// that avoids the need for a FFI out.
registerPthreadPtr(PThread.mainThreadBlock, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
__emscripten_thread_init(PThread.mainThreadBlock, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
_emscripten_register_main_browser_thread_id(PThread.mainThreadBlock);
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ this.onmessage = function(e) {
threadInfoStruct = e.data.threadInfoStruct;

// Pass the thread address inside the asm.js scope to store it for fast access that avoids the need for a FFI out.
Module['registerPthreadPtr'](threadInfoStruct, /*isMainBrowserThread=*/0, /*isMainRuntimeThread=*/0);
Module['__emscripten_thread_init'](threadInfoStruct, /*isMainBrowserThread=*/0, /*isMainRuntimeThread=*/0);

selfThreadId = e.data.selfThreadId;
parentThreadId = e.data.parentThreadId;
Expand Down
39 changes: 39 additions & 0 deletions system/lib/pthread/pthread_self.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
.globaltype thread_id, i32
thread_id:

.globaltype is_main_thread, i32
is_main_thread:

.globaltype is_runtime_thread, i32
is_runtime_thread:

.globl pthread_self
pthread_self:
Copy link
Collaborator

@juj juj Nov 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, so any function that is defined in .s file gets a _ prefixed to it? So pthread_self here becomes _pthread_self?

And any functions defined in C/C++ files do not get a _ prefix?

That is actually pretty backwards: in native compilers all assembly code is unprefixed, and all C/C++ code is prefixed with _. Is there a specific reason why this was flipped for Wasm backend? That seems a bit arbitrary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _ prefix is added to all native symbols by the emscripten JS code. We do this purely for legacy/asm.js compatability.

The actual compiler, llvm, does not emit the _ mangling for any symbols. I think maybe the _ mangling is some kind of mac or windows convention. It certainly doesn't exist on windows.

From the linker's POV there is no way to way to the difference between a C symbol and an assembler symbols. They are all just "native" sybmols and we mangle all of them when we import them into JS.

One day we should completely remove the mangling...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an issue to remove this mangling completely one day: #12844

.functype pthread_self () -> (i32)
global.get thread_id
end_function

.globl _emscripten_thread_init
_emscripten_thread_init:
.functype _emscripten_thread_init (i32, i32, i32) -> ()
local.get 0
global.set thread_id
local.get 1
global.set is_main_thread
local.get 2
global.set is_runtime_thread
end_function

# Semantically the same as testing "!ENVIRONMENT_IS_PTHREAD" in JS
.globl emscripten_is_main_runtime_thread
emscripten_is_main_runtime_thread:
.functype emscripten_is_main_runtime_thread () -> (i32)
global.get is_runtime_thread
end_function

# Semantically the same as testing "!ENVIRONMENT_IS_WORKER" in JS
.globl emscripten_is_main_browser_thread
emscripten_is_main_browser_thread:
.functype emscripten_is_main_browser_thread () -> (i32)
global.get is_main_thread
end_function
1 change: 1 addition & 0 deletions tools/system_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,7 @@ def get_files(self):
filenames=[
'library_pthread.c',
'emscripten_tls_init.c',
'pthread_self.s',
])
else:
files += [shared.path_from_root('system', 'lib', 'pthread', 'library_pthread_stub.c')]
Expand Down