Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use of OpenMP with Cuda or HIP as compile language #6972

Merged

Conversation

dalg24
Copy link
Member

@dalg24 dalg24 commented May 1, 2024

We recently discovered that configuring with

-DKokkos_ENABLE_{CUDA,HIP}=ON \
-DKokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE=ON \
-DKokkos_ENABLE_OPENMP=ON`

was broken and that the compiler flags to enable OpenMP support were not properly set, triggering an internal sanity check at compile time

In file included from <kokkos>/core/src/impl/Kokkos_Core.cpp:21:
In file included from <kokkos>/core/src/Kokkos_Core.hpp:45:
In file included from <kokkos>/build/compile_language/KokkosCore_Config_DeclareBackend.hpp:22:
In file included from <kokkos>/core/src/decl/Kokkos_Declare_OPENMP.hpp:21:
In file included from <kokkos>/core/src/OpenMP/Kokkos_OpenMP.hpp:203:
<kokkos>/core/src/OpenMP/Kokkos_OpenMP_Instance.hpp:23:2: error: "You enabled Kokkos OpenMP support without enabling OpenMP in the compiler!"
#error \

This PR adds these flag for compiling the CUDA/HIP source files. In contrast to the solution suggested by Daniel in #6965 (review), I am adding these flags in the cmake file that handles TPLs. The reason for doing it there instead of in the kokkos_arch.cmake file is that TPLs are only processed after which means that OpenMP_CXX_FLAGS is not defined the first time Kokkos is configured. I expect it only worked for Daniel because it was a reconfigure (that is 2nd time parsing the file with OpenMP now having been found).

I have tested (might be a strong word, I only checked the flag is properly added and the library builds) on Frontier with ROCm 5.7.1 but I did not check that it works for CUDA. @masterleinad do you mind testing it?

@dalg24 dalg24 added Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) Backend - CUDA CMake Backend - OpenMP labels May 1, 2024
@dalg24 dalg24 requested review from masterleinad and Rombur and removed request for masterleinad May 1, 2024 13:27
Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me on polaris with the Cuda backend. It seems we don't need to add any linker flags. I'm fine with looking into that if it ever comes up.

@crtrott crtrott merged commit ed4d254 into kokkos:develop May 1, 2024
25 of 29 checks passed
@Rombur
Copy link
Member

Rombur commented May 1, 2024

I tried this PR on Frontier and it works fine but the library does not contain the path to libomp. I needed to set LD_LIBRARY_PATH. I don't know if this is expected.

@ndellingwood
Copy link
Contributor

Does this merit a changelog entry for 4.4 #6914 , or was this a fixup only applicable to recent changes on develop?

@masterleinad
Copy link
Contributor

Does this merit a changelog entry for 4.4 #6914 , or was this a fixup only applicable to recent changes on develop?

done

@dalg24 dalg24 deleted the fix_kokkos_compile_language_cuda_hip_w_omp branch May 1, 2024 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend - CUDA Backend - HIP Backend - OpenMP Bug Broken / incorrect code; it could be Kokkos' responsibility, or others’ (e.g., Trilinos) CMake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants