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

[Thomas Lively via Phabricator via cfe-commits]

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:
> > I just realized that if we have atomics but not bulk memory and TLS is stripped, then we will be in the awkward situation of both using atomics and disallowing atomics because the module is not thread safe. I think the best solution would be to go back and forcibly strip both atomics and TLS if either of them would be stripped.
> Done.
We can be more fine grained than this. We should strip atomics or TLS only if the relevant feature is missing or the other one is stripped. It is possible to have thread-locals and bulk memory but not have any atomics to strip.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64537