[PATCH] D49194: [WebAssembly] Add tests for weaker memory consistency orderings

Fri Jul 13 10:05:41 PDT 2018

jfb added a comment.

In https://reviews.llvm.org/D49194#1161116, @aheejin wrote:

> Currently LLVM treats all atomic IR instructions as volatile. Link0 <https://github.com/llvm-mirror/llvm/blob/d79789afc64bdd2ef879c97f97848f436bf0692d/lib/CodeGen/SelectionDAG/SelectionDAG.cpp#L6035-L6039> Link1 <https://github.com/llvm-mirror/llvm/blob/d79789afc64bdd2ef879c97f97848f436bf0692d/lib/CodeGen/SelectionDAG/SelectionDAG.cpp#L6000-L6003> So I'm not exactly sure what you mean by 'upgrade `volatile` to `seq_cst`' is. For non-atomic volatile memory instructions, they are not atomic, so they don't have orderings.

I mean `volatile int i;`. Agreed they don't have ordering, but they also don't really have semantics within WebAssembly. I think the user-friendly thing to do is to translate them to atomic WebAssembly instructions, and break up any larger than max-size volatile accesses to atomic ones (which is legal in the C++ memory model, volatile can tear). Transforming volatile to regular instructions isn't legal in the C++ memory model because volatile means you can only touch each byte once, and that's not true for regular load / store.

> And I haven't implemented translation rules for `fence` or `asm volatile(""::: "memory");` yet. The wasm thread proposal <https://github.com/WebAssembly/threads/blob/master/proposals/threads/Overview.md> does not have a fence instruction itself. Because in the current wasm spec all atomic memory accesses are sequentially consistent, do you think we can translate them to a nop safely? Please let me know if I'm mistaken.

Fences cannot be transformed to nops: they can be used to order non-atomic instructions.

Repository:
  rL LLVM

https://reviews.llvm.org/D49194