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

[JF Bastien via Phabricator via llvm-commits]

jfb added a comment.

> 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.

I have a talk for you! https://youtu.be/IB57wIf9W1k?t=52m20s
I talk about volatile in a few places, but 52m20s is the main one. Basically I'm advocating for something similar to the weird MSVC x86 volatile behavior because some users write code like a loop which updates a volatile variable which another thread reads, and expect it to stay in the loop (not hoisted out). Lowering to regular load / store breaks the guarantee that volatiles effects touch the variable's bytes once per source event. Such user code should really use atomic, but we don't need to be pedants about it.

I can make a more elaborate argument for it, but it's really about not being assholes to users who just want their updates to be visible. I have a bunch of reference in the talk's GitHub page, some about volatile.

If / when we add support for shared memory, setjmp / longjmp, signals, then having volatile translate to a sequence of always-lock-free load / store (with extra volatile semantics to preserve correctness) won't be a big deal. We can totally break up the wider atomics, and we don't have a choice anyways, so that's fine. RMW volatile operations don't really make sense, but we can do the same thing (seq_cst, and break up larger things).

>>> @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.

Agreed. I meant actual noop, not our other not-actual noop :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D49194