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

Kokkos launch compiler updates #3778

Merged
merged 6 commits into from
Mar 3, 2021

Conversation

jrmadsen
Copy link
Contributor

@jrmadsen jrmadsen commented Feb 4, 2021

  • Creates new find_package(Kokkos COMPONENTS launch_compiler) mode for using the launch compiler feature regardless of whether CUDA is enabled
  • kokkos_launch_compiler was extended to support forwarding to the original compiler Kokkos was compiled with
  • There is only one known caveat: kokkos is installed with nvcc_wrapper and downstream application uses clang
    • This results in the default host compiler to use NVCC_WRAPPER_DEFAULT_COMPILER from environment or nothing and without specify NVCC_WRAPPER_DEFAULT_COMPILER during compilation clang will be used and it will fail
    • Moral of the story: don't use nvcc_wrapper anymore when installing kokkos

- Restored -DKOKKOS_DEPENDENCE to KOKKOS_LINK_OPTIONS
- Verbose message
- Added new "launch_compiler" for Kokkos_FIND_COMPONENTS to enable universally
- updated kokkos_launch_compiler to handle more than just nvcc_wrapper
- Added build_cmake_installed_different_compilers for testing
- Replaced @Kokkos_CXX_COMPILER_ID@ with @KOKKOS_CXX_COMPILER_ID@
- Configure @NVCC_WRAPPER_DEFAULT_COMPILER@ into kokkos_launch_compiler
- Update internal version of kokkos_compilation
CMakeLists.txt Show resolved Hide resolved
bin/kokkos_launch_compiler Outdated Show resolved Hide resolved
bin/kokkos_launch_compiler Outdated Show resolved Hide resolved
jrmadsen and others added 2 commits February 4, 2021 16:42
Co-authored-by: Daniel Arndt <arndtd@ornl.gov>
@jrmadsen
Copy link
Contributor Author

jrmadsen commented Feb 8, 2021

Retest this please

1 similar comment
@jrmadsen
Copy link
Contributor Author

Retest this please

@crtrott
Copy link
Member

crtrott commented Mar 2, 2021

I guess this is ok? But I just don't know. What happened before if you installed Kokkos with nvcc_wrapper and then downstream use clang? But we can just say we don't support that.

@DavidPoliakoff DavidPoliakoff self-requested a review March 2, 2021 19:03
@jrmadsen
Copy link
Contributor Author

jrmadsen commented Mar 2, 2021

What happened before if you installed Kokkos with nvcc_wrapper and then downstream use clang?

Before this, i don't know. You probably just got compiler-flag errors since Kokkos would export compiler flags for NVCC. After this PR, the problem with installing Kokkos via nvcc_wrapper is that we don't actually know the underlying host compiler at CMake time (since they could configure and then set NVCC_WRAPPER_DEFAULT_COMPILER in the env later) so it can't save that compiler for downstream projects. Thus if the default g++ is version 4.9 and the user set NVCC_WRAPPER_DEFAULT_COMPILER=g++-8 when building Kokkos after cmake was run, kokkos_launch_compiler in the install tree would not "know" to set NVCC_WRAPPER_DEFAULT_COMPILER to g++-8.

@jrmadsen
Copy link
Contributor Author

jrmadsen commented Mar 2, 2021

I actually think this makes a rather strong argument for us deprecating the direct use of nvcc_wrapper. E.g. make a really annoying warning in the Kokkos CMake that says stop setting the C++ compiler to nvcc_wrapper.

@jrmadsen
Copy link
Contributor Author

jrmadsen commented Mar 2, 2021

And it would be a win in my and @DavidPoliakoff war against compiler-wrappers.

@jrmadsen
Copy link
Contributor Author

jrmadsen commented Mar 2, 2021

But we can just say we don't support that.

Also, now it is supported. Since you could do find_package(Kokkos COMPONENTS launch_compiler) and even if the downstream compiler were clang, you would end up compiling your kokkos code with nvcc_wrapper.

@crtrott crtrott merged commit 2d9e256 into kokkos:develop Mar 3, 2021
masterleinad added a commit to masterleinad/kokkos that referenced this pull request Mar 11, 2021
* Support for generic use of kokkos_launch_compiler

- Restored -DKOKKOS_DEPENDENCE to KOKKOS_LINK_OPTIONS
- Verbose message
- Added new "launch_compiler" for Kokkos_FIND_COMPONENTS to enable universally
- updated kokkos_launch_compiler to handle more than just nvcc_wrapper
- Added build_cmake_installed_different_compilers for testing

* Miscellanous fixes

- Replaced @Kokkos_CXX_COMPILER_ID@ with @KOKKOS_CXX_COMPILER_ID@
- Configure @NVCC_WRAPPER_DEFAULT_COMPILER@ into kokkos_launch_compiler
- Update internal version of kokkos_compilation

* Fix to configuration of kokkos_launch_compiler

* Removed Kokkos_LAUNCH_COMPILER_INFO --> Should be in docs!

* Apply suggestions from code review

Co-authored-by: Daniel Arndt <arndtd@ornl.gov>

* formatting

Co-authored-by: Daniel Arndt <arndtd@ornl.gov>
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.

None yet

4 participants