Fix broken openmp atomic usage

[p12tic]

OMP_ATOMIC_UPDATE and OMP_ATOMIC_WRITE are currently completely broken.

Currently OMP_ATOMIC_UPDATE and OMP_ATOMIC_WRITE are defined as follows:

#define OMP_ATOMIC_UPDATE _Pragma("omp atomic update")
#define OMP_ATOMIC_WRITE _Pragma("omp atomic write")

These macros are used as follows:

#pragma OMP_ATOMIC_UPDATE
#pragma OMP_ATOMIC_WRITE

Which results in the following code after preprocessor expansion:

#pragma _Pragma("omp atomic update")
#pragma _Pragma("omp atomic write")

This is invalid pragma which is ignored by the compiler. The correct usage of _Pragma is without #pragma as follows:

_Pragma("omp atomic update")
_Pragma("omp atomic write")

Fixing this issue reveals another problem that "omp atomic" is very restrictive in what expressions it accepts. In many cases the expressions include access to std::vector and so on, which "omp atomic" does not support.

This problem is addressed by using boost::atomic_ref which allows to apply atomic operations on any variable. Because "omp atomic" is hard to use, it is replaced with boost::atomic_ref in all cases.

As a result of this change we need to bump Boost dependency to 1.73.

[simogasp]

This must come from an old legacy code that has been merge. And it's indeed disturbing.
Can't we just make a find and replace for all those macro and replace them with the proper pragmas instead of using boost?
I don't know if mixing different things (omp, boost concurrency) is a good idea, especially when OpenMP has its own way to manage atomicity.

[p12tic]

@simogasp
It's unfortunately not easily possible. For example this does not work:

std::vector<int> x;
...
#pragma omp atomic
x[i]++;

OpenMP requires a separate variable in the expression to be modified. E.g.

std::vector<int> x;
...

auto& element = x[i];
#pragma omp atomic
element++;

I would also like to point out that usage of OpenMP does not change the underlying memory model that is defined by the C++ standard. As a consequence we can use whatever synchronization primitives we want without risk of breakage.

This PR should be viewed in light of #1236 which would move off OpenMP synchronization primitives in the rest of codebase.

[p12tic]

@fabiencastan @simogasp Is there anything missing from this PR that prevents it from being merged? It fixes a quite important issue in a way that prevents the issue from happening ever again and is actually more convenient compared to proper usage of #pragma omp atomic.

[p12tic]

@fabiencastan @simogasp Just a friendly ping :-)

It would be great if you could indicate if this PR will potentially be accepted. If not, I will create a separate PR that fixes the bug and refactor my other code to not assume that related changes will be accepted upstream.

[p12tic]

Thanks Fabien.