Select OpenMP data sharing mode based on specific GCC versions

[SunBlack]

Starting with Clang 10 (released some days ago) clang supports OpenMP 4.5 (201511) instead of 3.1 (201107). As value of _GNUC is still 4, we need to extend our exception for GCC, so that Clang can use the newer data sharing model.

Note: Clang 10 can OpenMP 5.0, too, if you add compiler flag -fopenmp-version=50. But as far as I can see we don't get additional compiler errors with OpenMP 5.0 (only short check via godbolt).

[kunaltyagi]

We should be checking for clang version in this case too, right? Or is the clang version strictly dependent on the OpenMP version?

PS: maybe some more parenthesis in the macro would help

[SunBlack]

We should be checking for clang version in this case too, right? Or is the clang version strictly dependent on the OpenMP version?

I don't think so, as we only need a version check for GCC (as already mentioned here). And as currently all Clang versions are working on gobolt with this macro, I don't believe Clang requires it too.

PS: maybe some more parenthesis in the macro would help

So you suggest (?):

#if (defined _OPENMP && (_OPENMP <= 201307)) || (__GNUC__ < 9 && !defined __clang__)

[kunaltyagi]

The macro reads funny. Perhaps refactor it by compiler behavior. If GCC then this, else if clang then this. This might be more manageable in case clang changes behavior (or we add support MSVC + OpenMP. Do we already?)

[SunBlack]

There is no specific flag where I can see it GCC or not. Alternatively I could use (__GNUC__ => 6 && __GNUC__ < 9) (GCC 6 is the first Version which have the wrong data sharing rule), as clang reports always __GNUC__ == 4. But I have no idea if there are other compilers which defines __GNUC__, so no idea which check is the best.

Or do we have a value we are getting from CMake?

If we would not support GCC #if (defined _OPENMP && (_OPENMP <= 201307)) would be enough, as all other compilers seem to stick to the data sharing role that matches the OpenMP version.

[kunaltyagi]

all other compilers seem to stick to the data sharing role that matches the OpenMP version

In that case, just make en exception for GCC using the 2 diff rules (one for GCC version, one for OpenMP). No need to drag other compilers in 😄

[SunBlack]

all other compilers seem to stick to the data sharing role that matches the OpenMP version

In that case, just make en exception for GCC using the 2 diff rules (one for GCC version, one for OpenMP). No need to drag other compilers in 😄

You are right. In case someone has trouble with any other compiler, which define itself as Gnu compiler, we can check what we do then.

I adjusted the code as it was currently a bit lucky that it worked, as I didn't checked if __GNUC__ is defined before I checked the value.

[kunaltyagi]

Much better now 😄

[kunaltyagi]

A better title for the changelog...

And two more eyes