[PATCH] D65718: [LangRef] Document forward-progress requirement

[JF Bastien via Phabricator via llvm-commits]

jfb added a comment.

In D65718#1624863 <https://reviews.llvm.org/D65718#1624863>, @RalfJung wrote:

> > Does this need a side effect? Per the current definition, no. Per C++, yes.
>
> What does "need a side-effect" mean here? Do you mean whether `llvm.sideffect` has to be added to avoid UB?


I mean: if my language wants to generate a loop with a fence in it, and a non-atomic load, how do I get LLVM to generate that code without eliminating the code? Do I need `llvm.sideeffect`?

> Per my reading of C++, an atomic fence is considered a side-effect for the purpose of forward progress (looks like an atomic operation to me, even if it is relaxed), so this loop is //not// UB and may //not// be optimized away.

A fence is not an atomic operation on its own in C++. See http://eel.is/c++draft/atomics.fences where it says "is a synchronization operation *if* [...]".
Relaxed atomics aren't synchronization operations in C++.

>> The question of how things should be is harder
> 
> Is it worth saying in the docs that this is subject to discussion, or so?
>  Basically I hope we can use this patch as the //start// of a discussion, not as a way of cementing the current semantics (which are a bad fit for every language except for C++).

I see two approaches:

- Document what LLVM does today, get people to agree that it's actually what is done, leave a note saying "we'd like to change this", and propose changes from current practice in the RFC.
- Write down what you think the rules should be, including any changes to LLVM's builtins and optimizations, and send and RFC proposing that what you think is right be adopted.

What I don't want is a hand wavy description of what could be. Being imprecise is strictly worst than what we have today (undocumented) because it gives the impression of being correct, and of being something we won't change in the future.



================
Comment at: llvm/docs/LangRef.rst:2497
+(including but not limited to accessing inaccessible memory, volatile or
+atomic memory access, or other forms of thread synchronization). Failure to
+cause a side-effect in a finite amount of time results in undefined behavior.
----------------
RalfJung wrote:
> jfb wrote:
> > efriedma wrote:
> > > jfb wrote:
> > > > A relaxed atomic load is a sufficient side-effect?
> > > We want to consider a busy-wait loop like "while (foo){}", where "foo" is an atomic variable, as making forward progress; otherwise, there isn't any way to write such a loop.  We also want to allow erasing dead atomic loads.
> > > 
> > > We might need to modify the definition of a side-effect to make this work.
> > Right, I'm trying to understand what guarantees LLVM offers, and if they're the same or more than C++.
> > Where it makes sense I'd like this documented, with references.
> > 
> > http://eel.is/c++draft/intro.progress
> > http://eel.is/c++draft/intro.multithread#intro.races-3
> > 
> A loop `while (/* relaxed load */ == 0) {}` followed by an acquire-fence should definitely //not// be optimized away. That is a perfectly reasonable pattern to establish synchronization though fences and relaxed accesses.
> 
> The C++ standard specifically says "atomic operations" in loops are good enough, and while one can argue about whether a relaxed load synchronizes (it sometimes does, as in the above), I think we can agree that it is an atomic operation?
I asked about a loop with a relaxed atomic load, no acquire fence.
```
atomic<int> working;
// ...
while (working.load(memory_order_relaxed) != 0) {
  // Wait some more...
}
```
Can this be optimized out? Per C++... kinda?
What about the direct lowering to LLVM IR, does it need a side effect to avoid being optimized out?

For reference: wg21.link/n4455 and http://wg21.link/p0062



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

https://reviews.llvm.org/D65718