From 92d557ab19258d139d090b6e8da30e140baec19d Mon Sep 17 00:00:00 2001 From: Cedric Chevalier Date: Wed, 27 Mar 2024 18:36:20 +0100 Subject: [PATCH 1/5] Test include KokkosComm header files using <> Tests use the library like an external one, so they shoud use <> instead of "" to include header files. --- perf_tests/test_2dhalo.cpp | 2 +- perf_tests/test_main.cpp | 2 +- perf_tests/test_sendrecv.cpp | 2 +- perf_tests/test_utils.hpp | 2 +- unit_tests/test_isendrecv.cpp | 2 +- unit_tests/test_main.cpp | 2 +- unit_tests/test_reduce.cpp | 2 +- unit_tests/test_sendrecv.cpp | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/perf_tests/test_2dhalo.cpp b/perf_tests/test_2dhalo.cpp index dd59c2b6..a0671ca8 100644 --- a/perf_tests/test_2dhalo.cpp +++ b/perf_tests/test_2dhalo.cpp @@ -16,7 +16,7 @@ #include "test_utils.hpp" -#include "KokkosComm.hpp" +#include #include diff --git a/perf_tests/test_main.cpp b/perf_tests/test_main.cpp index 4898d6e4..9454ce87 100644 --- a/perf_tests/test_main.cpp +++ b/perf_tests/test_main.cpp @@ -14,7 +14,7 @@ // //@HEADER -#include "KokkosComm_include_mpi.hpp" +#include #include #include diff --git a/perf_tests/test_sendrecv.cpp b/perf_tests/test_sendrecv.cpp index ead1c7aa..36db111c 100644 --- a/perf_tests/test_sendrecv.cpp +++ b/perf_tests/test_sendrecv.cpp @@ -16,7 +16,7 @@ #include "test_utils.hpp" -#include "KokkosComm.hpp" +#include template void send_recv(benchmark::State &, MPI_Comm comm, const Space &space, int rank, diff --git a/perf_tests/test_utils.hpp b/perf_tests/test_utils.hpp index ccb9afed..7ee62686 100644 --- a/perf_tests/test_utils.hpp +++ b/perf_tests/test_utils.hpp @@ -20,7 +20,7 @@ #include -#include "KokkosComm_include_mpi.hpp" +#include // F is a function that takes (state, MPI_Comm, args...) template diff --git a/unit_tests/test_isendrecv.cpp b/unit_tests/test_isendrecv.cpp index 709acf0a..73636580 100644 --- a/unit_tests/test_isendrecv.cpp +++ b/unit_tests/test_isendrecv.cpp @@ -31,7 +31,7 @@ using std::MDSPAN_PREFIX() mdspan; #include -#include "KokkosComm.hpp" +#include template class IsendRecv : public testing::Test { diff --git a/unit_tests/test_main.cpp b/unit_tests/test_main.cpp index 13161754..14890bb9 100644 --- a/unit_tests/test_main.cpp +++ b/unit_tests/test_main.cpp @@ -20,7 +20,7 @@ #include #include -#include "KokkosComm_include_mpi.hpp" +#include int main(int argc, char *argv[]) { #if 0 diff --git a/unit_tests/test_reduce.cpp b/unit_tests/test_reduce.cpp index fd3b8499..79242f3a 100644 --- a/unit_tests/test_reduce.cpp +++ b/unit_tests/test_reduce.cpp @@ -16,7 +16,7 @@ #include -#include "KokkosComm.hpp" +#include template class Reduce : public testing::Test { diff --git a/unit_tests/test_sendrecv.cpp b/unit_tests/test_sendrecv.cpp index 358026ff..00ab6d53 100644 --- a/unit_tests/test_sendrecv.cpp +++ b/unit_tests/test_sendrecv.cpp @@ -16,7 +16,7 @@ #include -#include "KokkosComm.hpp" +#include template class SendRecv : public testing::Test { From 6197bffcba6f28421051983b80170a38bd5ec651 Mon Sep 17 00:00:00 2001 From: Cedric Chevalier Date: Wed, 27 Mar 2024 20:10:01 +0100 Subject: [PATCH 2/5] Cleaning up include files Only use #include "" toward subdirectories. --- src/KokkosComm.hpp | 12 +++++++++--- src/KokkosComm_collective.hpp | 2 -- src/KokkosComm_pack_traits.hpp | 6 +++--- src/KokkosComm_request.hpp | 2 +- src/KokkosComm_traits.hpp | 5 ++--- src/impl/KokkosComm_isend.hpp | 4 ---- src/impl/KokkosComm_recv.hpp | 3 --- src/impl/KokkosComm_reduce.hpp | 3 --- src/impl/KokkosComm_send.hpp | 2 -- 9 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/KokkosComm.hpp b/src/KokkosComm.hpp index 9392f132..e5dae0c2 100644 --- a/src/KokkosComm.hpp +++ b/src/KokkosComm.hpp @@ -16,14 +16,20 @@ #pragma once -#include "KokkosComm_collective.hpp" #include "KokkosComm_version.hpp" + +#include + +#include "KokkosComm_pack_traits.hpp" +#include "KokkosComm_traits.hpp" + +#include "KokkosComm_request.hpp" +#include "KokkosComm_collective.hpp" + #include "impl/KokkosComm_isend.hpp" #include "impl/KokkosComm_recv.hpp" #include "impl/KokkosComm_send.hpp" -#include - namespace KokkosComm { template diff --git a/src/KokkosComm_collective.hpp b/src/KokkosComm_collective.hpp index 24120496..6b4149bc 100644 --- a/src/KokkosComm_collective.hpp +++ b/src/KokkosComm_collective.hpp @@ -18,8 +18,6 @@ #include "impl/KokkosComm_reduce.hpp" -#include - namespace KokkosComm { template diff --git a/src/KokkosComm_pack_traits.hpp b/src/KokkosComm_pack_traits.hpp index 177e561e..ea36934e 100644 --- a/src/KokkosComm_pack_traits.hpp +++ b/src/KokkosComm_pack_traits.hpp @@ -18,9 +18,9 @@ #include "KokkosComm_traits.hpp" -#include "KokkosComm_concepts.hpp" -#include "KokkosComm_mdspan.hpp" -#include "KokkosComm_packer.hpp" +#include "impl/KokkosComm_concepts.hpp" +#include "impl/KokkosComm_mdspan.hpp" +#include "impl/KokkosComm_packer.hpp" /*! \brief Defines a common interface for packing and unpacking Kokkos::View-like types \file KokkosComm_traits.hpp diff --git a/src/KokkosComm_request.hpp b/src/KokkosComm_request.hpp index ddd59181..4c552beb 100644 --- a/src/KokkosComm_request.hpp +++ b/src/KokkosComm_request.hpp @@ -18,7 +18,7 @@ #include -#include "KokkosComm_include_mpi.hpp" +#include "impl/KokkosComm_include_mpi.hpp" // MPI_REQUEST_NULL, MPI_STATUS_IGNORE namespace KokkosComm { diff --git a/src/KokkosComm_traits.hpp b/src/KokkosComm_traits.hpp index 565ff631..768f6604 100644 --- a/src/KokkosComm_traits.hpp +++ b/src/KokkosComm_traits.hpp @@ -20,9 +20,8 @@ #pragma once -#include "KokkosComm_mdspan.hpp" - -#include "KokkosComm_concepts.hpp" +#include "impl/KokkosComm_mdspan.hpp" +#include "impl/KokkosComm_concepts.hpp" namespace KokkosComm { diff --git a/src/impl/KokkosComm_isend.hpp b/src/impl/KokkosComm_isend.hpp index e53b6e08..010c94c9 100644 --- a/src/impl/KokkosComm_isend.hpp +++ b/src/impl/KokkosComm_isend.hpp @@ -20,10 +20,6 @@ #include -#include "KokkosComm_pack_traits.hpp" -#include "KokkosComm_request.hpp" -#include "KokkosComm_traits.hpp" - // impl #include "KokkosComm_include_mpi.hpp" diff --git a/src/impl/KokkosComm_recv.hpp b/src/impl/KokkosComm_recv.hpp index d07b82e0..2b7f900e 100644 --- a/src/impl/KokkosComm_recv.hpp +++ b/src/impl/KokkosComm_recv.hpp @@ -18,9 +18,6 @@ #include -#include "KokkosComm_pack_traits.hpp" -#include "KokkosComm_traits.hpp" - // impl #include "KokkosComm_include_mpi.hpp" diff --git a/src/impl/KokkosComm_reduce.hpp b/src/impl/KokkosComm_reduce.hpp index 53be01fe..fbd5a546 100644 --- a/src/impl/KokkosComm_reduce.hpp +++ b/src/impl/KokkosComm_reduce.hpp @@ -18,9 +18,6 @@ #include -#include "KokkosComm_pack_traits.hpp" -#include "KokkosComm_traits.hpp" - // impl #include "KokkosComm_include_mpi.hpp" #include "KokkosComm_types.hpp" diff --git a/src/impl/KokkosComm_send.hpp b/src/impl/KokkosComm_send.hpp index 11848ae7..ce0c4da1 100644 --- a/src/impl/KokkosComm_send.hpp +++ b/src/impl/KokkosComm_send.hpp @@ -18,8 +18,6 @@ #include -#include "KokkosComm_pack_traits.hpp" - // impl #include "KokkosComm_include_mpi.hpp" From 622d591d52731f829caeb55dd678f943853a1516 Mon Sep 17 00:00:00 2001 From: Cedric Chevalier Date: Thu, 28 Mar 2024 10:52:17 +0100 Subject: [PATCH 3/5] Remove "impl" from header search path This shows that tests rely on "impl" details. --- CMakeLists.txt | 3 --- perf_tests/test_main.cpp | 2 +- perf_tests/test_utils.hpp | 2 +- src/CMakeLists.txt | 14 +++++++------- unit_tests/test_main.cpp | 2 +- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3786e76b..58b4b700 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -53,9 +53,6 @@ endif() include(cmake/flags.cmake) add_subdirectory(src) -kokkoscomm_add_cxx_flags(TARGET KokkosComm INTERFACE) -message(STATUS ${KOKKOSCOMM_PUBLIC_HEADERS}) -set_target_properties(KokkosComm PROPERTIES PUBLIC_HEADER "${KOKKOSCOMM_PUBLIC_HEADERS}") ## Version config file set(KOKKOSCOMM_VERSION_MAJOR ${CMAKE_PROJECT_VERSION_MAJOR} CACHE STRING "" FORCE) diff --git a/perf_tests/test_main.cpp b/perf_tests/test_main.cpp index 9454ce87..e649426f 100644 --- a/perf_tests/test_main.cpp +++ b/perf_tests/test_main.cpp @@ -14,7 +14,7 @@ // //@HEADER -#include +#include #include #include diff --git a/perf_tests/test_utils.hpp b/perf_tests/test_utils.hpp index 7ee62686..bc1e5506 100644 --- a/perf_tests/test_utils.hpp +++ b/perf_tests/test_utils.hpp @@ -20,7 +20,7 @@ #include -#include +#include // F is a function that takes (state, MPI_Comm, args...) template diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 26a7e767..3d1fe5b9 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -2,17 +2,17 @@ add_library(KokkosComm INTERFACE) target_include_directories(KokkosComm INTERFACE $ + $ ) -target_include_directories(KokkosComm INTERFACE - $ -) -target_include_directories(KokkosComm INTERFACE - $ -) + target_link_libraries(KokkosComm INTERFACE MPI::MPI_CXX Kokkos::kokkos) if(KOKKOSCOMM_USE_KOKKOS_MDSPAN) target_link_libraries(KokkosComm INTERFACE mdspan) endif() +kokkoscomm_add_cxx_flags(TARGET KokkosComm INTERFACE) + +# FIXME Not a good idea, we should explicitely list the headers file(GLOB_RECURSE KOKKOSCOMM_PUBLIC_HEADERS ${CMAKE_CURRENT_LIST_DIR}/*.hpp) -set(KOKKOSCOMM_PUBLIC_HEADERS ${KOKKOSCOMM_PUBLIC_HEADERS} PARENT_SCOPE) \ No newline at end of file + +set_target_properties(KokkosComm PROPERTIES PUBLIC_HEADER "${KOKKOSCOMM_PUBLIC_HEADERS}") diff --git a/unit_tests/test_main.cpp b/unit_tests/test_main.cpp index 14890bb9..ab648fab 100644 --- a/unit_tests/test_main.cpp +++ b/unit_tests/test_main.cpp @@ -20,7 +20,7 @@ #include #include -#include +#include int main(int argc, char *argv[]) { #if 0 From 2d96ec98d1bc866c586786f0013932ee60b47a4f Mon Sep 17 00:00:00 2001 From: Cedric Chevalier Date: Wed, 3 Apr 2024 15:24:45 +0200 Subject: [PATCH 4/5] Explicitely list files --- src/CMakeLists.txt | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3d1fe5b9..f78e2cbf 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -10,9 +10,25 @@ if(KOKKOSCOMM_USE_KOKKOS_MDSPAN) target_link_libraries(KokkosComm INTERFACE mdspan) endif() +# FIXME Not sure it is really possible to set BUILD_INTERFACE for header only libraries +# We should ensure that we do not export convenience flags to users kokkoscomm_add_cxx_flags(TARGET KokkosComm INTERFACE) -# FIXME Not a good idea, we should explicitely list the headers -file(GLOB_RECURSE KOKKOSCOMM_PUBLIC_HEADERS ${CMAKE_CURRENT_LIST_DIR}/*.hpp) +set (KokkosCommPublicHeaders + KokkosComm.hpp + KokkosComm_collective.hpp + KokkosComm_pack_traits.hpp + KokkosComm_request.hpp + KokkosComm_traits.hpp + impl/KokkosComm_concepts.hpp + impl/KokkosComm_include_mpi.hpp + impl/KokkosComm_isend.hpp + impl/KokkosComm_mdspan.hpp + impl/KokkosComm_packer.hpp + impl/KokkosComm_recv.hpp + impl/KokkosComm_reduce.hpp + impl/KokkosComm_send.hpp + impl/KokkosComm_types.hpp +) -set_target_properties(KokkosComm PROPERTIES PUBLIC_HEADER "${KOKKOSCOMM_PUBLIC_HEADERS}") +set_target_properties(KokkosComm PROPERTIES PUBLIC_HEADER "${KokkosCommPublicHeaders}") From fb870bb5fd7e3736f3a076c4eed3782e9cc9bb5d Mon Sep 17 00:00:00 2001 From: Cedric Chevalier Date: Wed, 3 Apr 2024 15:31:11 +0200 Subject: [PATCH 5/5] Use GNUInstallDirs --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 58b4b700..54f51c1e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,6 +52,8 @@ endif() include(cmake/flags.cmake) +include (GNUInstallDirs) + add_subdirectory(src) ## Version config file @@ -65,6 +67,7 @@ configure_file( ) target_include_directories(KokkosComm INTERFACE $ + $ ) ## Package config file