[Mirror] Move CMake logic from snapshotted Kokkos package to Trilinos

[csiefer2]

Automated mirror of upstream PR trilinos#15113 @trilinos/kokkos

Motivation

Move CMake logic from the snapshotted Kokkos package to Trilinos. Hopefully this will make it less awkward to deal with Trilinos specifc deviations from Kokkos defaults. Add a missing optional dependency on quadmath to Kokkos.

Testing

In a serial build I saw no difference in the CMakeCache.txt before and after the change. Let's see how badly the AT will fail...

[github-actions]

CDash for AT1 results [Only accessible from Sandia networks]
CDash for AT2 results [Currently only accessible from Sandia networks]

[gemini-code-assist]

Code Review

This pull request refactors Kokkos configuration within Trilinos by moving several settings from packages/kokkos/cmake/kokkos_configure_trilinos.cmake to a new top-level file cmake/TrilinosKokkosSettings.cmake. It also adds quadmath as an optional dependency. Feedback highlights several issues with the new settings file: a missing else block for OpenMP configuration, potential syntax errors due to unquoted variable expansions in if statements, and an assert_defined call that will likely fail because it runs before the variables are initialized by TriBITS. Additionally, a setting to disable external GTest was lost during the refactor and should be restored if still required.

[gemini-code-assist]

The else block that sets Kokkos_ENABLE_OPENMP to OFF is missing here, which is inconsistent with the CUDA and HPX blocks below. This will cause the assert_defined call on line 26 to fail if Trilinos_ENABLE_OpenMP is not set. Additionally, the variable expansion in the if condition should be quoted to prevent syntax errors if the variable is undefined or empty.

if(NOT "${Trilinos_ENABLE_OpenMP}" STREQUAL "")
  set(Kokkos_ENABLE_OPENMP ${Trilinos_ENABLE_OpenMP} CACHE BOOL "Whether to build OpenMP backend" FORCE)
else()
  set(Kokkos_ENABLE_OPENMP OFF CACHE BOOL "Whether to build OpenMP backend" FORCE)
endif()

[gemini-code-assist]

This assert_defined call will likely cause the configuration to fail because variables such as Kokkos_ENABLE_THREADS, Kokkos_ENABLE_HIP, Kokkos_ENABLE_SYCL, and Kokkos_ENABLE_OPENACC are not initialized in this script. Since this file is included before TRIBITS_PROJECT() in the top-level CMakeLists.txt, these variables will be undefined unless explicitly provided by the user. In the original implementation, this check occurred within the Kokkos package configuration after TriBITS had initialized these variables.

[gemini-code-assist]

Variable expansions in if conditions should be quoted to prevent syntax errors if the variable is undefined or empty.

if(NOT "${TPL_ENABLE_CUDA}" STREQUAL "")

[gemini-code-assist]

Variable expansions in if conditions should be quoted to prevent syntax errors if the variable is undefined or empty.

if(NOT "${TPL_ENABLE_HPX}" STREQUAL "")

[gemini-code-assist]

The logic set(CMAKE_DISABLE_FIND_PACKAGE_GTest ON) was removed during this refactor and not relocated to TrilinosKokkosSettings.cmake. If this setting is still necessary for Trilinos CI to avoid conflicts with external GTest installations, it should be restored or moved to the new settings file.

[cgcgcg]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the Kokkos configuration logic within Trilinos by introducing a new CMake file, TrilinosKokkosSettings.cmake, to centralize backend enablement settings. However, the current implementation introduces critical issues: the logic for Kokkos_ENABLE_ATOMICS_BYPASS is evaluated too early, potentially causing race conditions in threaded builds, and the forced disabling of CUDA and HPX backends prevents proper auto-detection. Additionally, the removal of GTest configuration and backend variable assertions from the original package configuration file requires further review to ensure build stability.

[gemini-code-assist]

The logic for Kokkos_ENABLE_ATOMICS_BYPASS is evaluated too early here. Since it runs before TRIBITS_PROJECT(), it only sees variables set explicitly by the user. If a backend (like OpenMP or CUDA) is enabled by TriBITS defaults or through package dependencies, this check will incorrectly conclude that only Serial is enabled and force Kokkos_ENABLE_ATOMICS_BYPASS to ON. This can lead to race conditions in threaded builds. Furthermore, variables like Kokkos_ENABLE_THREADS, Kokkos_ENABLE_HIP, etc., are checked here but not yet defined by Kokkos's internal logic. This logic should remain in a location that is processed after all backend enablement decisions are finalized, such as within the Kokkos package configuration.

[gemini-code-assist]

The else block forcing Kokkos_ENABLE_CUDA to OFF when TPL_ENABLE_CUDA is empty is problematic. Since this file is included before TRIBITS_PROJECT(), TPL_ENABLE_CUDA will be empty unless explicitly set by the user via the command line. Forcing it to OFF here effectively disables TriBITS's ability to auto-detect CUDA or enable it via other dependency logic. It is also inconsistent with the OpenMP logic at line 7 which correctly avoids the else block.

endif()

[gemini-code-assist]

Similar to the CUDA logic above, forcing Kokkos_ENABLE_HPX to OFF when TPL_ENABLE_HPX is empty prevents auto-detection and is inconsistent with the OpenMP handling.

endif()