[PATCH] D130053: [WebAssembly] Use `localexec` as default TLS model for non-Emscripten targets

Mon Jul 25 01:21:03 PDT 2022

penzn accepted this revision.
penzn added a comment.

Oops, commented on outdated version (also had some issues with UI leaving repeated inline comments, sorry).

Out of curiosity, why disabling bulk memory disables TLS? Couldn't find why that is the case in tools conventions.

================
Comment at: llvm/test/CodeGen/WebAssembly/tls-local-exec.ll:5
+
+; Also, run the same tests without a specified TLS mode--this should still emit `localexec` code.
+; RUN: sed -e 's/\[\[TLS_MODE\]\]//' %s | llc -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -mattr=+bulk-memory,atomics - | FileCheck --check-prefixes=CHECK,TLS %s
----------------
abrown wrote:
> sbc100 wrote:
> > sbc100 wrote:
> > > Maybe add something like "...on non-emscripten targets which don't currently support dynamic linking."
> > > 
> > > Also, should we run this with explicit general dynamic too? 
> > How about explicit general dynamic TLS mode here too (since I think that is what clang generates)?
> Yeah, here's what I get when I inject `(generaldynamic)`:
> 
> 
> ```
> /.../llvm-project/build/bin/llc: error: /.../llvm-project/build/bin/llc: <stdin>:103:30: error: expected localdynamic, initialexec or localexec
> @tls = internal thread_local(generaldynamic) global i32 0
> ```
I don't think it is allowed to be specified directly, it is default-only mode.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130053