commit fc46cfb297cb89e1e36e0940ecf77f2a3eda8815
author bjacob <benoitjacob@google.com> Thu Jul 07 13:54:24 2022 -0400
committer GitHub <noreply@github.com> Thu Jul 07 13:54:24 2022 -0400
tree 3574ffb36722eaf4a80a6f2a7a5b06b0eea67ec7
parent 4804bd33746ee1c5528f49d61b5f65b5bd34337f

Access worker->state with acquire/release, not seq_cst. (#9735)

Actually one place was already acquire/release, inconsistently with the
rest that was seq_cst. That was likely a bug, though it hadn't
manifested itself so far under TSan.

We had to standardize one way or the other: either all seq_cst or
all acquire/release.

I thought about why we might need this to be seq_cst and though actually
we probably don't. Changing to weakly ordered means that the order in
which different workers change state may be seen differently by
different threads, but that doens't seem to matter. It would have move
consequences if there were other atomics that we are accessing seq_cst
in conjunction with worker->state and we actually relied on sequential
consistency, but there don't really seem to be.

The comment in worker.h points to worker->wake_notification, and the
implementation in synchronization.c has its own issues with inconsistent
memory orders (seq_cst in iree_notification_deinitialize vs acq_rel in
iree_notification_post) so it doesn't seem that we would rely on the
seq_cst there being actually sequentially consistent with worker->state
changes.