omp module logic troubled by newer cmake

[loriab]

I don't think the autocmake OpenMP module is playing nicely with modern Kitware OpenMP module.

In particular, when C/CXX/Fortran are active languages, if FindOpenMP finds flags & libs for C & CXX but not for Fortran, then OPENMP_FOUND is False, so no flags get added to CMAKE__FLAGS in this section. But then Fortran does pick up flags in this section b/c empty from previous. So net result (cmake 3.9) is that only C/CXX openmp detected by kitware module, but only CMAKE_Fortran_FLAGS end up with any openmp flags after passing through autocmake_omp.

[bast]

Thanks Lori! I will try to reproduce this.

[bast]

I believe there is a problem but at this moment cannot reproduce it with CMake 3.9.6 and GNU 8.1.0. I probably use a different compiler. Using which compilers do you observe this?

[loriab]

I'm on cmake 3.9.4 and gcc 7.2.0 (from conda). conda gfortran isn't providing omp_lib header or module, so that's probably why kitware findopenmp is failing to detect for Fortran, which starts the cascade.

[bast]

OK thanks. It would be nice if we could query OpenMP_<lang>_FOUND but these were only introduced in 3.9.

[bast]

We could for each language check whether either OpenMP_<lang>_FOUND or OpenMP_FOUND. Also the add_definitions(-DHAVE_OPENMP) seems kind of ugly from the modern CMake perspective. What is your opinion @robertodr?

[loriab]

Yes, I was just going to ask if there'd be forseeable trouble with if(${OPENMP_FOUND} OR ${OpenMP_C_FOUND}) per-language? I don't know what cmake wants in the way of quotes these days.

[bast]

I expect no trouble. Not sure what to do with the add_definitions but independently of it the current problem could be solved with:

if(DEFINED CMAKE_C_COMPILER_ID)
  if(OPENMP_FOUND OR OPENMP_C_FOUND)
    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
  endif()
endif()

# etc. for the other languages

On the other hand the "proper" way for 3.9 and above is actually this:
https://github.com/bast/cmake-recipes/blob/master/chapter-03/recipe-05/cxx-example/CMakeLists.txt

[robertodr]

The add_definitions should stay as-is, I think. @bast did copying the modern FindOpenMP.cmake module work for OpenMP detection? I think that would actually be the most reasonable choice...

[bast]

You mean shipping the latest FindOpenMP.cmake with autocmake and checking that it works also with 2.8? Just to make sure I understand the question.

[loriab]

I'll experiment with those lines, thanks.

I'm a huge fan of the imported target in general. But I don't dare do it here b/c of the problem I was discussing with @robertodr about needing to suppress lgomp linking for GCC+MKL (this is also the problem that led to the nasty table here. So basically I want the flags out of FindOpenMP but not the link libs.

[robertodr]

You can still get the flags out from the new module (though that might be on its way to deprecation)

[loriab]

By as-is on the definitions, you mean set when C found, @robertodr? Or potentially added three times if all langs enabled?

[robertodr]

Gee, what a mess! No we definitely don't want that, so I guess either they spill over to the client:

if(OpenMP_FOUND)
  target_compile_definitions(<target-name>
     PUBLIC
        HAVE_OPENMP
     )
endif()
# or a generator expression
target_compile_definitions(<target-name>
  PUBLIC
    "$<$<BOOL:${OPENMP_FOUND}>:HAVE_OPENMP>"
  )

or it spills over to the compiler flags:

set(CXX_FLAGS ${OpenMP_CXX_FLAGS} -DHAVE_OPENMP)

quite ugly in both cases, but I'd rather prefer the former: explicit is better than implicit (of sorts) and you get to choose the name of the preprocessor definition!

@bast, yes that's what I meant. I recall it didn't work out for FindMPI.cmake. (This thread is moving very fast, I'm losing half of the comments 😸 )

[loriab]

I'm going with

    set(_omp_enabled_for_enabled_lang OFF)

    if(DEFINED CMAKE_C_COMPILER_ID)
        if(OPENMP_FOUND OR OpenMP_C_FOUND)
            set(_omp_enabled_for_enabled_lang ON)
            set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
        endif()
    endif()

    if(DEFINED CMAKE_CXX_COMPILER_ID)
        if(OPENMP_FOUND OR OpenMP_CXX_FOUND)
            set(_omp_enabled_for_enabled_lang ON)
            set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
        endif()
    endif()

    if(DEFINED CMAKE_Fortran_COMPILER_ID)
        if(OPENMP_FOUND OR OpenMP_Fortran_FOUND)
            set(_omp_enabled_for_enabled_lang ON)
            set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} ${OpenMP_Fortran_FLAGS}")
        endif()
    endif()

    if(${_omp_enabled_for_enabled_lang})
        add_definitions(-DHAVE_OPENMP)
    endif()