[PATCH] D53184: [LangRef] Clarify semantics of volatile operations.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: llvm/trunk/docs/LangRef.rst:2196
+may not modify any other memory accessible by the module being compiled.
+A volatile operation may not call any code in the current module.
+
----------------
jfb wrote:
> hfinkel wrote:
> > jfb wrote:
> > > Sorry for missing the initial discussion. This disallows using `mmap` to map the same physical memory into two different virtual addresses in the same process. In C++ `volatile` loads and stores exist to denote "external modifications", where "external" is w.r.t. the abstract machine. It seems like a misstep to say that "A volatile operation may not modify any other memory accessible by the module being compiled."
> > There are certainly some things that you can arrange using mmap, signal handlers, and other system calls that fall outside of the model that we support. This is one of them. To put it another way, if you do this, then you'd need to access all copies with volatile accesses. Accessing some copies as volatile, and some copies not, isn't something I can see how we can support (otherwise, we'd need to assume that all memory was modified by all volatile accesses to any address, and the cost of that is certainly not worth the benefits).
> > 
> > Please feel free to suggest language to make all of this clearer.
> Agreed that both sides, reader and writer, need to use `volatile`. It seems like the current wording doesn't allow for this. From your response it sounds like it was an oversight? If so I can indeed send a follow-up.
>From my perspective, if you use mmap to map some backing store to multiple places in the address space, then that's not "memory" as is intended by the statement (it's something else, like MMIO registers) - it's something you essentially cannot access using regular memory accesses. We might certainly be able to make the text clearer, or more carefully classify things in this regard.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53184/new/

https://reviews.llvm.org/D53184