[PATCH] D64776: [WebAssembly] Compile all TLS on Emscripten as local-exec

[Guanzhong Chen via Phabricator via llvm-commits]

quantum marked 4 inline comments as done.
quantum added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp:191
       report_fatal_error("only -ftls-model=local-exec is supported for now",
                          false);
     }
----------------
aheejin wrote:
> So does this mean:
> 1. If this is emscripten, convert all non-local-exec to local-exec
> 2. If this is not emscripten, error on non-local exec
> 
> Is this right?
> 
> Anyhow, it might be better to tell what global variable is causing the problem to users in the error message, something like
> ```
> report_fatal_error("only -ftls-model=local-exec is supported for now in non-Emscripten OSes: " + GA->getGlobal()->getName());
> ```
> 
Yes, it's right. Good point regarding the message, I'll fix it.


================
Comment at: llvm/test/CodeGen/WebAssembly/tls-general-dynamic.ll:1
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -mattr=+atomics,+bulk-memory | FileCheck %s --check-prefixes=CHECK,TLS
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -mattr=+atomics,+bulk-memory -fast-isel | FileCheck %s --check-prefixes=CHECK,TLS
+; RUN: not llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -mattr=+atomics,+bulk-memory 2>&1 | FileCheck %s --check-prefix=ERROR
+; RUN: not llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -mattr=+atomics,+bulk-memory -fast-isel 2>&1 | FileCheck %s --check-prefix=ERROR
----------------
aheejin wrote:
> Do we need a separate copy of `tls.ll` for this test? The file contents look the same. Can't we add a new test line or something to the existing file?
No, the difference is that in this file, we must say

    @tls = internal thread_local global i32 0

whereas in the other file, it has to say

    @tls = internal thread_local(localexec) global i32 0

I thought about using `grep`, but @tlively suggested otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64776