CMake 312

[henryiii]

This adds support for the new OpenMP discovery on macOS in CMake 3.12. It probably also helps on systems where something else, like -openmp, is used instead of -fopenmp. I'm getting a warning "unused arguments" from clang, but it's now working in OpenMP mode. (Before this patch, JIT compiling on macOS with brew install libomp was broken).

Also bypassing a bug in the MSVC server mode not supporting arbitrary strings in minimum version. Updating max policies to 3.12.

[ddemidov]

I am getting the following error with this:

FAILED: examples/CMakeFiles/benchmark.dir/benchmark.cpp.o 
/usr/bin/c++  -D/usr/lib/libpthread.so" -DVEXCL_BACKEND_JIT -DVEXCL_OMP_FLAGS="\"-fopenmp -l/usr/lib/libgomp.so" -I../ -O2 -g -DNDEBUG   -fopenmp -Wall -Wno-missing-braces -Wno-deprecated-declarations -Wno-ignored-attributes -Wno-unused-local-typedefs -std=gnu++11 -MD -MT examples/CMakeFiles/benchmark.dir/benchmark.cpp.o -MF examples/CMakeFiles/benchmark.dir/benchmark.cpp.o.d -o examples/CMakeFiles/benchmark.dir/benchmark.cpp.o -c ../examples/benchmark.cpp
/bin/sh: -c: line 0: unexpected EOF while looking for matching `"'
/bin/sh: -c: line 1: syntax error: unexpected end of file

Looks like VEXCL_OMP_FLAGS is misquoted.

[henryiii]

That's odd, a single extra " is being added.

[henryiii]

What system are you running on so I can debug it? I wasn't able to reproduce in an Ubuntu 18.04 docker container. EDIT: Nevermind, I both was able to reproduce and know what the problem is.

[ddemidov]

I am on Arch, but Travis seems to be having the same problem: https://travis-ci.org/ddemidov/vexcl/jobs/407576320#L634

[codecov[bot]]

Codecov Report

Merging #257 into master will decrease coverage by 0.91%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
- Coverage   88.13%   87.21%   -0.92%     
==========================================
  Files         128      129       +1     
  Lines       15235    13425    -1810     
==========================================
- Hits        13427    11709    -1718     
+ Misses       1808     1716      -92

Impacted Files	Coverage Δ
vexcl/backend/jit/compiler.hpp	84% <ø> (-0.62%)	:arrow_down:
vexcl/backend/common.hpp	0% <0%> (-100%)	:arrow_down:
vexcl/util.hpp	0% <0%> (-100%)	:arrow_down:
vexcl/backend/jit/source.hpp	0% <0%> (-99.04%)	:arrow_down:
vexcl/generator.hpp	0% <0%> (-96.53%)	:arrow_down:
vexcl/vector_view.hpp	0% <0%> (-95.06%)	:arrow_down:
vexcl/vector.hpp	0% <0%> (-90.91%)	:arrow_down:
vexcl/cache.hpp	0% <0%> (-90.91%)	:arrow_down:
vexcl/devlist.hpp	0% <0%> (-78.58%)	:arrow_down:
vexcl/backend/jit/context.hpp	0% <0%> (-66.67%)	:arrow_down:
... and 87 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1ce08cb...84a5346. Read the comment docs.

[ddemidov]

The examples are now able to compile, but I get

g++: error: unrecognized command line option ‘-fopenmp -l/usr/lib/libgomp.so -l/usr/lib/libpthread.so’

during runtime.

[ddemidov]

The compile command line looks like this:

[1/2] /usr/bin/c++  -DVEXCL_BACKEND_JIT -DVEXCL_OMP_FLAGS="\"-fopenmp -l/usr/lib/libgomp.so -l/usr/lib/libpthread.so\"" -I../ -O2 -g -DNDEBUG   -fopenmp -Wall -Wno-missing-braces -Wno-deprecated-declarations -Wno-ignored-attributes -Wno-unused-local-typedefs -std=gnu++11 -MD -MT examples/CMakeFiles/benchmark.dir/benchmark.cpp.o -MF examples/CMakeFiles/benchmark.dir/benchmark.cpp.o.d -o examples/CMakeFiles/benchmark.dir/benchmark.cpp.o -c ../examples/benchmark.cpp

so the problem I think is that the inner set of escaped quotes is not needed here.

[ddemidov]

Doing this:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d1c56a55..c78cc68f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -205,7 +205,7 @@ if(TARGET JIT)
        message(STATUS "VEXCL_OMP_FLAGS=${VEXCL_OMP_FLAGS}")

         # Pass the required flags to code
-        target_compile_definitions(JIT INTERFACE VEXCL_OMP_FLAGS="${VEXCL_OMP_FLAGS}")
+        target_compile_definitions(JIT INTERFACE VEXCL_OMP_FLAGS=${VEXCL_OMP_FLAGS})
     endif()

     target_link_libraries(JIT INTERFACE Common ${CMAKE_DL_LIBS})

sort of works, in the sense the runtime error changes to:

/usr/bin/ld: cannot find -l/usr/lib/libgomp.so
/usr/bin/ld: cannot find -l/usr/lib/libpthread.so
collect2: error: ld returned 1 exit status

[ddemidov]

This works both at compile and at runtime:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d1c56a55..9d6b4ce2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -200,12 +200,12 @@ if(TARGET JIT)

         set(VEXCL_OMP_FLAGS "${OpenMP_CXX_FLAGS}")
         foreach(item ${OpenMP_CXX_LIBRARIES})
-            set(VEXCL_OMP_FLAGS "${VEXCL_OMP_FLAGS} -l${item}")
+            set(VEXCL_OMP_FLAGS "${VEXCL_OMP_FLAGS} ${item}")
         endforeach()
        message(STATUS "VEXCL_OMP_FLAGS=${VEXCL_OMP_FLAGS}")

         # Pass the required flags to code
-        target_compile_definitions(JIT INTERFACE VEXCL_OMP_FLAGS="${VEXCL_OMP_FLAGS}")
+        target_compile_definitions(JIT INTERFACE VEXCL_OMP_FLAGS=${VEXCL_OMP_FLAGS})
     endif()

     target_link_libraries(JIT INTERFACE Common ${CMAKE_DL_LIBS})

[henryiii]

I'll check this fix tomorrow!

[henryiii]

That seems to work correctly on macOS, as well. We'll see how the tests fare.

[ddemidov]

This is strange: it passes the tests locally on my linux system, and on macOS at travis, but travis fails the linux tests both with g++ and clang.

[henryiii]

This should be a bit safer. It checks for -Xpreprocessor or -Xclang, which signals the flag is only affecting the preprocessor and not the linker, then adds the explicit libraries only in that case.

[ddemidov]

Ok, this looks good to me, thank you!

[henryiii]

Nice, libomp is already in the macOS testing, and brew is CMake 3.12, so the tests should be testing OpenMP on macOS. 👍