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

Compilation with Kokkos develop broken after Kokkos PR 7394 #275

Open
maartenarnst opened this issue Oct 6, 2024 · 2 comments
Open

Compilation with Kokkos develop broken after Kokkos PR 7394 #275

maartenarnst opened this issue Oct 6, 2024 · 2 comments

Comments

@maartenarnst
Copy link
Contributor

Compilation of kokkos-tools with kokkos develop is broken after

The reason seems to be that the kokkos PR moved the function int_for_synchronization_reason from Kokkos_Profiling.hpp to Kokkos_Profiling_Interface.hpp. However, this file Kokkos_Profiling_Interface.hpp is also in kokkos-tools/profiling/all/impl. And so because of the kokkos PR, the two files are no longer in sync.

When we now compile kokkos-tools with kokkos develop, we get a compilation error saying that the compiler can't find int_for_synchronization_reason. The compilation error arises when compiling the kokkos-tools tests. That's indeed where both kokkos-tools and kokkos are needed, and the two versions of the same header being out of sync can cause a problem.

In file included from /opt/kokkos/include/impl/Kokkos_Tools.hpp:24,
                 from /opt/kokkos/include/Kokkos_HostSpace.hpp:37,
                 from /opt/kokkos/include/OpenMP/Kokkos_OpenMP.hpp:30,
                 from /opt/kokkos/include/decl/Kokkos_Declare_OPENMP.hpp:21,
                 from /opt/kokkos/include/KokkosCore_Config_DeclareBackend.hpp:22,
                 from /opt/kokkos/include/Kokkos_Core.hpp:45,
                 from /workspaces/kokkos/kokkos-tools/tests/UnitTestMain.cpp:3:
/opt/kokkos/include/impl/Kokkos_Profiling.hpp: In function 'void Kokkos::Tools::Experimental::Impl::profile_fence_event(const std::string&, Kokkos::Tools::Experimental::SpecialSynchronizationCases, const FencingFunctor&)':
/opt/kokkos/include/impl/Kokkos_Profiling.hpp:211:39: error: there are no arguments to 'int_for_synchronization_reason' that depend on a template parameter, so a declaration of 'int_for_synchronization_reason' must be available [-fpermissive]
  211 |       name, device_id_root<Space>() + int_for_synchronization_reason(reason),

The reason that the kokkos-tools version of Kokkos_Profiling_Interface.hpp has precedence over the kokkos version, and we thus get the error here, seems be that kokkos-tools uses hard include_directories rather than a CMake target:

  • kokkos-tools/CMakeLists.txt

    Lines 119 to 126 in a9453b2

    # make Kokkos profiling interface available for native profilers
    include_directories(${CMAKE_CURRENT_SOURCE_DIR}/profiling/all)
    set(COMMON_HEADERS_PATH ${CMAKE_CURRENT_BINARY_DIR}/common)
    include_directories(${COMMON_HEADERS_PATH})
    # Allow all tools to include any file.
    include_directories(${CMAKE_CURRENT_SOURCE_DIR}/common)

Unfortunately, simply updating kokkos-tools/profiling/all/impl/Kokkos_Profiling_Interface.hpp does not seem to be a good solution. When we update this file, compilation works with kokkos develop. However, when we update this file and then compile with kokkos 4.4.1, compilation fails because there are now two definitions of int_for_synchronization_reason ("ambiguous ...").

A "quick fix" would be to update kokkos-tools/profiling/all/impl/Kokkos_Profiling_Interface.hpp by adding the function protected by "#if KOKKOS_VERSION >= 40499". However, this may not be a good fix, because the versions of this file in Kokkos and kokkos-tools would then be indefinitely out of sync.

@csiefer2 @crtrott @dalg24 @romintomasetti

@csiefer2
Copy link
Contributor

csiefer2 commented Oct 17, 2024

@maartenarnst The patch for that is included in the PR @vlkale linked to.

I think the bigger question you raise though is "What version of Kokkos is kokkos-tools supposed to work with?" Since we don't release kokkos-tools when we release Kokkos, this is a bit of an issue.

@maartenarnst
Copy link
Contributor Author

Hi @csiefer2. Thanks for the work on #276! It's probably best to keep this issue open until #276 is merged.

Indeed! I agree, there's a bigger issue about "what version of Kokkos is kokkos-tools supposed to work with?" I'll open a new issue to raise that bigger issue.

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

No branches or pull requests

2 participants