[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 added a comment. In D64537#1586614 , @quantum wrote: > In D64537#1586577 , @dschuff wrote: > > > Another high-level question (based just on reading the CL description): The > > TLS-size intrinsic is per-func

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum added a comment. In D64537#1588699 , @dschuff wrote: > I had a reply that got eaten here, so I'm going to keep trolling you on your > CL since we don't have a design doc for this. > The `offset` field of a data segment initializer can be a `globa

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

dschuff added a comment. I had a reply that got eaten here, so I'm going to keep trolling you on your CL since we don't have a design doc for this. The `offset` field of a data segment initializer can be a `global.get` on an imported global. (https://webassembly.github.io/spec/core/valid/instr

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 210188. quantum added a comment. Disable atomics when TLS is stripped Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/include/clang/Basic/BuiltinsWebAsse

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

tlively accepted this revision. tlively added a comment. Nice work! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

This revision was automatically updated to reflect the committed changes. Closed by commit rL366272: [WebAssembly] Implement thread-local storage (local-exec model) (authored by quantum, committed by ). Changed prior to commit: https://reviews.llvm.org/D64537?vs=210188&id=210191#toc Repository

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 210187. quantum added a comment. Remove extra braces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/include/clang/Basic/BuiltinsWebAssembly.def clang/

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 210184. quantum added a comment. Removed the trailing `.`. I would add a test to make sure things work without `-fdata-sections`, but `this->Options.DataSections = true;` is hard-coded in `WebAssemblyTargetMachine.cpp`. Repository: rG LLVM Github Monorep

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 accepted this revision. sbc100 added inline comments. This revision is now accepted and ready to land. Comment at: lld/wasm/Writer.cpp:400 + StringRef name = segment->getName(); + if (name.startswith(".tdata.") || name.startswith(".tbss.")) +tlsUsed = tru

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 210166. quantum added a comment. More fine-grainted stripping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/include/clang/Basic/BuiltinsWebAssembly.de

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

tlively added a comment. LGTM apart from one last comment Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:193 + +if (!Features[WebAssembly::FeatureBulkMemory]) Stripped |= stripThreadLocals(M); quantum wrote: > tlively wrote: > >

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum added a comment. In D64537#1586809 , @aheejin wrote: > Where should we call `__wasm_init_tls`, in case within a library? Dynamic libraries are not supported by the local-exec TLS model. Support will be implemented later. The idea is that we'll h

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

aheejin added a comment. Where should we call `__wasm_init_tls`, in case within a library? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 ___ cfe-commits maili

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum added a comment. In D64537#1586577 , @dschuff wrote: > Another high-level question (based just on reading the CL description): The > TLS-size intrinsic is per-function, does that mean that the tls-init function > is called for every function? are

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

dschuff added a comment. Another high-level question (based just on reading the CL description): The TLS-size intrinsic is per-function, does that mean that the tls-init function is called for every function? are there just multiple TLS sections per object file? Repository: rG LLVM Github M

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

dschuff added a comment. The `offset` field of a segment can be a constant expression which can be a `global.get` of an imported global. So we could have an imported global `__tls_base` which is different for

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209966. quantum added a comment. Fix duplicate end function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/include/clang/Basic/BuiltinsWebAssembly.def

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum added inline comments. Comment at: lld/test/wasm/data-segments.ll:7 ; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.o -o %t.atomics.wasm -; RUN: obj2yaml %t.atomics.wasm | FileCheck %s --check-prefix ACTIVE +; RUN: obj2yaml %t.at

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209953. quantum marked 16 inline comments as done. quantum added a comment. Deal with review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/in

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

tlively added inline comments. Comment at: lld/wasm/Driver.cpp:543 + "__wasm_init_tls", WASM_SYMBOL_VISIBILITY_HIDDEN, + make(I32ArgSignature, "__wasm_init_tls")); + quantum wrote: > aheejin wrote: > > Does this TLS thing work when `Config->Shared == tr

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

aheejin added inline comments. Comment at: lld/test/wasm/data-segments.ll:7 ; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.o -o %t.atomics.wasm -; RUN: obj2yaml %t.atomics.wasm | FileCheck %s --check-prefix ACTIVE +; RUN: obj2yaml %t.at

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum added inline comments. Comment at: lld/test/wasm/data-segments.ll:7 ; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.o -o %t.atomics.wasm -; RUN: obj2yaml %t.atomics.wasm | FileCheck %s --check-prefix ACTIVE +; RUN: obj2yaml %t.at

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209658. quantum marked 14 inline comments as done. quantum added a comment. Change per review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/i

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

aheejin added inline comments. Comment at: lld/test/wasm/data-segments.ll:7 ; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.o -o %t.atomics.wasm -; RUN: obj2yaml %t.atomics.wasm | FileCheck %s --check-prefix ACTIVE +; RUN: obj2yaml %t.at

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209656. quantum added a comment. Use signed LEB128 for `i32.const`. Basically, do what D64612 did. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llv

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum added a comment. In D64537#1584069 , @dschuff wrote: > Oh, btw, any reason these have to be passive segments? Why can't we just make > them active segments and let the VM initialize them for us? These need to be passive segments so that we can i

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

dschuff added a comment. Oh, btw, any reason these have to be passive segments? Why can't we just make them active segments and let the VM initialize them for us? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D6

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

dschuff added a comment. If there's any chance this TLS ABI could be useful for WASI (I don't know if there's been any WASI work on threads yet, but it seems like there's no reason it couldn't be), then we should start a doc in tool-conventions for it. If not then we should get it behind the em

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum added a comment. @sbc100 As for `__tls_base` being immutable, I am not sure how this will interact with dynamic linking once we implement it. On native platforms, `dlopen`ed libraries have their TLS blocks allocated on first use. What exactly happens to `dlopen`ed libraries when there a

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209566. quantum added a comment. Don't show a crash report for incorrect flags to use TLS Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/include/clang/B

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209560. quantum marked an inline comment as done. quantum added a comment. Switch to using ISelDAGToDAG instead of ISelLowering Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum marked 4 inline comments as done. quantum added inline comments. Comment at: lld/wasm/Writer.cpp:629 + // Merge .tbss into .tdata so that they share the same offsets. + if (name.startswith(".tbss.")) +return ".tdata"; sbc100 wrote: > Maybe write thi

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209535. quantum added a comment. Guard `__wasm_init_tls` and TLS globals behind a flag (currently `--shared-memory`). @tlively and I discussed offline and agreed that it's probably best to use the existing flag instead of adding another flag that needs to b

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 added a comment. Its really great to see this change BTW! Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 ___ cfe-commits mailing list cfe-commi

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

sbc100 added a comment. I wonder if `__tls_base` should be allocated by the embedder (or by the parent/creator thread). Then it could be an *immutable* global import that is allocated up front. I guess `__stack_pointer` should be treated in the same way (except mutable). I don't want to bl

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum marked 28 inline comments as done. quantum added inline comments. Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:33 +// Thread-local storage +TARGET_BUILTIN(__builtin_wasm_tls_size, "z", "nc", "bulk-memory") + tlively wrote: > quantum wrote

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209391. quantum marked 10 inline comments as done. quantum added a comment. Apply review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/include

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

quantum updated this revision to Diff 209376. quantum added a comment. Rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64537/new/ https://reviews.llvm.org/D64537 Files: clang/include/clang/Basic/BuiltinsWebAssembly.def clang/lib/CodeGen/

[PATCH] D64537: [WebAssembly] Implement thread-local storage (local-exec model)

tlively added inline comments. Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:33 +// Thread-local storage +TARGET_BUILTIN(__builtin_wasm_tls_size, "z", "nc", "bulk-memory") + quantum wrote: > quantum wrote: > > aheejin wrote: > > > Why is it `c`(co