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

[Dan Gohman via Phabricator via llvm-commits]

sunfish added a comment.

In https://reviews.llvm.org/D49194#1170428, @jfb wrote:

> In https://reviews.llvm.org/D49194#1170425, @sunfish wrote:
>
> > For the record, I'm opposed to having the WebAssembly backend translate `asm volatile(""::: "memory")` differently from other targets in LLVM or GCC, which to my knowledge always translate it to a no-op, even on platforms with hardware fence instructions.
>
>
> I think it should lower and not error out, and thought should be put into what it should lower to. No-op seems fine to me, but I'd like to hear a rationale.


I'm not opposed to doing something here. I am opposed to doing something in the memory model that is markedly different from what all other targets in LLVM and all other compilers do, without agreeing on it with the maintainers of other backends, which I assume would be in the same boat as WebAssembly here.

In https://reviews.llvm.org/D49194#1171855, @jfb wrote:

> In https://reviews.llvm.org/D49194#1171262, @aheejin wrote:
>
> > @jfb
> >
> > 1. About translating volatiles to atomics, just in case we later happen to add some feature that more closely matches volatile or support more relaxed memory models than seq_cst and change volatile's translation rules, some user programs that worked before may not work anymore. But maybe this is ok because that means people have been relying on an undefined behavior..?
>
>
> Agreed. Further, we can't really translate volatile to regular operations without breaking the C++ memory model, and the only other choice we really have is seq_cst.


Could you say more about this? My non-expert understanding of volatile is that it doesn't do anything unless you're doing memory-mapped I/O or signal handlers, neither of which WebAssembly has right now. I'd be happy to read any background material or standard citations you could provide.

>> @sunfish
>> 
>> 1. You mean, in case they have fence instructions, they translate LLVM IR's `fence`s to their fence instructions but not `asm volatile(""::: "memory")`? What do other platforms do to ensure the memory barrier if they translate `asm volatile(""::: "memory")` to a nop?

My current position is that `asm volatile(""::: "memory")` should translate to zero instructions (specifically, not even a no-op instruction). This is what it currently does, so no change is needed.


Repository:
  rL LLVM

https://reviews.llvm.org/D49194