[Openmp-commits] [PATCH] D76780: [OpenMP] Added memory barrier to solve data race

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

In D76780#1942190 <https://reviews.llvm.org/D76780#1942190>, @bryanpkc wrote:

> In D76780#1942144 <https://reviews.llvm.org/D76780#1942144>, @bryanpkc wrote:
>
> > Why doesn't the `KMP_MB` after the store to `th_next_waiting` guarantee that the unblocked thread sees that store?
>
>
> Answering my own question, the ARM architecture reference manual states that the `DMB` instruction ensures that all affected memory accesses by the PE executing the `DMB` that appear in program order before the `DMB` and those which originate from a different PE, ...which have been Observed-by the PE before the `DMB` is executed, are Observed-by each PE, ...before any affected memory accesses that appear in program order after the `DMB` are Observed-by **that PE**.
>
> I think this means, in this case, that the store of `FALSE` to `th_spin_here` is observed in the correct order only by the releasing thread that issued the `DMB`. Other threads (e.g. the spinning thread) could still see the updates to `th_spin_here` and `th_next_waiting` out of order, unless they also issue `DMB`, which is the fix in this patch.


Yes, without reference to a specific architecture I'd formulate it as follows:

1. releasing thread

  th_next_waiting = 0;
  KMP_MB();
  th_spin_here = FALSE;

ensures that the write to `th_next_waiting` is committed to memory **before** `th_spin_here` is set.

2. spinning thread

  KMP_WAIT(spin_here_p, FALSE, KMP_EQ, lck);
  KMP_MB();
  th_next_waiting != 0

ensures that the read of `th_next_waiting` comes from memory **after** spinning on `th_spin_here`.

Taken together this is the idiom to synchronize a value from one thread to another. (Well I think a full memory barrier is actually more than would be needed...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76780