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

Compile as cmake language #555

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JBludau
Copy link
Contributor

@JBludau JBludau commented Jul 26, 2024

Some documentation for the Kokkos as CMake language that Kokkos already provides and that solve the problem described in kokkos/kokkos#7144

Will stay a draft for now as we might need to update the example in core and add stuff to the tutorial

@masterleinad masterleinad requested a review from ajpowelsnl July 29, 2024 18:59
@masterleinad
Copy link
Contributor

Looks like we need something like

diff --git a/cmake/KokkosConfigCommon.cmake.in b/cmake/KokkosConfigCommon.cmake.in
index d3ac39ffa..fe021199e 100644
--- a/cmake/KokkosConfigCommon.cmake.in
+++ b/cmake/KokkosConfigCommon.cmake.in
@@ -33,6 +33,9 @@ ENDFOREACH()
 
 IF(Kokkos_ENABLE_CUDA)
   SET(Kokkos_CUDA_ARCHITECTURES @KOKKOS_CUDA_ARCHITECTURES@)
+  IF(Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE)
+    SET(CMAKE_CUDA_ARCHITECTURES @KOKKOS_CUDA_ARCHITECTURES@)
+  ENDIF()
 ENDIF()
 
 IF(Kokkos_ENABLE_HIP)
@@ -300,8 +303,8 @@ FUNCTION(kokkos_compilation)
             # set the properties if defined
             IF(COMP_${_TYPE})
                 # MESSAGE(STATUS "Using ${COMP_COMPILER} :: ${_TYPE} :: ${COMP_${_TYPE}}")
-                SET_PROPERTY(${_TYPE} ${COMP_${_TYPE}} PROPERTY RULE_LAUNCH_COMPILE "${Kokkos_COMPILE_LAUNCHER} ${COMP_COMPILER} ${CMAKE_CXX_COMPILER}")
-                SET_PROPERTY(${_TYPE} ${COMP_${_TYPE}} PROPERTY RULE_LAUNCH_LINK "${Kokkos_COMPILE_LAUNCHER} ${COMP_COMPILER} ${CMAKE_CXX_COMPILER}")
+                SET_PROPERTY(${_TYPE} ${COMP_${_TYPE}} PROPERTY LANGUAGE ${Kokkos_COMPILE_LANGUAGE})
+                SET_PROPERTY(${_TYPE} ${COMP_${_TYPE}} PROPERTY CUDA_ARCHITECTURES ${Kokkos_CUDA_ARCHITECTURES})
             ENDIF()
         ENDFOREACH()
     ENDIF()

for this to be usable with Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE.

@masterleinad
Copy link
Contributor

The challenge is basically that we should set CUDA_ARCHITECTURES for every target that is to be compiled with Kokkos (and we can't set the property for a source file). On the other hand, we can only set the compile language for a source file, not for a target and we can't really iterate over the source files of a target in general. That means we have to use kokkos_compilation on all the source files and we must find Kokkos before declaring any targets (or also use kokkos_compilation on all targets).

@ajpowelsnl
Copy link
Contributor

ajpowelsnl commented Jul 30, 2024

@cedricchevalier19 - this entry is relevant to 7144, CMAKE_AS_LANGUAGE and kokkos_compilation()

@dalg24
Copy link
Member

dalg24 commented Jul 30, 2024

@cedricchevalier19 - this entry is possibly relevant to 7144, CMAKE_AS_LANGUAGE and kokkos_compilation()

Jakob clearly states in the description that this work is a proposed resolution for that very issue so yes it is (hopefully) relevant

@masterleinad
Copy link
Contributor

kokkoc_compilation in its current form only sets the launch compiler. Hence, it's not useful with Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE=ON. I think we should just document the current behavior for now.

docs/source/building.md Outdated Show resolved Hide resolved
@@ -49,6 +49,20 @@ There are numerous device backends, options, and architecture-specific optimizat
````
which activates the OpenMP backend. All the options controlling device backends, options, architectures, and third-party libraries (TPLs) are given in [CMake Keywords](../keywords).

## Separate Compilation via CMake Language
Copy link
Member

Choose a reason for hiding this comment

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

I know it is now from you, but I am wondering why "separate compilation"?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's technically "separable compilation" and was introduced in kokkos/kokkos#3136 with meaning such as "the compilation settings can be different for every target; they are separable".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no hard feelings about this. My line of reasoning was: Currently we don't offer the possibility to separate the compilation but rather we kind of force the separation when Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE=ON, as the source files and targets that use kokkos will require to have properties set by cmake manually or might not compile

Copy link
Member

Choose a reason for hiding this comment

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

I understand, but most people will read this as "it is the way to do libraries." Separable is slightly better.

@JBludau
Copy link
Contributor Author

JBludau commented Jul 31, 2024

kokkoc_compilation in its current form only sets the launch compiler. Hence, it's not useful with Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE=ON. I think we should just document the current behavior for now.

I agree with your assessment, I ended at the same places in the cmake files.
Nevertheless, I would vote for trying to support this feature, even if the user needs to annotate both the source and target. HIP seems to recommend the use of the CMake language feature (see first Attention box)

@masterleinad
Copy link
Contributor

Nevertheless, I would vote for trying to support this feature, even if the user needs to annotate both the source and target.

Sure. I was just suggesting a more incremental approach. First, document the current behavior of kokkos_compilation, then try to accommodate it when using Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE (or even define a different function since the behavior needed is very different), and finally document it.

@masterleinad
Copy link
Contributor

HIP seems to recommend the use of the CMake language feature (see first Attention box)

Kokkos just works better when not enabling HIP (or CUDA) as a feature, though.

Co-authored-by: Cédric Chevalier <cedric.chevalier019@proton.me>
* * ``Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE``
* Enables Kokkos behaving like a CMake language, see `Separate Compilation <building.html#separate-compilation-via-cmake-language>`_.
* ``OFF``

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
* * ``Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE``
* Use native CMake language support
* Analogous to CUDA/HIP as CMake language
* ``OFF``

Copy link
Contributor

Choose a reason for hiding this comment

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

This must be three lines not four for rendering the table correctly.

## Separate Compilation via CMake Language

Kokkos supports separating the compilation of source files using Kokkos from others. This is controlled similarly to a CMake language. The feature requires Kokkos to be compiled with the keyword `Kokkos_ENABLE_COMPILE_AS_CMAKE_LANGUAGE=ON`. The availability of the feature can be requested via `find_package(Kokkos COMPONENTS separable_compilation)`.
The `kokkos_compilation` CMake function marks files in the application's source that contain Kokkos code to be compiled with the correct compiler and flags.
Copy link
Contributor

@nilsvu nilsvu Jan 3, 2025

Choose a reason for hiding this comment

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

Could you clarify here how kokkos_compilation interacts with linking the Kokkos::kokkos target?

Specifically, I have a library target with GPU kernels that should be compiled with nvcc, and an executable target that should be compiled with plain gcc (because it's a pain to keep the whole code compatible with nvcc). Can I link the executable target with Kokkos::kokkos (e.g. so I can call Kokkos::initialize) without triggering it to be compiled with nvcc? I want to do this:

# Kernels library (compile with nvcc)
add_library(mykernels ...)
# Need to link kokkos for kernels obviously
target_link_libraries(mykernels PRIVATE Kokkos::kokkos)
# Compile kernels with nvcc
kokkos_compilation(TARGET mykernels)

# Executable (compile with gcc)
add_executable(myexec main.cpp)
# Use some kokkos functions and launch kernels in executable
target_link_libraries(myexec PRIVATE mykernels Kokkos::kokkos)
# Will myexec compile with gcc or nvcc now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In it's current state kokkos_compilation is not working as it would imply.

To get what you want to work you would need to separate your Kokkos dependent files into a subdirectory, create a library, wrap our interfaces and then link this library to Kokkos with PRIVATE. An example can be found here.

But you can not call Kokkos::initialize directly in your main with this. As soon as you use anything from the Kokkos interface you will need to compile with the compiler Kokkos was compiled with. A lot of the code in Kokkos is templated and can only be fully instantiated by the compiler if it know how you use it in your code.
Does this clarify it a bit?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification! So does this mean only targets that I have marked with kokkos_compilation should link Kokkos::kokkos? That would be helpful to add to the docs I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kokkos_compilation as a function is only useful if you do find_package(Kokkos COMPONENTS separable_compilation). But in that case: yes.

But I guess the question if you want to separate the compilation mainly depends on what classes you use in the interfaces of your functions (which translates to INTERFACE?PUBLIC vs. PRIVATE in target_link_libraries).

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.

6 participants