[PATCH] D49194: [WebAssembly] Add tests for weaker memory consistency orderings
JF Bastien via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 23 09:36:05 PDT 2018
jfb added a comment.
In https://reviews.llvm.org/D49194#1171262, @aheejin wrote:
> Thank you for the comments and sorry for the delayed reply!
>
> @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.
> 2. We currently don't have fences. Do you think using a (sequentially consistent) atomic load from an address 0 as a fence would be ok in this case? Or do you have any suggestions?
I think you want an idempotent RMW operation, such as seq_cst OR with 0, at an arbitrary (valid) address. The top of stack might be better because it's likely in cache (and I assume always valid?).
> And do you think we should translate `asm volatile(""::: "memory")` to whatever we translate a fence to as well?
I just think we should translate it to something because it's a fairly common idiom (along with atomic_signal_fence). Both of these should probably just be noops (and if we ever do signals we'll strengthen the signal fence).
> @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?
Repository:
rL LLVM
https://reviews.llvm.org/D49194
More information about the llvm-commits
mailing list