From ae979c677db6600448d902033316fb3762e28f96 Mon Sep 17 00:00:00 2001 From: nscipione Date: Thu, 26 Sep 2024 16:44:54 +0100 Subject: [PATCH 1/8] Update mklgpu support to mkl preparing for oneapi 2025.0 release Update how dft mklgpu backend include header files from MKL and how cmake add them to compilation. This is done to avoid name conflicts between interface and library backends. Force compiler to look for some header following the "" and <> inclusions rules accordingly with C++ Core Guideline SF.12 --- src/dft/backends/mklgpu/CMakeLists.txt | 8 +++++--- src/dft/backends/mklgpu/backward.cpp | 6 +++++- src/dft/backends/mklgpu/commit.cpp | 6 +++++- src/dft/backends/mklgpu/forward.cpp | 6 +++++- src/dft/backends/mklgpu/mklgpu_helpers.hpp | 6 +++++- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/dft/backends/mklgpu/CMakeLists.txt b/src/dft/backends/mklgpu/CMakeLists.txt index 7e88a23d9..323f84499 100644 --- a/src/dft/backends/mklgpu/CMakeLists.txt +++ b/src/dft/backends/mklgpu/CMakeLists.txt @@ -32,12 +32,14 @@ add_library(${LIB_OBJ} OBJECT ) add_dependencies(onemkl_backend_libs_dft ${LIB_NAME}) -target_include_directories(${LIB_OBJ} - PUBLIC ${ONEMKL_INTERFACE_INCLUDE_DIRS} -) target_include_directories(${LIB_NAME} PUBLIC ${ONEMKL_INTERFACE_INCLUDE_DIRS} ) + +target_compile_options(${LIB_OBJ} + BEFORE PRIVATE -iquote $ +) + target_include_directories(${LIB_OBJ} PRIVATE ${PROJECT_SOURCE_DIR}/src ${CMAKE_BINARY_DIR}/bin diff --git a/src/dft/backends/mklgpu/backward.cpp b/src/dft/backends/mklgpu/backward.cpp index 6c4896c66..f6dc697fb 100644 --- a/src/dft/backends/mklgpu/backward.cpp +++ b/src/dft/backends/mklgpu/backward.cpp @@ -31,7 +31,11 @@ #include "mklgpu_helpers.hpp" // MKLGPU header -#include "oneapi/mkl/dfti.hpp" +#if INTEL_MKL_VERSION < 20250000 +#include +#else +#include +#endif namespace oneapi::mkl::dft::mklgpu { namespace detail { diff --git a/src/dft/backends/mklgpu/commit.cpp b/src/dft/backends/mklgpu/commit.cpp index d3a3f1cd6..28e78620f 100644 --- a/src/dft/backends/mklgpu/commit.cpp +++ b/src/dft/backends/mklgpu/commit.cpp @@ -36,7 +36,11 @@ #include "../stride_helper.hpp" // MKLGPU header -#include "oneapi/mkl/dfti.hpp" +#if INTEL_MKL_VERSION < 20250000 +#include +#else +#include +#endif // MKL 2024.1 deprecates input/output strides. #include "mkl_version.h" diff --git a/src/dft/backends/mklgpu/forward.cpp b/src/dft/backends/mklgpu/forward.cpp index 39da42e45..70f61e066 100644 --- a/src/dft/backends/mklgpu/forward.cpp +++ b/src/dft/backends/mklgpu/forward.cpp @@ -32,7 +32,11 @@ #include "mklgpu_helpers.hpp" // MKLGPU header -#include "oneapi/mkl/dfti.hpp" +#if INTEL_MKL_VERSION < 20250000 +#include +#else +#include +#endif /** Note that in this file, the Intel oneMKL-GPU library's interface mirrors the diff --git a/src/dft/backends/mklgpu/mklgpu_helpers.hpp b/src/dft/backends/mklgpu/mklgpu_helpers.hpp index 6813297ea..dbdb68171 100644 --- a/src/dft/backends/mklgpu/mklgpu_helpers.hpp +++ b/src/dft/backends/mklgpu/mklgpu_helpers.hpp @@ -24,7 +24,11 @@ #include "oneapi/mkl/dft/detail/types_impl.hpp" // MKLGPU header -#include "oneapi/mkl/dfti.hpp" +#if INTEL_MKL_VERSION < 20250000 +#include +#else +#include +#endif namespace oneapi { namespace mkl { From 562f1dbdc763fd65107eb3247b30fdfc053189b0 Mon Sep 17 00:00:00 2001 From: nscipione Date: Tue, 1 Oct 2024 09:59:41 +0100 Subject: [PATCH 2/8] Add comment to explain why a flag is needed --- src/dft/backends/mklgpu/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dft/backends/mklgpu/CMakeLists.txt b/src/dft/backends/mklgpu/CMakeLists.txt index 323f84499..d90935dbf 100644 --- a/src/dft/backends/mklgpu/CMakeLists.txt +++ b/src/dft/backends/mklgpu/CMakeLists.txt @@ -36,6 +36,9 @@ target_include_directories(${LIB_NAME} PUBLIC ${ONEMKL_INTERFACE_INCLUDE_DIRS} ) +# Due to using the same file name for different headers in this library and in +# MKL Intel library, we force the compiler to follow C++ Core Guideline SF.12 +# using the flag "-iquote" to avoid conflicts and find the correct header. target_compile_options(${LIB_OBJ} BEFORE PRIVATE -iquote $ ) From 015fb534eef2964af897aa9aa6401970db9d5bcf Mon Sep 17 00:00:00 2001 From: nscipione Date: Thu, 3 Oct 2024 09:44:12 +0100 Subject: [PATCH 3/8] Add header file to properly check MKL version at compile-time --- src/dft/backends/mklgpu/backward.cpp | 1 + src/dft/backends/mklgpu/commit.cpp | 2 +- src/dft/backends/mklgpu/forward.cpp | 1 + src/dft/backends/mklgpu/mklgpu_helpers.hpp | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dft/backends/mklgpu/backward.cpp b/src/dft/backends/mklgpu/backward.cpp index f6dc697fb..4899ed3e7 100644 --- a/src/dft/backends/mklgpu/backward.cpp +++ b/src/dft/backends/mklgpu/backward.cpp @@ -30,6 +30,7 @@ #include "mklgpu_helpers.hpp" +#include "mkl_version.h" // MKLGPU header #if INTEL_MKL_VERSION < 20250000 #include diff --git a/src/dft/backends/mklgpu/commit.cpp b/src/dft/backends/mklgpu/commit.cpp index 28e78620f..bae1eb69f 100644 --- a/src/dft/backends/mklgpu/commit.cpp +++ b/src/dft/backends/mklgpu/commit.cpp @@ -35,6 +35,7 @@ #include "dft/backends/mklgpu/mklgpu_helpers.hpp" #include "../stride_helper.hpp" +#include "mkl_version.h" // MKLGPU header #if INTEL_MKL_VERSION < 20250000 #include @@ -43,7 +44,6 @@ #endif // MKL 2024.1 deprecates input/output strides. -#include "mkl_version.h" #if INTEL_MKL_VERSION < 20240001 #error MKLGPU requires oneMKL 2024.1 or later #endif diff --git a/src/dft/backends/mklgpu/forward.cpp b/src/dft/backends/mklgpu/forward.cpp index 70f61e066..9dbffa081 100644 --- a/src/dft/backends/mklgpu/forward.cpp +++ b/src/dft/backends/mklgpu/forward.cpp @@ -31,6 +31,7 @@ #include "mklgpu_helpers.hpp" +#include "mkl_version.h" // MKLGPU header #if INTEL_MKL_VERSION < 20250000 #include diff --git a/src/dft/backends/mklgpu/mklgpu_helpers.hpp b/src/dft/backends/mklgpu/mklgpu_helpers.hpp index dbdb68171..6d41105a7 100644 --- a/src/dft/backends/mklgpu/mklgpu_helpers.hpp +++ b/src/dft/backends/mklgpu/mklgpu_helpers.hpp @@ -23,6 +23,7 @@ #include "oneapi/mkl/detail/exceptions.hpp" #include "oneapi/mkl/dft/detail/types_impl.hpp" +#include "mkl_version.h" // MKLGPU header #if INTEL_MKL_VERSION < 20250000 #include From c0c06ce63c30fc6b8c98c8fad06cd3f6becd8381 Mon Sep 17 00:00:00 2001 From: nscipione Date: Thu, 3 Oct 2024 10:16:15 +0100 Subject: [PATCH 4/8] Comment problematic enum for testing purposes --- src/dft/backends/mklgpu/mklgpu_helpers.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dft/backends/mklgpu/mklgpu_helpers.hpp b/src/dft/backends/mklgpu/mklgpu_helpers.hpp index 6d41105a7..710dbaf6f 100644 --- a/src/dft/backends/mklgpu/mklgpu_helpers.hpp +++ b/src/dft/backends/mklgpu/mklgpu_helpers.hpp @@ -69,13 +69,13 @@ inline constexpr dft::config_param to_mklgpu(dft::detail::config_param param) { case iparam::FORWARD_SCALE: return oparam::FORWARD_SCALE; case iparam::NUMBER_OF_TRANSFORMS: return oparam::NUMBER_OF_TRANSFORMS; case iparam::COMPLEX_STORAGE: return oparam::COMPLEX_STORAGE; - case iparam::REAL_STORAGE: return oparam::REAL_STORAGE; + //case iparam::REAL_STORAGE: return oparam::REAL_STORAGE; case iparam::CONJUGATE_EVEN_STORAGE: return oparam::CONJUGATE_EVEN_STORAGE; case iparam::FWD_DISTANCE: return oparam::FWD_DISTANCE; case iparam::BWD_DISTANCE: return oparam::BWD_DISTANCE; case iparam::WORKSPACE: return oparam::WORKSPACE; - case iparam::ORDERING: return oparam::ORDERING; - case iparam::TRANSPOSE: return oparam::TRANSPOSE; + //case iparam::ORDERING: return oparam::ORDERING; + //case iparam::TRANSPOSE: return oparam::TRANSPOSE; case iparam::PACKED_FORMAT: return oparam::PACKED_FORMAT; case iparam::WORKSPACE_PLACEMENT: return oparam::WORKSPACE; // Same as WORKSPACE case iparam::WORKSPACE_EXTERNAL_BYTES: return oparam::WORKSPACE_BYTES; From 0c06b044cbe22804a5618111957cb05e270c9892 Mon Sep 17 00:00:00 2001 From: nscipione Date: Mon, 7 Oct 2024 16:06:09 +0100 Subject: [PATCH 5/8] Add exception check due to internal message change The new MKL release changed the message of unimplemented exception. Since tests were skipped by checking that message, some tests started to fail. Update skipping check to keep compatibility with both versions. --- tests/unit_tests/dft/source/compute_tests.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/dft/source/compute_tests.cpp b/tests/unit_tests/dft/source/compute_tests.cpp index 005f833ef..2d534f11a 100644 --- a/tests/unit_tests/dft/source/compute_tests.cpp +++ b/tests/unit_tests/dft/source/compute_tests.cpp @@ -75,7 +75,8 @@ class ComputeTests_real_real_out_of_place_REAL } \ catch (std::exception & e) { \ std::string msg = e.what(); \ - if (msg.find("FFT_UNIMPLEMENTED") != std::string::npos) { \ + if ((msg.find("FFT_UNIMPLEMENTED") != std::string::npos) || \ + msg.find("unimplemented") != std::string::npos) { \ std::cout << "Skipping test because: \"" << msg << "\"" << std::endl; \ GTEST_SKIP(); \ } \ From 97e20a3c123e8132c5b218ff43a031475178aad1 Mon Sep 17 00:00:00 2001 From: nscipione Date: Mon, 7 Oct 2024 16:47:26 +0100 Subject: [PATCH 6/8] Remove some enum translation in mklgpu_helpers Removing enum elements that cause compilation failure in MKL 2025.0 Apparently also previous version don't need them. --- src/dft/backends/mklgpu/mklgpu_helpers.hpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/dft/backends/mklgpu/mklgpu_helpers.hpp b/src/dft/backends/mklgpu/mklgpu_helpers.hpp index 710dbaf6f..4b93d1423 100644 --- a/src/dft/backends/mklgpu/mklgpu_helpers.hpp +++ b/src/dft/backends/mklgpu/mklgpu_helpers.hpp @@ -69,13 +69,10 @@ inline constexpr dft::config_param to_mklgpu(dft::detail::config_param param) { case iparam::FORWARD_SCALE: return oparam::FORWARD_SCALE; case iparam::NUMBER_OF_TRANSFORMS: return oparam::NUMBER_OF_TRANSFORMS; case iparam::COMPLEX_STORAGE: return oparam::COMPLEX_STORAGE; - //case iparam::REAL_STORAGE: return oparam::REAL_STORAGE; case iparam::CONJUGATE_EVEN_STORAGE: return oparam::CONJUGATE_EVEN_STORAGE; case iparam::FWD_DISTANCE: return oparam::FWD_DISTANCE; case iparam::BWD_DISTANCE: return oparam::BWD_DISTANCE; case iparam::WORKSPACE: return oparam::WORKSPACE; - //case iparam::ORDERING: return oparam::ORDERING; - //case iparam::TRANSPOSE: return oparam::TRANSPOSE; case iparam::PACKED_FORMAT: return oparam::PACKED_FORMAT; case iparam::WORKSPACE_PLACEMENT: return oparam::WORKSPACE; // Same as WORKSPACE case iparam::WORKSPACE_EXTERNAL_BYTES: return oparam::WORKSPACE_BYTES; From a2cd4b2ba0e237c47f7e33c543d7971b54f711ab Mon Sep 17 00:00:00 2001 From: nscipione Date: Tue, 8 Oct 2024 17:07:31 +0100 Subject: [PATCH 7/8] Fix typo --- tests/unit_tests/dft/source/compute_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/dft/source/compute_tests.cpp b/tests/unit_tests/dft/source/compute_tests.cpp index 2d534f11a..7f737ef50 100644 --- a/tests/unit_tests/dft/source/compute_tests.cpp +++ b/tests/unit_tests/dft/source/compute_tests.cpp @@ -76,7 +76,7 @@ class ComputeTests_real_real_out_of_place_REAL catch (std::exception & e) { \ std::string msg = e.what(); \ if ((msg.find("FFT_UNIMPLEMENTED") != std::string::npos) || \ - msg.find("unimplemented") != std::string::npos) { \ + (msg.find("Unimplemented") != std::string::npos)) { \ std::cout << "Skipping test because: \"" << msg << "\"" << std::endl; \ GTEST_SKIP(); \ } \ From 18bd3da50a093f7826408b1c19384749ae93b52c Mon Sep 17 00:00:00 2001 From: nscipione Date: Thu, 10 Oct 2024 16:55:40 +0100 Subject: [PATCH 8/8] Address PR review comment --- src/dft/backends/mklgpu/CMakeLists.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dft/backends/mklgpu/CMakeLists.txt b/src/dft/backends/mklgpu/CMakeLists.txt index d90935dbf..8ec322de8 100644 --- a/src/dft/backends/mklgpu/CMakeLists.txt +++ b/src/dft/backends/mklgpu/CMakeLists.txt @@ -37,8 +37,9 @@ target_include_directories(${LIB_NAME} ) # Due to using the same file name for different headers in this library and in -# MKL Intel library, we force the compiler to follow C++ Core Guideline SF.12 -# using the flag "-iquote" to avoid conflicts and find the correct header. +# the Intel(R) oneAPI Math Kernel Library, we force the compiler to follow C++ +# Core Guideline SF.12 using the flag "-iquote" to avoid conflicts and find the +# correct header. target_compile_options(${LIB_OBJ} BEFORE PRIVATE -iquote $ )