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.

Sign up for GitHub

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

Jump to bottom

Kokkos: Don't let Kokkos set CMAKE_CXX_FLAGS #12572

Closed
wants to merge 3 commits into from
Closed

Kokkos: Don't let Kokkos set CMAKE_CXX_FLAGS #12572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cef2a7
Don't let Kokkos set CMAKE_CXX_FLAGS
masterleinad Dec 1, 2023
537022c
Still add KOKKOS_CUDA_FE_OPTIONS to KOKKOS_ALL_COMPILE_OPTIONS
masterleinad Jan 24, 2024
84fdfd5
Cherry-pick kokkos/kokkos#6510
masterleinad Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions packages/kokkos/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ ENDIF()
# subpackages

## This restores the old behavior of ProjectCompilerPostConfig.cmake
# It sets the CMAKE_CXX_FLAGS globally to those used by Kokkos
# We must do this before KOKKOS_PACKAGE_DECL
IF (KOKKOS_HAS_TRILINOS)
# Overwrite the old flags at the top-level
Expand Down Expand Up @@ -280,21 +279,13 @@ IF (KOKKOS_HAS_TRILINOS)
SET(KOKKOSCORE_XCOMPILER_OPTIONS "${KOKKOSCORE_XCOMPILER_OPTIONS} -Xcompiler ${XCOMP_FLAG}")
LIST(APPEND KOKKOS_ALL_COMPILE_OPTIONS -Xcompiler ${XCOMP_FLAG})
ENDFOREACH()
SET(KOKKOSCORE_CXX_FLAGS "${KOKKOSCORE_COMPILE_OPTIONS} ${KOKKOSCORE_XCOMPILER_OPTIONS}")
IF (KOKKOS_ENABLE_CUDA)
STRING(REPLACE ";" " " KOKKOSCORE_CUDA_OPTIONS "${KOKKOS_CUDA_OPTIONS}")
FOREACH(CUDAFE_FLAG ${KOKKOS_CUDAFE_OPTIONS})
SET(KOKKOSCORE_CUDAFE_OPTIONS "${KOKKOSCORE_CUDAFE_OPTIONS} -Xcudafe ${CUDAFE_FLAG}")
LIST(APPEND KOKKOS_ALL_COMPILE_OPTIONS -Xcudafe ${CUDAFE_FLAG})
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this block of code seems to have the side effect that the cuda fe flags are no longer passed to Trilinos through KOKKOS_ALL_COMPILE_OPTIONS. May be good to doublecheck if that’s intended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. For now, it's better not to modify KOKKOS_ALL_COMPILE_OPTIONS. I pushed a corresponding commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK!

Copy link
Member

@bartlettroscoe bartlettroscoe Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The handling of KOKKOS_ALL_COMPILE_OPTIONS is a bit confusing to me so I don't understand the logic here. From what I can tell, these options are written to a configured file KokkosTrilinosConfig.cmake and added to the Kokkos::kokkos target using target_compiler_options(). Then that file gets read in and written to the end of the generated file KokkosConfig.cmake which gets installed. Why not just directly add those compile options to the individual Kokkos libraries using target_compiler_options()? Then those options would be carried along with the direct Kokkos libraries no matter how they are used.

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 agree but KOKKOS_ALL_COMPILE_OPTIONS doesn't cause the issue we are trying to fix here and we wanted to keep the changes here minimal.
Note that all this logic is just for Trilinos. A standalone Kokkos doesn't even go through these lines of code.

With the refactoring in #11779 we should eliminate all the weirdness we had in Kokkos just to make Trilinos work.

ENDFOREACH()
SET(KOKKOSCORE_CXX_FLAGS "${KOKKOSCORE_CXX_FLAGS} ${KOKKOSCORE_CUDA_OPTIONS} ${KOKKOSCORE_CUDAFE_OPTIONS}")
ENDIF()
# Both parent scope and this package
# In ProjectCompilerPostConfig.cmake, we capture the "global" flags Trilinos wants in
# TRILINOS_TOPLEVEL_CXX_FLAGS
SET(CMAKE_CXX_FLAGS "${TRILINOS_TOPLEVEL_CXX_FLAGS} ${KOKKOSCORE_CXX_FLAGS}" PARENT_SCOPE)
SET(CMAKE_CXX_FLAGS "${TRILINOS_TOPLEVEL_CXX_FLAGS} ${KOKKOSCORE_CXX_FLAGS}")
#CMAKE_CXX_FLAGS will get added to Kokkos and Kokkos dependencies automatically here
#These flags get set up in KOKKOS_PACKAGE_DECL, which means they
#must be configured before KOKKOS_PACKAGE_DECL
SET(KOKKOS_ALL_COMPILE_OPTIONS
Expand Down
8 changes: 4 additions & 4 deletions packages/kokkos/simd/src/Kokkos_SIMD_Scalar.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,31 +224,31 @@ template <typename T>
using data_type = std::conditional_t<std::is_floating_point_v<T>, T, double>;
return Experimental::simd<data_type, Experimental::simd_abi::scalar>(
Kokkos::floor(static_cast<data_type>(a[0])));
};
}

template <typename T>
[[nodiscard]] KOKKOS_FORCEINLINE_FUNCTION auto ceil(
Experimental::simd<T, Experimental::simd_abi::scalar> const& a) {
using data_type = std::conditional_t<std::is_floating_point_v<T>, T, double>;
return Experimental::simd<data_type, Experimental::simd_abi::scalar>(
Kokkos::ceil(static_cast<data_type>(a[0])));
};
}

template <typename T>
[[nodiscard]] KOKKOS_FORCEINLINE_FUNCTION auto round(
Experimental::simd<T, Experimental::simd_abi::scalar> const& a) {
using data_type = std::conditional_t<std::is_floating_point_v<T>, T, double>;
return Experimental::simd<data_type, Experimental::simd_abi::scalar>(
Experimental::round_half_to_nearest_even(static_cast<data_type>(a[0])));
};
}

template <typename T>
[[nodiscard]] KOKKOS_FORCEINLINE_FUNCTION auto trunc(
Experimental::simd<T, Experimental::simd_abi::scalar> const& a) {
using data_type = std::conditional_t<std::is_floating_point_v<T>, T, double>;
return Experimental::simd<data_type, Experimental::simd_abi::scalar>(
Kokkos::trunc(static_cast<data_type>(a[0])));
};
}

template <class T>
[[nodiscard]] KOKKOS_FORCEINLINE_FUNCTION
Expand Down
Loading