[Bug c++/98632] New: Warn about unspecified expression ordering for atomics with non-relaxed memory ordering.

[Bug c++/98632] New: Warn about unspecified expression ordering for atomics with non-relaxed memory ordering.

[tilps at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98632

            Bug ID: 98632
           Summary: Warn about unspecified expression ordering for atomics
                    with non-relaxed memory ordering.
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tilps at hotmail dot com
  Target Milestone: ---

C++ defines that execution ordering for expressions is largely unspecified.  In
cases where there are multiple atomic operations in an expression for which
there is no standard required ordering, and if those atomic operations are
marked with a non-relaxed memory ordering, it would be useful to have a
warning. Since the compiler is technically free to reorder them in-spite of the
memory ordering indicating that the user cares about the specific ordering.

While it might be able to be argued that the warning should fire for any
expression involving just a single atomic and some other expression component
that would be unable to be reordered if the sub-expressions had been assigned
to locals first, it seems that would be likely to have vastly more false
positives than expressions that involve multiple atomic operations. So I would
suggest only triggering for expressions involving multiple atomic operations.

[Bug c++/98632] Warn about unspecified expression ordering for atomics with non-relaxed memory ordering.

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98632

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Please provide a code example, showing the warnings you'd like for it.

[Bug c++/98632] Warn about unspecified expression ordering for atomics with non-relaxed memory ordering.

[tilps at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98632

--- Comment #2 from tilps at hotmail dot com ---
*rough sketch*

class TaskConsumer {

  void run() {
    if (taken_count_.load(std::memory_order_acquire) <
task_count_.load(std::memory_order_acquire)) {
      taken_count_.fetch_add(1, std::memory_order_acq_rel);
      // ... handle the new task...
    }
  }

  void addTask() {
    task_count_.fetch_add(1, std::memory_order_acq_rel);
  }

  void reset() {
    task_count_.store(0, std::memory_order_release);
    taken_count_.store(0, std::memory_order_release);
  }

  std::atomic<int> task_count_;
  std::atomic<int> taken_count_;
};

The above is not a 'complete' code sample, just illustrative.
One thread is calling 'run' in a loop.
Other thread calls reset, then add task some number of times.  Waits until it
knows the first thread has done all tasks (not covered in the code above, but
assume that it is thread-safe and establishes acquire/release ordering as
appropriate), then calls reset again and repeats the process.

In order for the 'if' statement to behave correctly taken count must be read
before task count. If task count is read first it can read the value in
task_count_ before reset, but the taken_count_ value after reset.

If an optimizer (not necessarily an existing gcc one) decides to reorder the if
statement because the standard says order is unspecified and it notices that
task_count_ is an earlier memory address to taken_count_ and presumes
reordering might give a small performance increase due to sequential memory
access and cpu prefetching assumptions... then the code breaks.

Thus I would like a warning pointing at that if statement with wording along
the lines of:
C++ standard does not guarantee ordering of evaluation in this expression, thus
the order of atomic operations with non-relaxed memory ordering in this
expression is unspecified. Extract them into separate expressions to guarantee
that the ordering is consistent with the memory ordering annotations.