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

[Nikita Popov via Phabricator via llvm-commits]

nikic planned changes to this revision.
nikic marked an inline comment as done.
nikic added a comment.

This documentation should probably be combined with the introduction of the `requires_forward_progress` attribute. The current codebase in not consistent in this regard (e.g. we have code in LoopDeletion specifically assuming no forward-progress requirement). It would be better to condition that kind of code on an attribute rather than drop it entirely and add it back later.



================
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.
----------------
jfb wrote:
> 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
> 
The [intro.progress] link you provided considers "perform[ing] a synchronization operation **or** an atomic operation" (emphasis mine) as forward progress. Whether the relaxed atomic load synchronizes is besides the point: It is still an atomic operation and as such sufficient to constitute forward progress. This is also consistent with the definition given here. Am I misunderstanding your concern?


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

https://reviews.llvm.org/D65718