Remove WASM_WORKERS_NO_TLS option (#16476)

* Remove WASM_WORKERS_NO_TLS option, and simplify Wasm Worker TLS creation by hiding it as an implementation detail - TLS block will be grafted onto the memory area set for the thread stack. Also fix MINIMAL_RUNTIME+WASM_WORKERS+WASM=0 build combination and add a test for that.

* Update test doc

* Remove wasm_worker_init_tls()

* Update code size test

* Rebaseline wasm2js tests.

Author: juj

site/source/docs/api_reference/wasm_workers.rst

@@ -33,9 +33,8 @@ which shares the same WebAssembly.Module and WebAssembly.Memory object. Then a
 ``postMessage()`` is passed to the Worker to ask it to execute the function
 ``run_in_worker()`` to print a string.
 
-For more complex threading scenarios, see the functions ``emscripten_create_wasm_worker_with_tls()``
-and ``emscripten_create_wasm_worker_no_tls()`` on how to manage the memory resources that
-the thread will use.
+To explicitly control the memory allocation placement when creating a worker, use the
+function ``emscripten_create_wasm_worker()``.
 
 Introduction
 ============

src/library_wasm_worker.js

@@ -56,11 +56,7 @@ mergeInto(LibraryManager.library, {
 #endif
 
     // Run the C side Worker initialization for stack and TLS.
-    _emscripten_wasm_worker_initialize(m['sb'], m['sz']
-#if !WASM_WORKERS_NO_TLS
-      , m['tb']
-#endif
-    );
+    _emscripten_wasm_worker_initialize(m['sb'], m['sz']);
     // The above function initializes the stack for this Worker, but C code cannot
     // call to extern __set_stack_limits() function, or Binaryen breaks with
     // "Fatal: Module::addFunction: __set_stack_limits already exists".
@@ -87,20 +83,16 @@ mergeInto(LibraryManager.library, {
   _wasmWorkerBlobUrl: "URL.createObjectURL(new Blob(['onmessage=function(d){onmessage=null;d=d.data;{{{ captureModuleArg() }}}{{{ instantiateWasm() }}}importScripts(d.js);{{{ instantiateModule() }}}d.wasm=d.mem=d.js=0;}'],{type:'application/javascript'}))",
 #endif
 
-  _emscripten_create_wasm_worker_with_tls__deps: ['wasm_workers', 'wasm_workers_id', '_wasm_worker_appendToQueue', '_wasm_worker_runPostMessage'
+  _emscripten_create_wasm_worker__deps: ['wasm_workers', 'wasm_workers_id', '_wasm_worker_appendToQueue', '_wasm_worker_runPostMessage'
 #if WASM_WORKERS == 2
     , '_wasmWorkerBlobUrl'
 #endif
   ],
-  _emscripten_create_wasm_worker_with_tls__postset: 'if (ENVIRONMENT_IS_WASM_WORKER) {\n'
+  _emscripten_create_wasm_worker__postset: 'if (ENVIRONMENT_IS_WASM_WORKER) {\n'
     + '_wasm_workers[0] = this;\n'
     + 'addEventListener("message", __wasm_worker_appendToQueue);\n'
     + '}\n',
-  _emscripten_create_wasm_worker_with_tls: function(stackLowestAddress, stackSize, tlsAddress) {
-#if ASSERTIONS
-    assert(stackLowestAddress % 16 == 0);
-    assert(stackSize % 16 == 0);
-#endif
+  _emscripten_create_wasm_worker: function(stackLowestAddress, stackSize) {
     let worker = _wasm_workers[_wasm_workers_id] = new Worker(
 #if WASM_WORKERS == 2 // WASM_WORKERS=2 mode embeds .ww.js file contents into the main .js file as a Blob URL. (convenient, but not CSP security safe, since this is eval-like)
       __wasmWorkerBlobUrl
@@ -122,19 +114,12 @@ mergeInto(LibraryManager.library, {
       'js': Module['mainScriptUrlOrBlob'] || _scriptDir,
       'wasmMemory': wasmMemory,
 #endif
-      'sb': stackLowestAddress, // sb = stack base
+      'sb': stackLowestAddress, // sb = stack bottom (lowest stack address, SP points at this when stack is full)
       'sz': stackSize,          // sz = stack size
-#if !WASM_WORKERS_NO_TLS
-      'tb': tlsAddress,         // tb = TLS base
-#endif
     });
     worker.addEventListener('message', __wasm_worker_runPostMessage);
     return _wasm_workers_id++;
   },
-  _emscripten_create_wasm_worker_no_tls__deps: ['_emscripten_create_wasm_worker_with_tls'],
-  _emscripten_create_wasm_worker_no_tls: function(stackLowestAddress, stackSize) {
-    return __emscripten_create_wasm_worker_with_tls(stackLowestAddress, stackSize, 0, 0);
-  },
 
   emscripten_terminate_wasm_worker: function(id) {
 #if ASSERTIONS

src/settings.js

@@ -1480,12 +1480,6 @@ var USE_PTHREADS = 0;
 // [compile+link] - affects user code at compile and system libraries at link.
 var WASM_WORKERS = 0;
 
-// Wasm Worker option: set to 1 to disable TLS for small code size gain when
-// not using TLS. This setting can be used to verify that there is no leftover
-// state stored in a Worker when manually implementing thread pooling in Workers.
-// [link]
-var WASM_WORKERS_NO_TLS = 0;
-
 // In web browsers, Workers cannot be created while the main browser thread
 // is executing JS/Wasm code, but the main thread must regularly yield back
 // to the browser event loop for Worker initialization to occur.

system/include/emscripten/wasm_worker.h

@@ -17,28 +17,16 @@ extern "C" {
                                   behaves, the dynamically allocated memory can never be freed, so use this function
                                   only in scenarios where the page does not need to deinitialize/tear itself down.
 
-   emscripten_create_wasm_worker_no_tls: Creates a Wasm Worker on the given placed stack address area, but no TLS.
-                                         Use this function on codebase that explicitly do not have any TLS state,
-                                         e.g. when the Worker is being reset to reinit execution at runtime, or to micro-
-                                         optimize code size when TLS is not needed. This function will assert() fail if
-                                         the compiled code does have any TLS uses in it. This function does not use any
-                                         dynamic memory allocation.
-
-   emscripten_create_wasm_worker_with_tls: Creates a Wasm Worker on given placed stack address and TLS area.
-                                           Use the Wasm built-in functions __builtin_wasm_tls_align() and
-                                           __builtin_wasm_tls_size() to obtain the needed memory size for the TLS area
-                                           (though note that __builtin_wasm_tls_align() can return zero when the TLS size
-                                           is zero, so be careful to avoid a divide by zero if/when rounding to this alignment)
-                                           Use this function to manually manage the memory that a Worker should use.
-                                           This function does not use any dynamic memory allocation.
+   emscripten_create_wasm_worker: Creates a Wasm Worker on given placed stack address.
+                                  Use this function to manually manage the memory that a Worker should use.
+                                  This function does not use any dynamic memory allocation.
 
    Returns an ID that represents the given Worker. If not building with Wasm workers enabled (-s WASM_WORKERS=0),
    these functions will return 0 to denote failure.
    Note that the Worker will be loaded up asynchronously, and initially will not be executing any code. Use
    emscripten_wasm_worker_post_function_*() set of functions to start executing code on the Worker. */
 emscripten_wasm_worker_t emscripten_malloc_wasm_worker(uint32_t stackSize);
-emscripten_wasm_worker_t emscripten_create_wasm_worker_no_tls(void *stackLowestAddress, uint32_t stackSize);
-emscripten_wasm_worker_t emscripten_create_wasm_worker_with_tls(void *stackLowestAddress, uint32_t stackSize, void *tlsAddress, uint32_t tlsSize);
+emscripten_wasm_worker_t emscripten_create_wasm_worker(void *stackLowestAddress, uint32_t stackSize);
 
 // Terminates the given Wasm Worker some time after it has finished executing its current, or possibly some subsequent
 // posted functions. Note that this function is not C++ RAII safe, but you must manually coordinate to release any

system/lib/compiler-rt/stack_limits.S

@@ -82,27 +82,44 @@ emscripten_stack_get_free:
 # __stack_base and __stack_end globals from a separate file as externs in order for that to work.
 .globl emscripten_wasm_worker_initialize
 emscripten_wasm_worker_initialize:
-  .functype emscripten_wasm_worker_initialize (PTR, i32, PTR) -> ()
+  .functype emscripten_wasm_worker_initialize (PTR /*stackLowestAddress*/, i32 /*stackSize*/) -> ()
 
+  // __stack_end = stackLowestAddress + (__builtin_wasm_tls_size() + 15) & -16;
   local.get 0
+  .globaltype __tls_size, PTR, immutable
+  global.get __tls_size
+#ifdef __wasm64__
+  i64.extend_i32_u
+#endif
+  PTR.add
+  PTR.const 0xf
+  PTR.add
+  PTR.const -0x10
+  PTR.and
   global.set __stack_end
 
+  // __stack_base = stackLowestAddress + stackSize;
   local.get 0
   local.get 1
 #ifdef __wasm64__
   i64.extend_i32_u
 #endif
   PTR.add
-  local.tee 0
   global.set __stack_base
 
+  // __wasm_init_tls(stackLowestAddress);
   local.get 0
-  global.set __stack_pointer
-
-  local.get 2
   .functype __wasm_init_tls (PTR) -> ()
   call __wasm_init_tls
 
+  // N.b. The function __wasm_init_tls above does not need
+  // __stack_pointer initialized, and it destroys the value it was set to.
+  // So we must initialize __stack_pointer only *after* completing __wasm_init_tls:
+
+  // __stack_pointer = __stack_base;
+  global.get __stack_base
+  global.set __stack_pointer
+
   end_function
 #endif
 

system/lib/wasm_worker/library_wasm_worker.c

@@ -2,68 +2,54 @@
 #include <emscripten/wasm_worker.h>
 #include <emscripten/threading.h>
 #include <emscripten/heap.h>
+#include <emscripten/stack.h>
 #include <malloc.h>
 
 #ifndef __EMSCRIPTEN_WASM_WORKERS__
 #error __EMSCRIPTEN_WASM_WORKERS__ should be defined when building this file!
 #endif
 
 // Options:
-// #define WASM_WORKER_NO_TLS 0/1 : set to 1 to disable TLS compilation support for a small code size gain
 // #define STACK_OVERFLOW_CHECK 0/1/2 : set to the current stack overflow check mode
 
 // Internal implementation function in JavaScript side that emscripten_create_wasm_worker() calls to
 // to perform the wasm worker creation.
-emscripten_wasm_worker_t _emscripten_create_wasm_worker_with_tls(void *stackLowestAddress, uint32_t stackSize, void *tlsAddress);
-emscripten_wasm_worker_t _emscripten_create_wasm_worker_no_tls(void *stackLowestAddress, uint32_t stackSize);
+emscripten_wasm_worker_t _emscripten_create_wasm_worker(void *stackLowestAddress, uint32_t stackSize);
 
 void __wasm_init_tls(void *memory);
 
-#if !WASM_WORKER_NO_TLS
 __attribute__((constructor(48)))
 static void emscripten_wasm_worker_main_thread_initialize() {
 	uintptr_t* sbrk_ptr = emscripten_get_sbrk_ptr();
 	__wasm_init_tls((void*)*sbrk_ptr);
 	*sbrk_ptr += __builtin_wasm_tls_size();
 }
-#endif
 
-emscripten_wasm_worker_t emscripten_create_wasm_worker_with_tls(void *stackLowestAddress, uint32_t stackSize, void *tlsAddress, uint32_t tlsSize)
+emscripten_wasm_worker_t emscripten_create_wasm_worker(void *stackLowestAddress, uint32_t stackSize)
 {
-#if WASM_WORKER_NO_TLS
-	return emscripten_create_wasm_worker_no_tls(stackLowestAddress, stackSize);
-#else
+	assert(stackLowestAddress != 0);
 	assert((uintptr_t)stackLowestAddress % 16 == 0);
+	assert(stackSize > 0);
 	assert(stackSize % 16 == 0);
-	assert(__builtin_wasm_tls_align() != 0 || __builtin_wasm_tls_size() == 0); // Internal consistency check: Clang can report __builtin_wasm_tls_align to be zero if there is no TLS used - double check it never reports zero if it is used.
-	assert(__builtin_wasm_tls_align() == 0 || (uintptr_t)tlsAddress % __builtin_wasm_tls_align() == 0 && "TLS memory address not aligned in a call to emscripten_create_wasm_worker_with_tls()! Please allocate memory with alignment from __builtin_wasm_tls_align() when creating a Wasm Worker!");
-	assert(tlsSize != 0 || __builtin_wasm_tls_size() == 0 && "Program code contains TLS: please use function emscripten_create_wasm_worker_with_tls() to create a Wasm Worker!");
-	assert(tlsSize == __builtin_wasm_tls_size() && "TLS size mismatch! Please reserve exactly __builtin_wasm_tls_size() TLS memory in a call to emscripten_create_wasm_worker_with_tls()");
-	assert(tlsAddress != 0 || tlsSize == 0);
-	return _emscripten_create_wasm_worker_with_tls((void*)stackLowestAddress, stackSize, tlsAddress);
-#endif
-}
 
-emscripten_wasm_worker_t emscripten_create_wasm_worker_no_tls(void *stackLowestAddress, uint32_t stackSize)
-{
-#if !WASM_WORKER_NO_TLS
-	return emscripten_create_wasm_worker_with_tls(stackLowestAddress, stackSize, 0, 0);
-#else
-	assert((uintptr_t)stackLowestAddress % 16 == 0);
-	assert(stackSize % 16 == 0);
-	assert(__builtin_wasm_tls_size() == 0 && "Cannot disable TLS with -sWASM_WORKERS_NO_TLS=1 when compiling code that does require TLS! Rebuild with -sWASM_WORKERS_NO_TLS=1 removed, or remove uses of TLS from the codebase.");
-	return _emscripten_create_wasm_worker_no_tls((void*)stackLowestAddress, stackSize);
-#endif
+	// Guard against a programming oopsie: The target Worker's stack cannot be part of the calling
+	// thread's stack.
+	assert(emscripten_stack_get_base() <= (uintptr_t)stackLowestAddress || emscripten_stack_get_end() >= (uintptr_t)stackLowestAddress + stackSize
+		&& "When creating a Wasm Worker, its stack should be located either in global data or on the heap, not on the calling thread's own stack!");
+
+	// The Worker's TLS area will be spliced off from the stack region.
+    // We expect TLS area to need to be at most 16 bytes aligned
+	assert(__builtin_wasm_tls_align() == 0 || 16 % __builtin_wasm_tls_align() == 0);
+
+	uint32_t tlsSize = (__builtin_wasm_tls_size() + 15) & -16;
+	assert(stackSize > tlsSize);
+
+	return _emscripten_create_wasm_worker(stackLowestAddress, stackSize);
 }
 
 emscripten_wasm_worker_t emscripten_malloc_wasm_worker(uint32_t stackSize)
 {
-#if WASM_WORKER_NO_TLS
-	return emscripten_create_wasm_worker_no_tls(memalign(16, stackSize), stackSize);
-#else
-	uint32_t tlsSize = __builtin_wasm_tls_size();
-	return emscripten_create_wasm_worker_with_tls(memalign(16, stackSize), stackSize, tlsSize ? memalign(__builtin_wasm_tls_align(), tlsSize) : 0, tlsSize);
-#endif
+	return emscripten_create_wasm_worker(memalign(16, stackSize), stackSize);
 }
 
 void emscripten_wasm_worker_sleep(int64_t nsecs)

system/lib/wasm_worker/library_wasm_worker_stub.c

@@ -8,11 +8,7 @@
 #error __EMSCRIPTEN_WASM_WORKERS__ should not be defined when building this file!
 #endif
 
-emscripten_wasm_worker_t emscripten_create_wasm_worker_with_tls(void *stackLowestAddress, uint32_t stackSize, void *tlsAddress, uint32_t tlsSize) {
-  return 0;
-}
-
-emscripten_wasm_worker_t emscripten_create_wasm_worker_no_tls(void *stackLowestAddress, uint32_t stackSize) {
+emscripten_wasm_worker_t emscripten_create_wasm_worker(void *stackLowestAddress, uint32_t stackSize) {
   return 0;
 }
 

tests/code_size/hello_wasm_worker_wasm.js

@@ -30,16 +30,15 @@ var p, q;
 
 WebAssembly.instantiate(b.wasm, {
     a: {
-        b: function(a, d, t) {
+        b: function(a, d) {
             let r = h[k] = new Worker(b.$wb);
             r.postMessage({
                 $ww: k,
                 wasm: b.wasm,
                 js: b.js,
                 mem: e,
                 sb: a,
-                sz: d,
-                tb: t
+                sz: d
             });
             r.addEventListener("message", n);
             return k++;
@@ -63,7 +62,7 @@ WebAssembly.instantiate(b.wasm, {
     p = a.g;
     q = a.i;
     f = a.h;
-    c ? (a = b, q(a.sb, a.sz, a.tb), removeEventListener("message", l), m = m.forEach(n), 
+    c ? (a = b, q(a.sb, a.sz), removeEventListener("message", l), m = m.forEach(n), 
     addEventListener("message", n)) : a.f();
     c || p();
 }));

tests/code_size/hello_wasm_worker_wasm.json

@@ -1,10 +1,10 @@
 {
   "a.html": 737,
   "a.html.gz": 433,
-  "a.js": 765,
-  "a.js.gz": 475,
-  "a.wasm": 1792,
-  "a.wasm.gz": 989,
-  "total": 3294,
-  "total_gz": 1897
+  "a.js": 753,
+  "a.js.gz": 468,
+  "a.wasm": 1800,
+  "a.wasm.gz": 994,
+  "total": 3290,
+  "total_gz": 1895
 }

tests/code_size/hello_webgl2_wasm2js.json

@@ -1,10 +1,10 @@
 {
-  "a.html": 594,
-  "a.html.gz": 389,
+  "a.html": 567,
+  "a.html.gz": 379,
   "a.js": 20046,
   "a.js.gz": 8213,
   "a.mem": 3171,
   "a.mem.gz": 2714,
-  "total": 23811,
-  "total_gz": 11316
+  "total": 23784,
+  "total_gz": 11306
 }