[PATCH] D126107: [lld][WebAssembly] Allow first-loader to use static TLS region

Thomas Lively via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 20 17:36:21 PDT 2022


tlively added inline comments.


================
Comment at: lld/test/wasm/tls.s:109
 # CHECK-NEXT:         Opcode:          I32_CONST
-# CHECK-NEXT:         Value:           0
+# CHECK-NEXT:         Value:           1024
 
----------------
Where does this number come from?


================
Comment at: lld/wasm/Writer.cpp:280
     if (!config->relocatable && seg->isTLS()) {
-      if (config->sharedMemory) {
+      if (WasmSym::tlsSize) {
         auto *tlsSize = cast<DefinedGlobal>(WasmSym::tlsSize);
----------------
Why these changes?


================
Comment at: lld/wasm/Writer.cpp:973-975
-  // TLS segments are initialized separately
-  if (segment->isTLS())
-    return false;
----------------
Why did this change?


================
Comment at: lld/wasm/Writer.cpp:1190-1196
+          // Call __wasm_init_tls, passing in the statically allocated TLS
+          // region.  This means that the first thread to load a given module
+          // does not need to do any dynamic allocation or call
+          // __wasm_init_tls.
+          writeU8(os, WASM_OPCODE_CALL, "CALL");
+          writeUleb128(os, WasmSym::initTLS->getFunctionIndex(),
+                       "__wasm_init_tls function index");
----------------
Is there only ever one TLS segment? It feels weird to have this in a loop.


================
Comment at: lld/wasm/Writer.cpp:1259-1260
       if (needsPassiveInitialization(s) && !s->isBss) {
+        // TODO(sbc): Remove this condition.  Its only here to support
+        // legacy caller of __wasm_init_tls.
+        if (config->sharedMemory && s->isTLS()) {
----------------
Can you elaborate on this comment? I don't understand why the condition should be removed. Surely we need to continue to avoid doing `data.drop` on TLS segments?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126107/new/

https://reviews.llvm.org/D126107



More information about the llvm-commits mailing list