[LangRef] Clarify semantics of volatile operations.

[efriedma]

Summary

Specifically, clarify the following:

1. Volatile load and store may access addresses that are not memory (MMIO registers, etc.).
2. Volatile load and store do not modify unrelated memory.
3. Volatile load and store do not trap.

Prompted by recent discussion of volatile on llvmdev. I think this is enough to resolve all the issues brought up in that discussion, at least from the perspective of LangRef.

Currently, there's sort of a split in the LLVM source code about whether volatile operations are allowed to trap; this resolves that dispute in favor of not allowing them to trap. (This is the opposite of the position I originally took, but thinking about it a bit more it makes sense for the uses we expect.)

Hal brought up the possibility on llvmdev that certain APIs like isSafeToLoadUnconditionally and FindAvailablePtrLoadStore might assume that volatile operations do in fact point to memory; I think we should just treat that as a bug, though.

[hfinkel]

Currently, there's sort of a split in the LLVM source code about whether volatile operations are allowed to trap; this resolves that dispute in favor of not allowing them to trap. (This is the opposite of the position I originally took, but thinking about it a bit more it makes sense for the uses we expect.)

I agree. They shouldn't trap.

Hal brought up the possibility on llvmdev that certain APIs like isSafeToLoadUnconditionally and FindAvailablePtrLoadStore might assume that volatile operations do in fact point to memory; I think we should just treat that as a bug, though.

I agree. We should not use the presence of a volatile operation to justify adding other non-volatile accesses to the same address (and, by definition, we can't add other volatile accesses).

docs/LangRef.rst

2188

How should we specify the interaction between this and inaccessiblememonly?

[cramertj]

Hal brought up the possibility on llvmdev that certain APIs like isSafeToLoadUnconditionally and FindAvailablePtrLoadStore might assume that volatile operations do in fact point to memory; I think we should just treat that as a bug, though.

I agree. We should not use the presence of a volatile operation to justify adding other non-volatile accesses to the same address (and, by definition, we can't add other volatile accesses).

Can we then also expand this part of the reference to clarify that optimizers must not change the number of volatile *or* nonvolatile operations to memory which is only accessed via volatile operations (with a note one way or the other WRT whether this includes loads/stores from pointers marked "dereferenceable")?

Certain memory accesses, such as load’s, store’s, and llvm.memcpy’s may be marked volatile. The optimizers must not change the number of volatile operations or change their order of execution relative to other volatile operations. The optimizers may change the order of volatile operations relative to non-volatile operations. This is not Java’s “volatile” and has no cross-thread synchronization behavior.

[kparzysz]

What thread was that discussion in? Accesses to device-mapped memory can trap, what is the reason for the compiler to assume that they can't?

[efriedma]

llvmdev thread is titled "Volatile and Inserted Loads/Stores on MMIO​".

Accesses to device-mapped memory can trap, what is the reason for the compiler to assume that they can't?

If we don't restrict the effects of a volatile load/store, we have to assume it could, at worst, modify any memory that isn't local to the function and unwind the stack, like a call to an unknown function. That would block optimizations around volatile accesses. That's not what LLVM implements, and it's not what other compilers implement.

Can we then also expand this part of the reference to clarify that optimizers must not change the number of volatile *or* nonvolatile operations to memory which is only accessed via volatile operations

This is implied by "a volatile load or store may access addresses which do not point to memory", but sure, I can expand that to state it explicitly.

[efriedma]

docs/LangRef.rst

2188

This is essentially supposed to be inaccessiblememonly (plus the pointed-to address, if it points into memory).

I'll take another shot at the wording.

[efriedma]

Took another shot at the wording. This should be a bit more explicit about what we're assuming, and what transforms are allowed.

[efriedma]

Ping

[efriedma]

Ping

[mkazantsev]

LGTM

[hfinkel]

In D53184#1296581, @mkazantsev wrote:

LGTM

I still think it would be better to explicitly say something about the access properties here relative to inaccessiblememonly functions. Also, are we saying that volatile loads can write to this inaccessible memory?

[efriedma]

I can add a sentence to specifically state that this can modify state accessed by functions marked with the inaccessiblememonly attribute, and vice versa, if that's the interaction you're worried about. (I don't think it's practical to have multiple forms of inaccessible state.)

[hfinkel]

In D53184#1333556, @efriedma wrote:

I can add a sentence to specifically state that this can modify state accessed by functions marked with the inaccessiblememonly attribute, and vice versa, if that's the interaction you're worried about. (I don't think it's practical to have multiple forms of inaccessible state.)

Thanks. Regarding this:

Also, are we saying that volatile loads can write to this inaccessible memory?

can you please clarify your thoughts?

[efriedma]

Yes, volatile loads and stores access the "same" state as inaccessiblememonly functions, I think.

In general, we have to make sure volatile operations don't get reordered. This is equivalent to saying that volatile operations read/modify some inaccessible state (something like a list of volatile accesses in the program). This essentially the same as the language for inaccessiblememonly.

We could say that there are two distinct regions of inaccessible state, one for volatile operations, and one for inaccessiblememonly, but that doesn't seem like a good tradeoff. inaccessiblememonly is already an approximation for the sake of making it easily usable: inaccessiblememonly calls have a dependency on other, unrelated, inaccessiblememonly calls, even if they don't actually touch the same state. If there's some intrinsic that's particularly sensitive to this, we can refine alias analysis for that intrinsic, either with a new attribute or a special case written in C++.

[hfinkel]

Yes, volatile loads and stores access the "same" state as inaccessiblememonly functions, I think.

I agree. If we need finer granularity sets of inaccessible state, we can create that and figure out how to attach it at that point.

I think that, in general, we're on the same page. It seems a bit unfortunate that a volatile *load* can modify this inaccessible state. In practice, however, it can (e.g., some hardware uses loads from some address as an automatically-sliding-window over some larger internal register file), and without this, it would be difficult to maintain ordering. We should be explicit about this, however, in the text: volatile loads can modify his inaccessible state (which is consistent with the current implementation: mayWriteToMemory returns true for volatile loads).

Also, I think that we should be explicit about call-graph impacts. volatile accesses may have target-specific side effects, but these don't include calling functions that are part of the module being compiled.

[efriedma]

Updated wording to clarify that volatile operations can modify inaccessible state, but not accessible state (except for the address of the operation). Clarify that all volatile operations can have side-effects, and can read and modify inaccessible state.

[efriedma]

Ping

[hfinkel]

In D53184#1365776, @efriedma wrote:

Ping

LGTM. Thanks!

[jfb]

llvm/trunk/docs/LangRef.rst

2196 ↗

(On Diff #182838)

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

[hfinkel]

llvm/trunk/docs/LangRef.rst

2196 ↗

(On Diff #182838)

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.

[jfb]

llvm/trunk/docs/LangRef.rst

2196 ↗

(On Diff #182838)

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.

[hfinkel]

llvm/trunk/docs/LangRef.rst

2196 ↗

(On Diff #182838)

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.

docs/LangRef.rst

 Certain memory accesses, such as :ref:`load <i_load>`'s,
 :ref:`store <i_store>`'s, and :ref:`llvm.memcpy <int_memcpy>`'s may be
 marked ``volatile``. The optimizers must not change the number of
 volatile operations or change their order of execution relative to other
 volatile operations. The optimizers *may* change the order of volatile
 operations relative to non-volatile operations. This is not Java's
 "volatile" and has no cross-thread synchronization behavior.
 
+A volatile load or store may have additional target-specific semantics;
+it may have side-effects, and it may access addresses which do not point
+to memory (like MMIO registers). This means the compiler may not use a
+volatile operation to prove a non-volatile access to that address has
+defined behavior.
+
+The allowed side-effects for volatile accesses are limited.  If a
+non-volatile store to the pointer value would be legal, a volatile
+operation may modify that memory. A volatile operation may not modify
+any other memory accessible by the module being compiled.
+
+The compiler may assume execution will continue after a volatile operation,
+so operations with side-effects or undefined behavior can be hoisted past
+a volatile operation.
+
 IR-level volatile loads and stores cannot safely be optimized into
 llvm.memcpy or llvm.memmove intrinsics even when those intrinsics are
 flagged volatile. Likewise, the backend should never split or merge
 target-legal volatile load/store instructions.
 
 .. admonition:: Rationale
 
  Platforms may rely on volatile loads and stores of natively supported