quantum marked an inline comment as done.
quantum added a comment.

In D64537#1579626 <https://reviews.llvm.org/D64537#1579626>, @sunfish wrote:

> This looks nice!
>
> > __wasm_init_tls(calloc(__builtin_wasm_tls_size(), 1));
>
> Would it make sense to change the API contract for `__wasm_init_tls` to 
> guarantee that initializes all bytes (with zeros as needed)? 
> `__wasm_init_tls` knows what bytes it's initializing, so we could use plain 
> `malloc` instead of `calloc` and avoid redundant zero initialization of those 
> bytes.


Updated the summary. I just realized that the way the code is written, the 
plain `malloc` would already work.



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1111
+
+  return DAG.getNode(ISD::ADD, DL, VT, TLSBase, TLSOffset);
+}
----------------
sunfish wrote:
> It looks like this supports local-exec, but would need to be extended to 
> handle initial-exec or the other TLS models. Assuming this is correct, could 
> you add a check for the TLS model and `report_fatal_error` on unsupported 
> models?
I'll add an error for non-local-exec for now.


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@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to