[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