cmake: Detect whether or not atomic is needed

[malaterre]

It turns out that GCC requires explicitly linking to a library atomic in order to support c11/atomics. gcc spec file will not handle it directly at least not on the following Debian archs: armel, m68k, mipsel, powerpc, sh4, sparc64 and riscv64.

Introduce a new cmake module to detect usage of gcc/atomic and add missing library to the link step.

For reference:

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

Fixes #1003

[jan-wassenberg]

Nice, thanks for adding this. It looks like the clang-5 builder/check is still failing, though it does seem fine on godbolt. Any ideas on how we might work around that?

[malaterre]

@jan-wassenberg I cannot get my hand on a clang-5 compiler. Any way you could add something like:

cat /home/runner/work/highway/highway/out/CMakeFiles/CMakeOutput.log
cat /home/runner/work/highway/highway/out/CMakeFiles/CMakeError.log

somewhere in your CI setup (in case of error)

[malaterre]

@jan-wassenberg Two things:

1. I seems the CI has stopped working on this PR
2. This code has been working nicely in libjxl: libjxl/libjxl@87fe7c1

I agree that on libjxl I used set(CMAKE_CXX_STANDARD 11) instead of a direct -std=c++11 but I fail to see why it would make any difference.

[malaterre]

lol, someone is adding -std=c++98 in my back:

/usr/bin/clang++-5.0 -DATOMICS_IN_LIBRARY  -Werror -std=c++11 -fPIE -std=c++98 -MD -MT CMakeFiles/cmTC_16d49.dir/src.cxx.o -MF CMakeFiles/cmTC_16d49.dir/src.cxx.o.d -o CMakeFiles/cmTC_16d49.dir/src.cxx.o -c /home/runner/work/highway/highway/out/CMakeFiles/CMakeTmp/src.cxx

@eustas any comment ?

[jan-wassenberg]

Good old CMake, it often makes a mess, sigh.

Very difficult to debug without access to the logs, that's a great trick with reading+printing it, nice!

It seems to me that CMAKE_CXX_STANDARD would be the correct way to avoid the problem, but it's only supported in 3.1. There's also target_compile_features but that seems to only be for targets (not sure it would work with try_compile), and it's also 3.1 only. I wonder if the CMake running is older than 3.1? The logs don't seem to mention CMake version.

[malaterre]

FYI:

• https://discourse.cmake.org/t/cmake-cxx-standard-cmake-3-16-vs-3-24/6623

[jan-wassenberg]

hm, so a CMake misbehavior indeed. Nice that you've raised it with them, let's see if there's a workaround.

[malaterre]

@jan-wassenberg Seems like this one is not my fault:

vqsort-inl.h:73:49: error: unused parameter 'p' [-Werror,-Wunused-parameter]

[jan-wassenberg]

Oh, indeed :) Fixing, thanks for pointing that out. (Our internal CI is still somehow suppressing unused arg warnings.)

I appreciate your heroic efforts to get this working, looks like we're almost there?

[malaterre]

I appreciate your heroic efforts to get this working, looks like we're almost there?

lol :) Please do not merge, I find my current patch sort of hackish. I'd like to get to the bottom issue of this set(CMAKE_CXX_EXTENSIONS OFF) issue. I'll rebase after your warnings fix. Thanks

[jan-wassenberg]

:) The fix is in.

[jan-wassenberg]

Congrats, seems you have found a decent workaround for the CMake issue. Are you happy with the current code, should we merge it?

[malaterre]

Are you happy with the current code, should we merge it?

I believe this is a good improvement. As mentionned in the comment, I still need to dig in cmake to understand the difference in behavior between 3.16 vs 3.24, but I suspect this should not hold this PR. So please merge, thanks !

[malaterre]

@jan-wassenberg I have no clue why clang5/6 are marked as This job was cancelled

[jan-wassenberg]

huh, strange, I thought I had seen them pass. Rerunning now..

[jan-wassenberg]

"GitHub Actions has encountered an internal error when running your job."
This seems to happen often:
actions/runner-images#6131
actions/runner-images#2179
actions/runner-images#4627
actions/runner-images#2579