quantum marked 5 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") + ---------------- quantum wrote: > aheejin wrote: > > Why is it `c`(const)? According to [[ > > https://github.com/llvm/llvm-project/blob/e6695821e592f95bffe1340b28be7bcfcce04284/clang/include/clang/Basic/Builtins.h#L104-L108 > > | this comment ]], this is true if this function has no side effects and > > doesn't read memory, i.e., the result should be only dependent on its > > arguments. Can't wasm globals be memory locations in machines? > I was thinking that since the global is immutable, so the value is always a > constant. According to @tlively, there is no solid definition on whether a global (especially a constant one), counts as memory access. For now, I am going to change this to not constant. We can always change it back later. ================ Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:134 + [], + [IntrNoMem, IntrSpeculatable]>; + ---------------- quantum wrote: > aheejin wrote: > > - Why is it speculatable? > > - I'm not sure if it's `InstNoMem`, because wasm globals can be memory > > locations after all. What do other people think? > @tlively suggested that I do this. It should be speculatable because it has > no side effects (since it returns a constant). As for `InstrNoMem`, I am not > sure if globals are memory, and whether reading a constant counts as memory > access. I think I am going to remove these attributes for now. We can add them back if we end up deciding that immutable globals are not memory accesses. 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
