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

If CMAKE_CXX_COMPILER_LAUNCHER is set , do not enable kokkos_launch_compiler #4870

Merged
merged 5 commits into from
Mar 30, 2022

Conversation

ajpowelsnl
Copy link
Contributor

@ajpowelsnl ajpowelsnl commented Mar 11, 2022

This PR adds another condition in setting INTERNAL_USE_COMPILER_LAUNCHER to prevent interaction with
kokkos_launch_compiler when CMAKE_CXX_COMPILER_LAUNCHER is set. This change fixes #4821.

@msimberg
Copy link
Contributor

Works for me, as in I get an error during CMake configuration suggesting to use nvcc_wrapper, clang, or kokkos_launch_compiler. However, since the last one is not actually an option I would maybe suggest to error out earlier with a message specific to CMAKE_CXX_COMPILER_LAUNCHER. Do you think that's reasonable?

@ajpowelsnl
Copy link
Contributor Author

Works for me, as in I get an error during CMake configuration suggesting to use nvcc_wrapper, clang, or kokkos_launch_compiler. However, since the last one is not actually an option I would maybe suggest to error out earlier with a message specific to CMAKE_CXX_COMPILER_LAUNCHER. Do you think that's reasonable?

Hi @msimberg -- I've added the error condition you requested. Please let us know if we can do anything further to help.

Comment on lines 1095 to 1096
m_scratch_locks =
m_policy.space().impl_internal_space_instance()->m_scratch_locks;
Copy link
Contributor

Choose a reason for hiding this comment

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

Neither this...

Comment on lines 1029 to 1030
m_scratch_locks =
m_policy.space().impl_internal_space_instance()->m_scratch_locks;
Copy link
Contributor

Choose a reason for hiding this comment

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

...nor this...

Comment on lines 852 to 858

Kokkos::parallel_reduce(
team_exec.set_scratch_size(1, Kokkos::PerTeam(team_scratch_size),
Kokkos::PerThread(thread_scratch_size)),
Functor(), Kokkos::Sum<typename Functor::value_type>(error_count));
Kokkos::fence();
ASSERT_EQ(error_count, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

...or this should be in this pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh my...! I do not know how those changes got into this PR. Maybe in my rebasing ...? In any case, I'll remove this stuff. Thank you for catching that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For all cases of files that shouldn't be in this PR, I've checked out the develop versions of these files, and there doesn't seem to be any difference between the versions. I will look back through the commits to see how these errant files ended up in this PR.

Comment on lines 43 to 44
# compiler AND `CMAKE_CXX_COMPILIER_LAUNCHER` is not set, set to use launcher. Will ensure CMAKE_CXX_COMPILER
# is replaced by nvcc_wrapper
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# compiler AND `CMAKE_CXX_COMPILIER_LAUNCHER` is not set, set to use launcher. Will ensure CMAKE_CXX_COMPILER
# is replaced by nvcc_wrapper
# compiler and `CMAKE_CXX_COMPILIER_LAUNCHER` is not set, set to use launcher.
# Will ensure CMAKE_CXX_COMPILER is replaced by nvcc_wrapper

Comment on lines 45 to 48
IF(Kokkos_COMPILE_LAUNCHER AND NOT INTERNAL_HAVE_COMPILER_NVCC AND NOT
CMAKE_CXX_COMPILER_LAUNCHER AND NOT KOKKOS_CXX_COMPILER_ID STREQUAL Clang)
MESSAGE(FATAL_ERROR "Cannot use CMAKE_CXX_COMPILER_LAUNCHER if the CMAKE_CXX_COMPILER is not able to compile CUDA code, i.e. nvcc_wrapper or
clang++!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use tabs.

Suggested change
IF(Kokkos_COMPILE_LAUNCHER AND NOT INTERNAL_HAVE_COMPILER_NVCC AND NOT
CMAKE_CXX_COMPILER_LAUNCHER AND NOT KOKKOS_CXX_COMPILER_ID STREQUAL Clang)
MESSAGE(FATAL_ERROR "Cannot use CMAKE_CXX_COMPILER_LAUNCHER if the CMAKE_CXX_COMPILER is not able to compile CUDA code, i.e. nvcc_wrapper or
clang++!")
IF(Kokkos_COMPILE_LAUNCHER AND NOT INTERNAL_HAVE_COMPILER_NVCC AND NOT
CMAKE_CXX_COMPILER_LAUNCHER AND NOT KOKKOS_CXX_COMPILER_ID STREQUAL Clang)
MESSAGE(FATAL_ERROR "Cannot use CMAKE_CXX_COMPILER_LAUNCHER if the CMAKE_CXX_COMPILER is not able to compile CUDA code, i.e. nvcc_wrapper or
clang++!")

`INTERNAL_USE_COMPILER_LAUNCHER` to prevent interaction with
kokkos_launch_compiler when `CMAKE_CXX_COMPILER_LAUNCHER` is set
`CMAKE_CXX_COMPILER_LAUNCHER` is set, and the `CMAKE_CXX_COMPILER` cannot
compile Cuda code
kokkos_compiler_id.cmake: Add another condition on setting
`INTERNAL_USE_COMPILER_LAUNCHER` to prevent interaction with
kokkos_launch_compiler when `CMAKE_CXX_COMPILER_LAUNCHER` is set
Copy link
Contributor

@msimberg msimberg left a comment

Choose a reason for hiding this comment

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

Thanks @ajpowelsnl! This looks good to me now.

@masterleinad
Copy link
Contributor

CUDA-9.2-NVCC failing in default_exec.overlap_mdrange_policy is spurious.

IF(Kokkos_COMPILE_LAUNCHER AND NOT INTERNAL_HAVE_COMPILER_NVCC AND NOT KOKKOS_CXX_COMPILER_ID STREQUAL Clang)
IF(CMAKE_CXX_COMPILER_LAUNCHER)
MESSAGE(FATAL_ERROR "Cannot use CMAKE_CXX_COMPILER_LAUNCHER if the CMAKE_CXX_COMPILER is not able to compile CUDA code, i.e. nvcc_wrapper or
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MESSAGE(FATAL_ERROR "Cannot use CMAKE_CXX_COMPILER_LAUNCHER if the CMAKE_CXX_COMPILER is not able to compile CUDA code, i.e. nvcc_wrapper or
MESSAGE(FATAL_ERROR "Cannot use CMAKE_CXX_COMPILER_LAUNCHER if the CMAKE_CXX_COMPILER is not able to compile CUDA code, i.e. nvcc_wrapper or

@ajpowelsnl
Copy link
Contributor Author

CUDA-9.2-NVCC failing in default_exec.overlap_mdrange_policy is spurious.

OK, thanks for the head's up.

@ajpowelsnl
Copy link
Contributor Author

ajpowelsnl commented Mar 28, 2022

Hi @dalg24 - retest this, please.

@masterleinad
Copy link
Contributor

Retest this please.

@crtrott crtrott merged commit 46ef54d into kokkos:develop Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting CMAKE_CXX_COMPILER_LAUNCHER breaks kokkos_launch_compiler
4 participants