[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