From 5a90a88debd66a6e887c6819c93fd01e3d19999d Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Fri, 13 Sep 2019 14:28:34 +0200 Subject: [PATCH 01/18] fix the unallowed code in MSVC --- core/base/combination.cpp | 2 +- core/test/log/stream.cpp | 18 +++++++++++------- include/ginkgo/core/base/abstract_factory.hpp | 4 ++-- include/ginkgo/core/base/executor.hpp | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/core/base/combination.cpp b/core/base/combination.cpp index 6fc3c9787fb..567d8d9778b 100644 --- a/core/base/combination.cpp +++ b/core/base/combination.cpp @@ -77,7 +77,7 @@ void Combination::apply_impl(const LinOp *alpha, const LinOp *b, { if (cache_.intermediate_x == nullptr || cache_.intermediate_x->get_size() != x->get_size()) { - cache_.intermediate_x = clone(x); + cache_.intermediate_x = x->clone(); } this->apply_impl(b, lend(cache_.intermediate_x)); auto dense_x = as>(x); diff --git a/core/test/log/stream.cpp b/core/test/log/stream.cpp index 9e9333b263b..4f22c49927c 100644 --- a/core/test/log/stream.cpp +++ b/core/test/log/stream.cpp @@ -74,7 +74,7 @@ TEST(Stream, CatchesAllocationCompleted) exec, gko::log::Logger::allocation_completed_mask, out); int dummy = 1; std::stringstream ptrstream; - ptrstream << &dummy; + ptrstream << std::hex << "0x" << reinterpret_cast(&dummy); logger->on( exec.get(), 42, reinterpret_cast(&dummy)); @@ -94,7 +94,7 @@ TEST(Stream, CatchesFreeStarted) exec, gko::log::Logger::free_started_mask, out); int dummy = 1; std::stringstream ptrstream; - ptrstream << &dummy; + ptrstream << std::hex << "0x" << reinterpret_cast(&dummy); logger->on( exec.get(), reinterpret_cast(&dummy)); @@ -113,7 +113,7 @@ TEST(Stream, CatchesFreeCompleted) exec, gko::log::Logger::free_completed_mask, out); int dummy = 1; std::stringstream ptrstream; - ptrstream << &dummy; + ptrstream << std::hex << "0x" << reinterpret_cast(&dummy); logger->on( exec.get(), reinterpret_cast(&dummy)); @@ -133,9 +133,11 @@ TEST(Stream, CatchesCopyStarted) int dummy_in = 1; int dummy_out = 1; std::stringstream ptrstream_in; - ptrstream_in << &dummy_in; + ptrstream_in << std::hex << "0x" + << reinterpret_cast(&dummy_in); std::stringstream ptrstream_out; - ptrstream_out << &dummy_out; + ptrstream_out << std::hex << "0x" + << reinterpret_cast(&dummy_out); logger->on( exec.get(), exec.get(), reinterpret_cast(&dummy_in), @@ -158,9 +160,11 @@ TEST(Stream, CatchesCopyCompleted) int dummy_in = 1; int dummy_out = 1; std::stringstream ptrstream_in; - ptrstream_in << &dummy_in; + ptrstream_in << std::hex << "0x" + << reinterpret_cast(&dummy_in); std::stringstream ptrstream_out; - ptrstream_out << &dummy_out; + ptrstream_out << std::hex << "0x" + << reinterpret_cast(&dummy_out); logger->on( exec.get(), exec.get(), reinterpret_cast(&dummy_in), diff --git a/include/ginkgo/core/base/abstract_factory.hpp b/include/ginkgo/core/base/abstract_factory.hpp index 5f60cb0149a..58a3407d3fc 100644 --- a/include/ginkgo/core/base/abstract_factory.hpp +++ b/include/ginkgo/core/base/abstract_factory.hpp @@ -220,8 +220,8 @@ class EnableDefaultFactory */ template typename std::enable_if< - not std::is_base_of::value || - not std::is_base_of::value, + !std::is_base_of::value || + !std::is_base_of::value, void>::type propagate_loggers(TheType *product) const {} diff --git a/include/ginkgo/core/base/executor.hpp b/include/ginkgo/core/base/executor.hpp index 7d015224785..3db0501f6d0 100644 --- a/include/ginkgo/core/base/executor.hpp +++ b/include/ginkgo/core/base/executor.hpp @@ -803,7 +803,7 @@ using DefaultExecutor = ReferenceExecutor; */ class CudaExecutor : public detail::ExecutorBase, public std::enable_shared_from_this { - friend class ExecutorBase; + friend class detail::ExecutorBase; public: /** From aaa7f73b98629d54fe0aaac9b00ccd24f1afec7f Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Fri, 13 Sep 2019 15:09:55 +0200 Subject: [PATCH 02/18] fix inherit problem --- include/ginkgo/core/base/lin_op.hpp | 14 +++++++++++--- include/ginkgo/core/stop/criterion.hpp | 14 +++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/include/ginkgo/core/base/lin_op.hpp b/include/ginkgo/core/base/lin_op.hpp index 156fa132f59..4a85c73326d 100644 --- a/include/ginkgo/core/base/lin_op.hpp +++ b/include/ginkgo/core/base/lin_op.hpp @@ -719,9 +719,17 @@ public: \ ::gko::LinOpFactory>; \ friend class ::gko::enable_parameters_type<_parameters_name##_type, \ _factory_name>; \ - using ::gko::EnableDefaultLinOpFactory< \ - _factory_name, _lin_op, \ - _parameters_name##_type>::EnableDefaultLinOpFactory; \ + explicit _factory_name(std::shared_ptr exec) \ + : ::gko::EnableDefaultLinOpFactory<_factory_name, _lin_op, \ + _parameters_name##_type>( \ + std::move(exec)) \ + {} \ + explicit _factory_name(std::shared_ptr exec, \ + const _parameters_name##_type ¶meters) \ + : ::gko::EnableDefaultLinOpFactory<_factory_name, _lin_op, \ + _parameters_name##_type>( \ + std::move(exec), parameters) \ + {} \ }; \ friend ::gko::EnableDefaultLinOpFactory<_factory_name, _lin_op, \ _parameters_name##_type>; \ diff --git a/include/ginkgo/core/stop/criterion.hpp b/include/ginkgo/core/stop/criterion.hpp index 9a5fc43dcf7..f619137387e 100644 --- a/include/ginkgo/core/stop/criterion.hpp +++ b/include/ginkgo/core/stop/criterion.hpp @@ -292,9 +292,17 @@ public: \ _factory_name, ::gko::stop::CriterionFactory>; \ friend class ::gko::enable_parameters_type<_parameters_name##_type, \ _factory_name>; \ - using ::gko::stop::EnableDefaultCriterionFactory< \ - _factory_name, _criterion, \ - _parameters_name##_type>::EnableDefaultCriterionFactory; \ + explicit _factory_name(std::shared_ptr exec) \ + : ::gko::stop::EnableDefaultCriterionFactory< \ + _factory_name, _criterion, _parameters_name##_type>( \ + std::move(exec)) \ + {} \ + explicit _factory_name(std::shared_ptr exec, \ + const _parameters_name##_type ¶meters) \ + : ::gko::stop::EnableDefaultCriterionFactory< \ + _factory_name, _criterion, _parameters_name##_type>( \ + std::move(exec), parameters) \ + {} \ }; \ friend ::gko::stop::EnableDefaultCriterionFactory< \ _factory_name, _criterion, _parameters_name##_type>; \ From a104d07673453326142ef1858968893cc4130adc Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Fri, 13 Sep 2019 16:53:15 +0200 Subject: [PATCH 03/18] use overload to make MSVC recognize the enable_if --- core/base/mtx_io.cpp | 9 +++++---- core/test/utils/matrix_generator.hpp | 7 +++++-- include/ginkgo/core/base/math.hpp | 3 ++- include/ginkgo/core/base/matrix_data.hpp | 7 +++++-- include/ginkgo/core/base/utils.hpp | 16 ++++++++++++++-- 5 files changed, 31 insertions(+), 11 deletions(-) diff --git a/core/base/mtx_io.cpp b/core/base/mtx_io.cpp index 0213f267baf..689991f217d 100644 --- a/core/base/mtx_io.cpp +++ b/core/base/mtx_io.cpp @@ -42,6 +42,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include namespace gko { @@ -161,7 +162,7 @@ class mtx_io { } private: - template + template = nullptr> static xstd::enable_if_t()> write_entry_impl( std::ostream &, const T &) { @@ -169,7 +170,7 @@ class mtx_io { "trying to write a complex matrix into a real entry format"); } - template + template = nullptr> static xstd::enable_if_t()> write_entry_impl( std::ostream &os, const T &value) { @@ -208,7 +209,7 @@ class mtx_io { } private: - template + template = nullptr> static xstd::enable_if_t(), T> read_entry_impl( std::istream &is) { @@ -220,7 +221,7 @@ class mtx_io { return {static_cast(real), static_cast(imag)}; } - template + template = nullptr> static xstd::enable_if_t(), T> read_entry_impl( std::istream &) { diff --git a/core/test/utils/matrix_generator.hpp b/core/test/utils/matrix_generator.hpp index 384cc332bac..bd04e2bd992 100644 --- a/core/test/utils/matrix_generator.hpp +++ b/core/test/utils/matrix_generator.hpp @@ -42,6 +42,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include @@ -50,7 +51,8 @@ namespace test { namespace detail { -template +template = nullptr> typename std::enable_if(), ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { @@ -58,7 +60,8 @@ get_rand_value(Distribution &&dist, Generator &&gen) } -template +template = nullptr> typename std::enable_if(), ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { diff --git a/include/ginkgo/core/base/math.hpp b/include/ginkgo/core/base/math.hpp index 21d6db6891d..1ef12c1e23e 100644 --- a/include/ginkgo/core/base/math.hpp +++ b/include/ginkgo/core/base/math.hpp @@ -36,6 +36,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +#include #include @@ -534,7 +535,7 @@ using std::isfinite; // use the optimized function for all supported types * returns `true` if both components of the given value are finite, meaning * they are neither +/- infinity nor NaN. */ -template +template = nullptr> GKO_INLINE GKO_ATTRIBUTES xstd::enable_if_t::value, bool> isfinite(const T &value) { diff --git a/include/ginkgo/core/base/matrix_data.hpp b/include/ginkgo/core/base/matrix_data.hpp index 54bd261c551..e1da8106089 100644 --- a/include/ginkgo/core/base/matrix_data.hpp +++ b/include/ginkgo/core/base/matrix_data.hpp @@ -39,6 +39,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include #include @@ -62,7 +63,8 @@ struct input_triple { }; -template +template = nullptr> typename std::enable_if(), ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { @@ -70,7 +72,8 @@ get_rand_value(Distribution &&dist, Generator &&gen) } -template +template = nullptr> typename std::enable_if(), ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { diff --git a/include/ginkgo/core/base/utils.hpp b/include/ginkgo/core/base/utils.hpp index bc237a00ddc..d9023fa5150 100644 --- a/include/ginkgo/core/base/utils.hpp +++ b/include/ginkgo/core/base/utils.hpp @@ -233,6 +233,18 @@ inline typename std::remove_reference::type &&give( } +/** + * Create an overload function. + * + * This is a helper function which makes the MSVC can recognize + * enable_if function. + * + * @param Index the index of overload function. + */ +template +using overload = std::integral_constant *; + + /** * Returns a non-owning (plain) pointer to the object pointed to by `p`. * @@ -243,7 +255,7 @@ inline typename std::remove_reference::type &&give( * @note This is the overload for owning (smart) pointers, that behaves the * same as calling .get() on the smart pointer. */ -template +template = nullptr> inline typename std::enable_if(), detail::pointee *>::type lend(const Pointer &p) @@ -261,7 +273,7 @@ lend(const Pointer &p) * @note This is the overload for non-owning (plain) pointers, that just * returns `p`. */ -template +template = nullptr> inline typename std::enable_if(), detail::pointee *>::type lend(const Pointer &p) From 459da12009874a6ec0c1c194ab1f0c5f2622fcdd Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Mon, 16 Sep 2019 14:38:11 +0200 Subject: [PATCH 04/18] fix gflags with static link and find CUDA library automatically in MSVC fix isfinite in MSVC with cuda move update_header in the devel_tool fix benchmark/spmv and cuda flags --- CMakeLists.txt | 43 ++++++++++++++++++--- benchmark/spmv/CMakeLists.txt | 10 +++-- cuda/CMakeLists.txt | 31 +++++++++++++++ cuda/factorization/par_ilu_kernels.cu | 4 +- reference/factorization/par_ilu_kernels.cpp | 4 +- third_party/gflags/CMakeLists.txt | 25 +++++++++--- 6 files changed, 97 insertions(+), 20 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d09d805dbcc..6a59996133e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,8 +22,13 @@ option(GINKGO_WITH_CLANG_TIDY "Make Ginkgo call `clang-tidy` to find programming option(GINKGO_WITH_IWYU "Make Ginkgo call `iwyu` (Include What You Use) to find include issues." OFF) set(GINKGO_VERBOSE_LEVEL "1" CACHE STRING "Verbosity level. Put 0 to turn off. 1 activates a few important messages.") -set(GINKGO_COMPILER_FLAGS "-Wpedantic" CACHE STRING - "Set the required CXX compiler flags, mainly used for warnings. Current default is `-Wpedantic`") +if(MSVC) + set(GINKGO_COMPILER_FLAGS "" CACHE STRING + "Set the required CXX compiler flags, mainly used for warnings. Current default is ``") +else() + set(GINKGO_COMPILER_FLAGS "-Wpedantic" CACHE STRING + "Set the required CXX compiler flags, mainly used for warnings. Current default is `-Wpedantic`") +endif() set(GINKGO_CUDA_COMPILER_FLAGS "" CACHE STRING "Set the required NVCC compiler flags, mainly used for warnings. Current default is an empty string") set(GINKGO_CUDA_ARCHITECTURES "Auto" CACHE STRING @@ -104,13 +109,39 @@ endif() configure_file(${Ginkgo_SOURCE_DIR}/include/ginkgo/config.hpp.in ${Ginkgo_BINARY_DIR}/include/ginkgo/config.hpp @ONLY) +# This is modified from https://gitlab.kitware.com/cmake/community/wikis/FAQ#dynamic-replace +if(MSVC) + if(BUILD_SHARED_LIBS) + foreach(flag_var + CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE + CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO + CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE + CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO) + if(${flag_var} MATCHES "/MT") + string(REGEX REPLACE "/MT" "/MD" ${flag_var} "${${flag_var}}") + endif(${flag_var} MATCHES "/MT") + set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) + endforeach(flag_var) + else() + foreach(flag_var + CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE + CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO + CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE + CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO) + if(${flag_var} MATCHES "/MD") + string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}") + endif(${flag_var} MATCHES "/MD") + set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) + endforeach(flag_var) + endif() +endif() + # Try to find the third party packages before using our subdirectories include(cmake/package_helpers.cmake) ginkgo_find_package(GTest "GTest::GTest;GTest::Main" FALSE 1.8.1) ginkgo_find_package(gflags gflags FALSE 2.2.2) ginkgo_find_package(RapidJSON rapidjson TRUE 1.1.0) add_subdirectory(third_party) # Third-party tools and libraries - # Needs to be first in order for `CMAKE_CUDA_DEVICE_LINK_EXECUTABLE` to be # propagated to the other parts of Ginkgo in case of building as static libraries if(GINKGO_BUILD_CUDA) @@ -139,12 +170,12 @@ if(GINKGO_DEVEL_TOOLS) COMMAND ${Ginkgo_SOURCE_DIR}/dev_tools/scripts/add_license.sh WORKING_DIRECTORY ${Ginkgo_SOURCE_DIR}) add_dependencies(format add_license) -endif() -# Generate the global `ginkgo/ginkgo.hpp` header with every call of make -add_custom_target(generate_ginkgo_header ALL + # Generate the global `ginkgo/ginkgo.hpp` header with every call of make + add_custom_target(generate_ginkgo_header ALL COMMAND ${Ginkgo_SOURCE_DIR}/dev_tools/scripts/update_ginkgo_header.sh WORKING_DIRECTORY ${Ginkgo_SOURCE_DIR}) +endif() # Installation diff --git a/benchmark/spmv/CMakeLists.txt b/benchmark/spmv/CMakeLists.txt index 7553bd646ce..528826ef8d8 100644 --- a/benchmark/spmv/CMakeLists.txt +++ b/benchmark/spmv/CMakeLists.txt @@ -2,10 +2,12 @@ add_executable(spmv spmv.cpp) target_link_libraries(spmv ginkgo gflags rapidjson) if (GINKGO_BUILD_CUDA) target_compile_definitions(spmv PRIVATE HAS_CUDA=1) - target_link_libraries(spmv ginkgo ${CUDA_RUNTIME_LIBS} - ${CUBLAS} ${CUSPARSE}) - target_include_directories(spmv SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS}) + if (NOT MSVC) + target_link_libraries(spmv ginkgo ${CUDA_RUNTIME_LIBS} + ${CUBLAS} ${CUSPARSE}) + target_include_directories(spmv SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS}) + endif() if(CMAKE_CUDA_COMPILER_VERSION GREATER_EQUAL "9.2") target_compile_definitions(spmv PRIVATE ALLOWMP=1) endif() -endif() +endif() \ No newline at end of file diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index d44728b81d4..02fad3dd80a 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -6,6 +6,37 @@ if (NOT BUILD_SHARED_LIBS) set(CMAKE_CUDA_DEVICE_LINK_EXECUTABLE ${CMAKE_CUDA_DEVICE_LINK_EXECUTABLE} PARENT_SCOPE) endif() +# MSVC can not find CUDA automatically +# Use CUDA_COMPILER PATH to define the CUDA TOOLKIT ROOT DIR +if ("${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}" STREQUAL "") + string(REPLACE "/bin/nvcc.exe" "" CMAKE_CUDA_ROOT_DIR ${CMAKE_CUDA_COMPILER}) + set(CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES "${CMAKE_CUDA_ROOT_DIR}/include") + set(CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES "${CMAKE_CUDA_ROOT_DIR}/lib/x64") +endif() + +# This is modified from https://gitlab.kitware.com/cmake/community/wikis/FAQ#dynamic-replace +if(MSVC) + if(BUILD_SHARED_LIBS) + foreach(flag_var + CMAKE_CUDA_FLAGS CMAKE_CUDA_FLAGS_DEBUG CMAKE_CUDA_FLAGS_RELEASE + CMAKE_CUDA_FLAGS_MINSIZEREL CMAKE_CUDA_FLAGS_RELWITHDEBINFO) + if(${flag_var} MATCHES "-MT") + string(REGEX REPLACE "-MT" "-MD" ${flag_var} "${${flag_var}}") + endif(${flag_var} MATCHES "-MT") + set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) + endforeach(flag_var) + else() + foreach(flag_var + CMAKE_CUDA_FLAGS CMAKE_CUDA_FLAGS_DEBUG CMAKE_CUDA_FLAGS_RELEASE + CMAKE_CUDA_FLAGS_MINSIZEREL CMAKE_CUDA_FLAGS_RELWITHDEBINFO) + if(${flag_var} MATCHES "-MD") + string(REGEX REPLACE "-MD" "-MT" ${flag_var} "${${flag_var}}") + endif(${flag_var} MATCHES "-MD") + set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) + endforeach(flag_var) + endif() +endif() + include(CudaArchitectureSelector) set(CUDA_INCLUDE_DIRS ${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}) diff --git a/cuda/factorization/par_ilu_kernels.cu b/cuda/factorization/par_ilu_kernels.cu index 7de750250c8..6f212b9d75c 100644 --- a/cuda/factorization/par_ilu_kernels.cu +++ b/cuda/factorization/par_ilu_kernels.cu @@ -223,12 +223,12 @@ __global__ __launch_bounds__(default_block_size) void compute_l_u_factors( sum += last_operation; // undo the last operation if (row > col) { auto to_write = sum / u_values[u_row_ptrs[col + 1] - 1]; - if (isfinite(to_write)) { + if (::gko::isfinite(to_write)) { l_values[l_idx - 1] = to_write; } } else { auto to_write = sum; - if (isfinite(to_write)) { + if (::gko::isfinite(to_write)) { u_values[u_idx - 1] = to_write; } } diff --git a/reference/factorization/par_ilu_kernels.cpp b/reference/factorization/par_ilu_kernels.cpp index 14c318a9033..d7df460e2d5 100644 --- a/reference/factorization/par_ilu_kernels.cpp +++ b/reference/factorization/par_ilu_kernels.cpp @@ -182,12 +182,12 @@ void compute_l_u_factors(std::shared_ptr exec, if (row > col) { // modify entry in L auto to_write = sum / vals_u[row_ptrs_u[col + 1] - 1]; - if (isfinite(to_write)) { + if (::gko::isfinite(to_write)) { vals_l[row_l - 1] = to_write; } } else { // modify entry in U auto to_write = sum; - if (isfinite(to_write)) { + if (::gko::isfinite(to_write)) { vals_u[row_u - 1] = to_write; } } diff --git a/third_party/gflags/CMakeLists.txt b/third_party/gflags/CMakeLists.txt index ab98c80b37b..44dc9cfa065 100644 --- a/third_party/gflags/CMakeLists.txt +++ b/third_party/gflags/CMakeLists.txt @@ -1,10 +1,23 @@ -ginkgo_load_git_package(gflags_external - "https://github.com/gflags/gflags.git" - "e171aa2d15ed9eb17054558e0b3a6a413bb01067" - "-DGFLAGS_BUILD_TESTING=OFF" "-DGFLAGS_BUILD_gflags_LIB=OFF" - "-DGFLAGS_BUILD_gflags_nothreads_LIB=ON" "-DGFLAGS_BUILD_STATIC_LIBS=ON" - "-DGFLAGS_BUILD_PACKAGING=OFF") +if(MSVC) + # cmake links dynamic runtime libraries by default in Visual Studio + # use the ginkgo's flags to use the same runtime libraries as ginkgo + ginkgo_load_git_package(gflags_external + "https://github.com/gflags/gflags.git" + "e171aa2d15ed9eb17054558e0b3a6a413bb01067" + "-DGFLAGS_BUILD_TESTING=OFF" "-DGFLAGS_BUILD_gflags_LIB=OFF" + "-DGFLAGS_BUILD_gflags_nothreads_LIB=ON" "-DGFLAGS_BUILD_STATIC_LIBS=ON" + "-DGFLAGS_BUILD_PACKAGING=OFF" "-DCMAKE_CXX_FLAGS_DEBUG=${CMAKE_CXX_FLAGS_DEBUG}" + "-DCMAKE_CXX_FLAGS_RELEASE=${CMAKE_CXX_FLAGS_RELEASE}") +else() + ginkgo_load_git_package(gflags_external + "https://github.com/gflags/gflags.git" + "e171aa2d15ed9eb17054558e0b3a6a413bb01067" + "-DGFLAGS_BUILD_TESTING=OFF" "-DGFLAGS_BUILD_gflags_LIB=OFF" + "-DGFLAGS_BUILD_gflags_nothreads_LIB=ON" "-DGFLAGS_BUILD_STATIC_LIBS=ON" + "-DGFLAGS_BUILD_PACKAGING=OFF") +endif() if(WIN32) + # gflags uses gflags_nothreads_static not gflags_nothreads_static in Windows. ginkgo_add_external_target(gflags gflags_nothreads_static build/include build/lib STATIC "_debug" gflags_external FALSE) else() ginkgo_add_external_target(gflags gflags_nothreads build/include build/lib STATIC "_debug" gflags_external FALSE) From 42673d5ca8b90ef0f77154cacd18ea12ccf5039f Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Thu, 19 Sep 2019 16:23:58 +0200 Subject: [PATCH 05/18] make MSVC compile shared library --- CMakeLists.txt | 26 ++++++-------------------- cmake/build_helpers.cmake | 25 ++++++++++++++++++++++++- cuda/CMakeLists.txt | 18 ++---------------- third_party/gflags/CMakeLists.txt | 5 +++-- third_party/gtest/CMakeLists.txt | 19 ++++++++++++++----- 5 files changed, 49 insertions(+), 44 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a59996133e..c63000520fd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -112,27 +112,13 @@ configure_file(${Ginkgo_SOURCE_DIR}/include/ginkgo/config.hpp.in # This is modified from https://gitlab.kitware.com/cmake/community/wikis/FAQ#dynamic-replace if(MSVC) if(BUILD_SHARED_LIBS) - foreach(flag_var - CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE - CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO - CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE - CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO) - if(${flag_var} MATCHES "/MT") - string(REGEX REPLACE "/MT" "/MD" ${flag_var} "${${flag_var}}") - endif(${flag_var} MATCHES "/MT") - set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) - endforeach(flag_var) + ginkgo_turn_to_windows_dynamic("CXX") + ginkgo_turn_to_windows_dynamic("C") + set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS TRUE) else() - foreach(flag_var - CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE - CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO - CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE - CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO) - if(${flag_var} MATCHES "/MD") - string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}") - endif(${flag_var} MATCHES "/MD") - set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) - endforeach(flag_var) + ginkgo_turn_to_windows_static("CXX") + ginkgo_turn_to_windows_static("C") + set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS FALSE) endif() endif() diff --git a/cmake/build_helpers.cmake b/cmake/build_helpers.cmake index 2ac9bfc0ede..2e92df4367c 100644 --- a/cmake/build_helpers.cmake +++ b/cmake/build_helpers.cmake @@ -55,4 +55,27 @@ function(ginkgo_check_shared_library name) message(FATAL_ERROR "Did not find this build in the environment variable PATH. " "Please add ${GINKGO_WINDOWS_SHARED_LIBRARY_PATH} into the environment variable PATH.") endif() -endfunction() \ No newline at end of file +endfunction() + +function(ginkgo_turn_windows_link lang from to) + foreach(flag_var + "CMAKE_${lang}_FLAGS" "CMAKE_${lang}_FLAGS_DEBUG" "CMAKE_${lang}_FLAGS_RELEASE" + "CMAKE_${lang}_FLAGS_MINSIZEREL" "CMAKE_${lang}_FLAGS_RELWITHDEBINFO" + ) + if(${flag_var} MATCHES "/${from}") + string(REGEX REPLACE "/${from}" "/${to}" ${flag_var} "${${flag_var}}") + endif(${flag_var} MATCHES "/${from}") + if(${flag_var} MATCHES "-${from}") + string(REGEX REPLACE "-${from}" "-${to}" ${flag_var} "${${flag_var}}") + endif(${flag_var} MATCHES "-${from}") + set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) + endforeach(flag_var) +endfunction() + +macro(ginkgo_turn_to_windows_static lang) + ginkgo_turn_windows_link(${lang} "MD" "MT") +endmacro() + +macro(ginkgo_turn_to_windows_dynamic lang) + ginkgo_turn_windows_link(${lang} "MT" "MD") +endmacro() \ No newline at end of file diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index 02fad3dd80a..99fdbf5eefe 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -17,23 +17,9 @@ endif() # This is modified from https://gitlab.kitware.com/cmake/community/wikis/FAQ#dynamic-replace if(MSVC) if(BUILD_SHARED_LIBS) - foreach(flag_var - CMAKE_CUDA_FLAGS CMAKE_CUDA_FLAGS_DEBUG CMAKE_CUDA_FLAGS_RELEASE - CMAKE_CUDA_FLAGS_MINSIZEREL CMAKE_CUDA_FLAGS_RELWITHDEBINFO) - if(${flag_var} MATCHES "-MT") - string(REGEX REPLACE "-MT" "-MD" ${flag_var} "${${flag_var}}") - endif(${flag_var} MATCHES "-MT") - set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) - endforeach(flag_var) + ginkgo_turn_to_windows_dynamic("CUDA") else() - foreach(flag_var - CMAKE_CUDA_FLAGS CMAKE_CUDA_FLAGS_DEBUG CMAKE_CUDA_FLAGS_RELEASE - CMAKE_CUDA_FLAGS_MINSIZEREL CMAKE_CUDA_FLAGS_RELWITHDEBINFO) - if(${flag_var} MATCHES "-MD") - string(REGEX REPLACE "-MD" "-MT" ${flag_var} "${${flag_var}}") - endif(${flag_var} MATCHES "-MD") - set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) - endforeach(flag_var) + ginkgo_turn_to_windows_static("CUDA") endif() endif() diff --git a/third_party/gflags/CMakeLists.txt b/third_party/gflags/CMakeLists.txt index 44dc9cfa065..410da8599c0 100644 --- a/third_party/gflags/CMakeLists.txt +++ b/third_party/gflags/CMakeLists.txt @@ -1,4 +1,4 @@ -if(MSVC) +if(MSVC AND NOT BUILD_SHARED_LIBS) # cmake links dynamic runtime libraries by default in Visual Studio # use the ginkgo's flags to use the same runtime libraries as ginkgo ginkgo_load_git_package(gflags_external @@ -7,7 +7,8 @@ if(MSVC) "-DGFLAGS_BUILD_TESTING=OFF" "-DGFLAGS_BUILD_gflags_LIB=OFF" "-DGFLAGS_BUILD_gflags_nothreads_LIB=ON" "-DGFLAGS_BUILD_STATIC_LIBS=ON" "-DGFLAGS_BUILD_PACKAGING=OFF" "-DCMAKE_CXX_FLAGS_DEBUG=${CMAKE_CXX_FLAGS_DEBUG}" - "-DCMAKE_CXX_FLAGS_RELEASE=${CMAKE_CXX_FLAGS_RELEASE}") + "-DCMAKE_CXX_FLAGS_RELEASE=${CMAKE_CXX_FLAGS_RELEASE}" "-DCMAKE_C_FLAGS_DEBUG=${CMAKE_C_FLAGS_DEBUG}" + "-DCMAKE_C_FLAGS_RELEASE=${CMAKE_C_FLAGS_RELEASE}") else() ginkgo_load_git_package(gflags_external "https://github.com/gflags/gflags.git" diff --git a/third_party/gtest/CMakeLists.txt b/third_party/gtest/CMakeLists.txt index aa4895ee52e..327620ab234 100644 --- a/third_party/gtest/CMakeLists.txt +++ b/third_party/gtest/CMakeLists.txt @@ -1,8 +1,17 @@ -ginkgo_load_git_package(gtest_external - "https://github.com/google/googletest.git" - "2fe3bd994b3189899d93f1d5a881e725e046fdc2" - # Work around the linking errors when compiling gtest with CUDA - "-Dgtest_disable_pthreads=ON") +if(MSVC AND BUILD_SHARED_LIBS) + # Force using shared runtime library when MSVC builds shared libraries + ginkgo_load_git_package(gtest_external + "https://github.com/google/googletest.git" + "2fe3bd994b3189899d93f1d5a881e725e046fdc2" + # Work around the linking errors when compiling gtest with CUDA + "-Dgtest_disable_pthreads=ON" "-Dgtest_force_shared_crt=TRUE") +else() + ginkgo_load_git_package(gtest_external + "https://github.com/google/googletest.git" + "2fe3bd994b3189899d93f1d5a881e725e046fdc2" + # Work around the linking errors when compiling gtest with CUDA + "-Dgtest_disable_pthreads=ON") +endif() ginkgo_add_external_target(GTest::GTest gtest src/googletest/include build/googlemock/gtest STATIC "d" gtest_external FALSE) ginkgo_add_external_target(GTest::Main gtest_main src/googletest/include build/googlemock/gtest From 2657c9b6008abc74d2cdef739b5d441526cb8777 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Fri, 20 Sep 2019 12:53:52 +0200 Subject: [PATCH 06/18] fix the ginkgo library path in MSVC and fix the third party path in MVSC --- cmake/build_helpers.cmake | 14 ++++++++-- cmake/package_helpers.cmake | 56 +++++++++++++++++++++++++++++-------- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/cmake/build_helpers.cmake b/cmake/build_helpers.cmake index 2e92df4367c..7b372777d7e 100644 --- a/cmake/build_helpers.cmake +++ b/cmake/build_helpers.cmake @@ -23,6 +23,16 @@ function(ginkgo_compile_features name) RUNTIME_OUTPUT_DIRECTORY "${GINKGO_WINDOWS_SHARED_LIBRARY_PATH}") set_property(TARGET "${name}" PROPERTY ARCHIVE_OUTPUT_DIRECTORY "${GINKGO_WINDOWS_SHARED_LIBRARY_PATH}") + if(MSVC) + # MSVC would create subfolder according to build_type. Ginkgo forces the output be the same whatever build_type is. + foreach(CONFIG ${CMAKE_CONFIGURATION_TYPES}) + string(TOUPPER ${CONFIG} CONFIG ) + set_property(TARGET "${name}" PROPERTY + RUNTIME_OUTPUT_DIRECTORY_${CONFIG} "${GINKGO_WINDOWS_SHARED_LIBRARY_PATH}") + set_property(TARGET "${name}" PROPERTY + ARCHIVE_OUTPUT_DIRECTORY_${CONFIG} "${GINKGO_WINDOWS_SHARED_LIBRARY_PATH}") + endforeach() + endif() if(GINKGO_CHECK_PATH) ginkgo_check_shared_library("${CMAKE_SHARED_LIBRARY_PREFIX}${name}${CMAKE_SHARED_LIBRARY_SUFFIX}") endif() @@ -49,7 +59,7 @@ function(ginkgo_check_shared_library name) # do not keep this variable in cache unset(EXISTING_DLL CACHE) endif() - endforeach(ITEM) + endforeach() if(NOT PASSED_TEST) # Did not find this build in the environment variable PATH message(FATAL_ERROR "Did not find this build in the environment variable PATH. " @@ -69,7 +79,7 @@ function(ginkgo_turn_windows_link lang from to) string(REGEX REPLACE "-${from}" "-${to}" ${flag_var} "${${flag_var}}") endif(${flag_var} MATCHES "-${from}") set(${flag_var} "${${flag_var}}" CACHE STRING "" FORCE) - endforeach(flag_var) + endforeach() endfunction() macro(ginkgo_turn_to_windows_static lang) diff --git a/cmake/package_helpers.cmake b/cmake/package_helpers.cmake index 7f0cab28548..ec5c28219a4 100644 --- a/cmake/package_helpers.cmake +++ b/cmake/package_helpers.cmake @@ -15,12 +15,30 @@ function(ginkgo_load_git_package package_name package_url package_tag) message(FATAL_ERROR "CMake step for ${package_name}/download failed: ${result}") endif() - execute_process(COMMAND ${CMAKE_COMMAND} --build . - RESULT_VARIABLE result - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/download) - if(result) - message(FATAL_ERROR - "Build step for ${package_name}/download failed: ${result}") + if(MSVC) + # MSVC decides the build_type in build step not cmake step, so Ginkgo builds Debug and Release type. + execute_process(COMMAND ${CMAKE_COMMAND} --build . --config Debug + RESULT_VARIABLE result + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/download) + if(result) + message(FATAL_ERROR + "Build Debug step for ${package_name}/download failed: ${result}") + endif() + execute_process(COMMAND ${CMAKE_COMMAND} --build . --config Release + RESULT_VARIABLE result + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/download) + if(result) + message(FATAL_ERROR + "Build Release step for ${package_name}/download failed: ${result}") + endif() + elseif() + execute_process(COMMAND ${CMAKE_COMMAND} --build . + RESULT_VARIABLE result + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/download) + if(result) + message(FATAL_ERROR + "Build step for ${package_name}/download failed: ${result}") + endif() endif() endfunction() @@ -76,9 +94,14 @@ macro(ginkgo_add_external_target new_target external_name includedir libdir buil # Declare include directories and library files set(${external_name}_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${libdir}) set(${external_name}_INCLUDE_DIR "${CMAKE_CURRENT_BINARY_DIR}/${includedir}") - set(${external_name}_LIBRARY_RELEASE "${${external_name}_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${CMAKE_${build_type}_LIBRARY_SUFFIX}") - set(${external_name}_LIBRARY_DEBUG "${${external_name}_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${debug_postfix}${CMAKE_${build_type}_LIBRARY_SUFFIX}") - + if(MSVC) + # Ginkgo only builds Debug and Release, so set the path without CMAKE_CFG_INTDIR. + set(${external_name}_LIBRARY_RELEASE "${${external_name}_BINARY_DIR}/Release/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${CMAKE_${build_type}_LIBRARY_SUFFIX}") + set(${external_name}_LIBRARY_DEBUG "${${external_name}_BINARY_DIR}/Debug/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${debug_postfix}${CMAKE_${build_type}_LIBRARY_SUFFIX}") + elseif() + set(${external_name}_LIBRARY_RELEASE "${${external_name}_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${CMAKE_${build_type}_LIBRARY_SUFFIX}") + set(${external_name}_LIBRARY_DEBUG "${${external_name}_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${debug_postfix}${CMAKE_${build_type}_LIBRARY_SUFFIX}") + endif() # Create an IMPORTED external library available in the GLOBAL scope if (${header_only}) add_library(${new_target} INTERFACE) @@ -94,10 +117,19 @@ macro(ginkgo_add_external_target new_target external_name includedir libdir buil set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION_RELEASE ${${external_name}_LIBRARY_RELEASE}) set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION_DEBUG ${${external_name}_LIBRARY_DEBUG}) # Since we do not really manage other build types, let's globally use the DEBUG symbols - if (NOT CMAKE_BUILD_TYPE MATCHES "[Rr][Ee][Ll][Ee][Aa][Ss][Ee]" - AND NOT CMAKE_BUILD_TYPE MATCHES "[Dd][Ee][Bb][Uu][Gg]") + if(MSVC) + # Only Debug build uses MDd or MTd, and others use MD or MT. + # MSVC would like to use same runtime library, so we use Debug third-party in Debug and Release third-party in Release. + set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION_DEBUG + ${${external_name}_LIBRARY_DEBUG}) set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION - ${${external_name}_LIBRARY_DEBUG}) + ${${external_name}_LIBRARY_RELEASE}) + elseif() + if (NOT CMAKE_BUILD_TYPE MATCHES "[Rr][Ee][Ll][Ee][Aa][Ss][Ee]" + AND NOT CMAKE_BUILD_TYPE MATCHES "[Dd][Ee][Bb][Uu][Gg]") + set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION + ${${external_name}_LIBRARY_DEBUG}) + endif() endif() endif() set_target_properties(${new_target} PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${${external_name}_INCLUDE_DIR}) From c7cb6f8e5d37a4b6a02077eb97f3741f9464eff9 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Mon, 23 Sep 2019 11:11:02 +0200 Subject: [PATCH 07/18] Fix gtest build static after build dynamic --- benchmark/spmv/CMakeLists.txt | 8 +++----- cmake/build_helpers.cmake | 2 +- cmake/package_helpers.cmake | 6 +++--- third_party/gtest/CMakeLists.txt | 4 ++-- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/benchmark/spmv/CMakeLists.txt b/benchmark/spmv/CMakeLists.txt index 528826ef8d8..4de3e8f4cbb 100644 --- a/benchmark/spmv/CMakeLists.txt +++ b/benchmark/spmv/CMakeLists.txt @@ -1,12 +1,10 @@ add_executable(spmv spmv.cpp) target_link_libraries(spmv ginkgo gflags rapidjson) if (GINKGO_BUILD_CUDA) + enable_language(CUDA) + set(CMAKE_CUDA_STANDARD 11) + set(CMAKE_CUDA_STANDARD_REQUIRED ON) target_compile_definitions(spmv PRIVATE HAS_CUDA=1) - if (NOT MSVC) - target_link_libraries(spmv ginkgo ${CUDA_RUNTIME_LIBS} - ${CUBLAS} ${CUSPARSE}) - target_include_directories(spmv SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS}) - endif() if(CMAKE_CUDA_COMPILER_VERSION GREATER_EQUAL "9.2") target_compile_definitions(spmv PRIVATE ALLOWMP=1) endif() diff --git a/cmake/build_helpers.cmake b/cmake/build_helpers.cmake index 7b372777d7e..8056580e3ad 100644 --- a/cmake/build_helpers.cmake +++ b/cmake/build_helpers.cmake @@ -18,7 +18,7 @@ function(ginkgo_compile_features name) set_property(TARGET "${name}" PROPERTY CXX_INCLUDE_WHAT_YOU_USE ${GINKGO_IWYU_PATH}) endif() if(GINKGO_CHANGED_SHARED_LIBRARY) - # Put all shared libraries and corresponding imported libraries into the specified path + # Put all shared libraries and corresponding imported libraries into the specficed path set_property(TARGET "${name}" PROPERTY RUNTIME_OUTPUT_DIRECTORY "${GINKGO_WINDOWS_SHARED_LIBRARY_PATH}") set_property(TARGET "${name}" PROPERTY diff --git a/cmake/package_helpers.cmake b/cmake/package_helpers.cmake index ec5c28219a4..67414aa48b1 100644 --- a/cmake/package_helpers.cmake +++ b/cmake/package_helpers.cmake @@ -31,7 +31,7 @@ function(ginkgo_load_git_package package_name package_url package_tag) message(FATAL_ERROR "Build Release step for ${package_name}/download failed: ${result}") endif() - elseif() + else() execute_process(COMMAND ${CMAKE_COMMAND} --build . RESULT_VARIABLE result WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/download) @@ -98,7 +98,7 @@ macro(ginkgo_add_external_target new_target external_name includedir libdir buil # Ginkgo only builds Debug and Release, so set the path without CMAKE_CFG_INTDIR. set(${external_name}_LIBRARY_RELEASE "${${external_name}_BINARY_DIR}/Release/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${CMAKE_${build_type}_LIBRARY_SUFFIX}") set(${external_name}_LIBRARY_DEBUG "${${external_name}_BINARY_DIR}/Debug/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${debug_postfix}${CMAKE_${build_type}_LIBRARY_SUFFIX}") - elseif() + else() set(${external_name}_LIBRARY_RELEASE "${${external_name}_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${CMAKE_${build_type}_LIBRARY_SUFFIX}") set(${external_name}_LIBRARY_DEBUG "${${external_name}_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${CMAKE_${build_type}_LIBRARY_PREFIX}${external_name}${debug_postfix}${CMAKE_${build_type}_LIBRARY_SUFFIX}") endif() @@ -124,7 +124,7 @@ macro(ginkgo_add_external_target new_target external_name includedir libdir buil ${${external_name}_LIBRARY_DEBUG}) set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION ${${external_name}_LIBRARY_RELEASE}) - elseif() + else() if (NOT CMAKE_BUILD_TYPE MATCHES "[Rr][Ee][Ll][Ee][Aa][Ss][Ee]" AND NOT CMAKE_BUILD_TYPE MATCHES "[Dd][Ee][Bb][Uu][Gg]") set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION diff --git a/third_party/gtest/CMakeLists.txt b/third_party/gtest/CMakeLists.txt index 327620ab234..85df72f132b 100644 --- a/third_party/gtest/CMakeLists.txt +++ b/third_party/gtest/CMakeLists.txt @@ -1,10 +1,10 @@ -if(MSVC AND BUILD_SHARED_LIBS) +if(MSVC) # Force using shared runtime library when MSVC builds shared libraries ginkgo_load_git_package(gtest_external "https://github.com/google/googletest.git" "2fe3bd994b3189899d93f1d5a881e725e046fdc2" # Work around the linking errors when compiling gtest with CUDA - "-Dgtest_disable_pthreads=ON" "-Dgtest_force_shared_crt=TRUE") + "-Dgtest_disable_pthreads=ON" "-Dgtest_force_shared_crt=${BUILD_SHARED_LIBS}") else() ginkgo_load_git_package(gtest_external "https://github.com/google/googletest.git" From ab51865c0817c1e77347a8aa521b6f17d456d49d Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Mon, 23 Sep 2019 16:44:21 +0200 Subject: [PATCH 08/18] recover spmv, fix link of cuda and gflags compile --- benchmark/spmv/CMakeLists.txt | 6 +++--- cuda/CMakeLists.txt | 13 +++++++++++-- cuda/base/math.hpp | 8 ++++---- include/ginkgo/core/base/math.hpp | 10 +++++++--- third_party/gflags/CMakeLists.txt | 2 +- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/benchmark/spmv/CMakeLists.txt b/benchmark/spmv/CMakeLists.txt index 4de3e8f4cbb..0653bb8b509 100644 --- a/benchmark/spmv/CMakeLists.txt +++ b/benchmark/spmv/CMakeLists.txt @@ -1,10 +1,10 @@ add_executable(spmv spmv.cpp) target_link_libraries(spmv ginkgo gflags rapidjson) if (GINKGO_BUILD_CUDA) - enable_language(CUDA) - set(CMAKE_CUDA_STANDARD 11) - set(CMAKE_CUDA_STANDARD_REQUIRED ON) target_compile_definitions(spmv PRIVATE HAS_CUDA=1) + target_link_libraries(spmv ginkgo ${CUDA_RUNTIME_LIBS} + ${CUBLAS} ${CUSPARSE}) + target_include_directories(spmv SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS}) if(CMAKE_CUDA_COMPILER_VERSION GREATER_EQUAL "9.2") target_compile_definitions(spmv PRIVATE ALLOWMP=1) endif() diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index 99fdbf5eefe..410a41ed566 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -33,8 +33,17 @@ set(CMAKE_CUDA_FLAGS "${CMAKE_CUDA_FLAGS}" PARENT_SCOPE) set(CMAKE_CUDA_COMPILER_VERSION ${CMAKE_CUDA_COMPILER_VERSION} PARENT_SCOPE) set(CUDA_INCLUDE_DIRS ${CUDA_INCLUDE_DIRS} PARENT_SCOPE) -find_library(CUDA_RUNTIME_LIBS cudart - HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) +# nvcc uses static cudartlibrary by default +# add `-cudart shared` into CMAKE_CUDA_FLAGS to force nvcc to use dynamic cudart library. +find_library(CUDA_RUNTIME_LIBS_DYNAMIC cudart + HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) +find_library(CUDA_RUNTIME_LIBS_STATIC cudart_static + HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) +if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart shared") + set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_DYNAMIC}" CACHE STRING "Path to a library" FORCE) +else() + set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC}" CACHE STRING "Path to a library" FORCE) +endif() find_library(CUBLAS cublas HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) find_library(CUSPARSE cusparse diff --git a/cuda/base/math.hpp b/cuda/base/math.hpp index fb378485139..b94351b7595 100644 --- a/cuda/base/math.hpp +++ b/cuda/base/math.hpp @@ -99,10 +99,10 @@ __device__ GKO_INLINE std::complex one>() // as a __device__ function, so it results in a compiler error. // Here, `isfinite` is written by hand, which might not be as performant as the // intrinsic function from CUDA, but it compiles and works. -#if defined(__CUDA_ARCH__) && defined(__CUDACC_VER_MAJOR__) && \ - defined(__CUDACC_VER_MINOR__) && \ - (__CUDACC_VER_MAJOR__ * 1000 + __CUDACC_VER_MINOR__) < 9002 && \ - (defined(__clang__) || defined(__ICC) || defined(__ICL)) +#if defined(__CUDA_ARCH__) && (defined(_MSC_VER) || \ + (defined(__CUDACC_VER_MAJOR__) && defined(__CUDACC_VER_MINOR__) && \ + (__CUDACC_VER_MAJOR__ * 1000 + __CUDACC_VER_MINOR__) < 9002 && \ + (defined(__clang__) || defined(__ICC) || defined(__ICL)))) namespace detail { diff --git a/include/ginkgo/core/base/math.hpp b/include/ginkgo/core/base/math.hpp index 1ef12c1e23e..e993b18d240 100644 --- a/include/ginkgo/core/base/math.hpp +++ b/include/ginkgo/core/base/math.hpp @@ -518,8 +518,12 @@ GKO_INLINE GKO_ATTRIBUTES constexpr T get_superior_power( // distinguishing between the CUDA `isfinite` and the `std::isfinite` when // it is put into the `gko` namespace, only enable `std::isfinite` when // compiling host code. -using std::isfinite; // use the optimized function for all supported types - +template +GKO_INLINE GKO_ATTRIBUTES xstd::enable_if_t::value, bool> +isfinite(const T &value) +{ + return std::isfinite(value); +} #endif // defined(__CUDA_ARCH__) @@ -535,7 +539,7 @@ using std::isfinite; // use the optimized function for all supported types * returns `true` if both components of the given value are finite, meaning * they are neither +/- infinity nor NaN. */ -template = nullptr> +template GKO_INLINE GKO_ATTRIBUTES xstd::enable_if_t::value, bool> isfinite(const T &value) { diff --git a/third_party/gflags/CMakeLists.txt b/third_party/gflags/CMakeLists.txt index 410da8599c0..7d407e5b594 100644 --- a/third_party/gflags/CMakeLists.txt +++ b/third_party/gflags/CMakeLists.txt @@ -1,4 +1,4 @@ -if(MSVC AND NOT BUILD_SHARED_LIBS) +if(MSVC) # cmake links dynamic runtime libraries by default in Visual Studio # use the ginkgo's flags to use the same runtime libraries as ginkgo ginkgo_load_git_package(gflags_external From e2570296e06ae346742cb6fa610fae986c4eb30a Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Tue, 24 Sep 2019 14:25:23 +0200 Subject: [PATCH 09/18] fix linux link and can be divided by 0.0 in MSVC --- core/test/base/math.cpp | 7 ++++--- cuda/CMakeLists.txt | 12 +++++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/core/test/base/math.cpp b/core/test/base/math.cpp index ada5d95d5b0..efe0a05943b 100644 --- a/core/test/base/math.cpp +++ b/core/test/base/math.cpp @@ -49,7 +49,8 @@ void test_real_isfinite() { using limits = std::numeric_limits; constexpr auto inf = limits::infinity(); - + // Use volatile to avoid MSVC report divided by zero. + volatile const T zero{0}; ASSERT_TRUE(gko::isfinite(T{0})); ASSERT_TRUE(gko::isfinite(-T{0})); ASSERT_TRUE(gko::isfinite(T{1})); @@ -60,8 +61,8 @@ void test_real_isfinite() ASSERT_FALSE(gko::isfinite(inf - inf)); // results in nan ASSERT_FALSE(gko::isfinite(inf / inf)); // results in nan ASSERT_FALSE(gko::isfinite(inf * T{2})); // results in inf - ASSERT_FALSE(gko::isfinite(T{1} / T{0})); // results in inf - ASSERT_FALSE(gko::isfinite(T{0} / T{0})); // results in nan + ASSERT_FALSE(gko::isfinite(T{1} / zero)); // results in inf + ASSERT_FALSE(gko::isfinite(T{0} / zero)); // results in nan } diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index 410a41ed566..bef23d9dda7 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -33,23 +33,29 @@ set(CMAKE_CUDA_FLAGS "${CMAKE_CUDA_FLAGS}" PARENT_SCOPE) set(CMAKE_CUDA_COMPILER_VERSION ${CMAKE_CUDA_COMPILER_VERSION} PARENT_SCOPE) set(CUDA_INCLUDE_DIRS ${CUDA_INCLUDE_DIRS} PARENT_SCOPE) +add_library(ginkgo_cuda $ "") + # nvcc uses static cudartlibrary by default -# add `-cudart shared` into CMAKE_CUDA_FLAGS to force nvcc to use dynamic cudart library. +# add `-cudart shared` or `-cudart=shared` according system into CMAKE_CUDA_FLAGS/GINKGO_CUDA_COMPILER_FLAGS +# to force nvcc to use dynamic cudart library. find_library(CUDA_RUNTIME_LIBS_DYNAMIC cudart HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) find_library(CUDA_RUNTIME_LIBS_STATIC cudart_static HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) -if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart shared") +if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart(=| )shared" OR "${GINKGO_CUDA_COMPILER_FLAGS}" MATCHES "-cudart(=| )shared") set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_DYNAMIC}" CACHE STRING "Path to a library" FORCE) else() set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC}" CACHE STRING "Path to a library" FORCE) + if(NOT MSVC) + # link cudart_static need rt, pthread, and dl + set_property(TARGET ginkgo_cuda PROPERTY INTERFACE_LINK_LIBRARIES rt pthread dl) + endif() endif() find_library(CUBLAS cublas HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) find_library(CUSPARSE cusparse HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) -add_library(ginkgo_cuda $ "") target_sources(ginkgo_cuda PRIVATE base/exception.cpp From 61ce61ba58fa45782895e91b89781ad8aa971d88 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Tue, 24 Sep 2019 16:01:58 +0200 Subject: [PATCH 10/18] add cygwin predefined macro --- CMakeLists.txt | 2 +- cmake/build_helpers.cmake | 4 +++- include/ginkgo/core/base/types.hpp | 4 ++-- reference/test/stop/combined.cpp | 4 ++-- reference/test/stop/time.cpp | 4 ++-- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c63000520fd..8b786ff63fe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -37,7 +37,7 @@ option(GINKGO_CUDA_DEFAULT_HOST_COMPILER "Tell Ginkgo to not automatically set t option(GINKGO_JACOBI_FULL_OPTIMIZATIONS "Use all the optimizations for the CUDA Jacobi algorithm" OFF) option(BUILD_SHARED_LIBS "Build shared (.so, .dylib, .dll) libraries" ON) -if(BUILD_SHARED_LIBS AND WIN32 AND (GINKGO_BUILD_TESTS OR GINKGO_BUILD_EXAMPLES OR GINKGO_BUILD_BENCHMARKS)) +if(BUILD_SHARED_LIBS AND (WIN32 OR CYGWIN) AND (GINKGO_BUILD_TESTS OR GINKGO_BUILD_EXAMPLES OR GINKGO_BUILD_BENCHMARKS)) # Change shared libraries output only if this build has executable program with shared libraries. set(GINKGO_CHANGED_SHARED_LIBRARY TRUE) option(GINKGO_CHECK_PATH "Tell Ginkgo to check if the environment variable PATH is available for this build." ON) diff --git a/cmake/build_helpers.cmake b/cmake/build_helpers.cmake index 8056580e3ad..4de582c665b 100644 --- a/cmake/build_helpers.cmake +++ b/cmake/build_helpers.cmake @@ -40,7 +40,9 @@ function(ginkgo_compile_features name) endfunction() function(ginkgo_check_shared_library name) - set(PATH_LIST $ENV{PATH}) + # Cygwin uses : not ; to split path + string(REPLACE ":" ";" ENV_PATH "$ENV{PATH}") + set(PATH_LIST ${ENV_PATH}) set(PASSED_TEST FALSE) foreach(ITEM IN LISTS PATH_LIST) string(REPLACE "\\" "/" ITEM "${ITEM}") diff --git a/include/ginkgo/core/base/types.hpp b/include/ginkgo/core/base/types.hpp index 703594cc3d9..4c72c5f1a8c 100644 --- a/include/ginkgo/core/base/types.hpp +++ b/include/ginkgo/core/base/types.hpp @@ -78,11 +78,11 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // Handle deprecated notices correctly on different systems -#if defined(_WIN32) +#if defined(_WIN32) || defined(__CYGWIN__) #define GKO_DEPRECATED(msg) __declspec(deprecated(msg)) #else #define GKO_DEPRECATED(msg) __attribute__((deprecated(msg))) -#endif // defined(_WIN32) +#endif // defined(_WIN32) || defined(__CYGWIN__) namespace gko { diff --git a/reference/test/stop/combined.cpp b/reference/test/stop/combined.cpp index 6e7d0db0c73..9f8629b0068 100644 --- a/reference/test/stop/combined.cpp +++ b/reference/test/stop/combined.cpp @@ -40,7 +40,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include -#ifdef _WIN32 +#if defined(_WIN32) || defined(__CYGWIN__) #include #endif @@ -56,7 +56,7 @@ using double_seconds = std::chrono::duration; inline void sleep_millisecond(unsigned int ms) { -#ifdef _WIN32 +#if defined(_WIN32) || defined(__CYGWIN__) Sleep(ms); #else std::this_thread::sleep_for(std::chrono::milliseconds(ms)); diff --git a/reference/test/stop/time.cpp b/reference/test/stop/time.cpp index 4db9ddccf7b..8d47b90ff2a 100644 --- a/reference/test/stop/time.cpp +++ b/reference/test/stop/time.cpp @@ -36,7 +36,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include -#ifdef _WIN32 +#if defined(_WIN32) || defined(__CYGWIN__) #include #endif @@ -51,7 +51,7 @@ using double_seconds = std::chrono::duration; inline void sleep_millisecond(unsigned int ms) { -#ifdef _WIN32 +#if defined(_WIN32) || defined(__CYGWIN__) Sleep(ms); #else std::this_thread::sleep_for(std::chrono::milliseconds(ms)); From c8f2b0423ccb551a0a5faa14385edaff56a661fb Mon Sep 17 00:00:00 2001 From: yhmtsai Date: Tue, 24 Sep 2019 22:44:46 +0200 Subject: [PATCH 11/18] try link cudart static in linux --- benchmark/spmv/CMakeLists.txt | 5 +++++ cuda/CMakeLists.txt | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/benchmark/spmv/CMakeLists.txt b/benchmark/spmv/CMakeLists.txt index 0653bb8b509..9cb437dcfe8 100644 --- a/benchmark/spmv/CMakeLists.txt +++ b/benchmark/spmv/CMakeLists.txt @@ -2,6 +2,11 @@ add_executable(spmv spmv.cpp) target_link_libraries(spmv ginkgo gflags rapidjson) if (GINKGO_BUILD_CUDA) target_compile_definitions(spmv PRIVATE HAS_CUDA=1) + if("${CUDA_RUNTIME_LIBS}" MATCHES "Threads::Threads") + # need to import Threads + set(THREADS_PREFER_PTHREAD_FLAG ON) + find_package(Threads REQUIRED) + endif() target_link_libraries(spmv ginkgo ${CUDA_RUNTIME_LIBS} ${CUBLAS} ${CUSPARSE}) target_include_directories(spmv SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS}) diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index bef23d9dda7..a7329f704ad 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -33,7 +33,6 @@ set(CMAKE_CUDA_FLAGS "${CMAKE_CUDA_FLAGS}" PARENT_SCOPE) set(CMAKE_CUDA_COMPILER_VERSION ${CMAKE_CUDA_COMPILER_VERSION} PARENT_SCOPE) set(CUDA_INCLUDE_DIRS ${CUDA_INCLUDE_DIRS} PARENT_SCOPE) -add_library(ginkgo_cuda $ "") # nvcc uses static cudartlibrary by default # add `-cudart shared` or `-cudart=shared` according system into CMAKE_CUDA_FLAGS/GINKGO_CUDA_COMPILER_FLAGS @@ -42,13 +41,16 @@ find_library(CUDA_RUNTIME_LIBS_DYNAMIC cudart HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) find_library(CUDA_RUNTIME_LIBS_STATIC cudart_static HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) + if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart(=| )shared" OR "${GINKGO_CUDA_COMPILER_FLAGS}" MATCHES "-cudart(=| )shared") set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_DYNAMIC}" CACHE STRING "Path to a library" FORCE) else() set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC}" CACHE STRING "Path to a library" FORCE) if(NOT MSVC) + set(THREADS_PREFER_PTHREAD_FLAG ON) + find_package(Threads REQUIRED) # link cudart_static need rt, pthread, and dl - set_property(TARGET ginkgo_cuda PROPERTY INTERFACE_LINK_LIBRARIES rt pthread dl) + set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC};rt;Threads::Threads;-Wl,--no-as-needed;dl" CACHE STRING "Path to a library" FORCE) endif() endif() find_library(CUBLAS cublas @@ -56,6 +58,7 @@ find_library(CUBLAS cublas find_library(CUSPARSE cusparse HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) +add_library(ginkgo_cuda $ "") target_sources(ginkgo_cuda PRIVATE base/exception.cpp From 2f9ed8264ed150e857c4f7d74ab129f7b3d67ce0 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Wed, 25 Sep 2019 17:02:56 +0200 Subject: [PATCH 12/18] use struct::value not constexpr func in enable_if --- cmake/build_helpers.cmake | 6 +++++- core/base/mtx_io.cpp | 16 +++++++-------- core/test/utils/matrix_generator.hpp | 10 ++++------ include/ginkgo/core/base/matrix_data.hpp | 10 ++++------ include/ginkgo/core/base/utils.hpp | 25 ++++++++---------------- 5 files changed, 29 insertions(+), 38 deletions(-) diff --git a/cmake/build_helpers.cmake b/cmake/build_helpers.cmake index 4de582c665b..450bef6cca8 100644 --- a/cmake/build_helpers.cmake +++ b/cmake/build_helpers.cmake @@ -41,7 +41,11 @@ endfunction() function(ginkgo_check_shared_library name) # Cygwin uses : not ; to split path - string(REPLACE ":" ";" ENV_PATH "$ENV{PATH}") + if(CYGWIN) + string(REPLACE ":" ";" ENV_PATH "$ENV{PATH}") + else() + set(ENV_PATH "$ENV{PATH}") + endif() set(PATH_LIST ${ENV_PATH}) set(PASSED_TEST FALSE) foreach(ITEM IN LISTS PATH_LIST) diff --git a/core/base/mtx_io.cpp b/core/base/mtx_io.cpp index 689991f217d..26995be0b4d 100644 --- a/core/base/mtx_io.cpp +++ b/core/base/mtx_io.cpp @@ -162,16 +162,16 @@ class mtx_io { } private: - template = nullptr> - static xstd::enable_if_t()> write_entry_impl( + template + static xstd::enable_if_t::value> write_entry_impl( std::ostream &, const T &) { throw GKO_STREAM_ERROR( "trying to write a complex matrix into a real entry format"); } - template = nullptr> - static xstd::enable_if_t()> write_entry_impl( + template + static xstd::enable_if_t::value> write_entry_impl( std::ostream &os, const T &value) { GKO_CHECK_STREAM(os << static_cast(value), @@ -209,8 +209,8 @@ class mtx_io { } private: - template = nullptr> - static xstd::enable_if_t(), T> read_entry_impl( + template + static xstd::enable_if_t::value, T> read_entry_impl( std::istream &is) { using real_type = remove_complex; @@ -221,8 +221,8 @@ class mtx_io { return {static_cast(real), static_cast(imag)}; } - template = nullptr> - static xstd::enable_if_t(), T> read_entry_impl( + template + static xstd::enable_if_t::value, T> read_entry_impl( std::istream &) { throw GKO_STREAM_ERROR( diff --git a/core/test/utils/matrix_generator.hpp b/core/test/utils/matrix_generator.hpp index bd04e2bd992..fa994bf3f4e 100644 --- a/core/test/utils/matrix_generator.hpp +++ b/core/test/utils/matrix_generator.hpp @@ -51,18 +51,16 @@ namespace test { namespace detail { -template = nullptr> -typename std::enable_if(), ValueType>::type +template +typename std::enable_if::value, ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { return dist(gen); } -template = nullptr> -typename std::enable_if(), ValueType>::type +template +typename std::enable_if::value, ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { return ValueType(dist(gen), dist(gen)); diff --git a/include/ginkgo/core/base/matrix_data.hpp b/include/ginkgo/core/base/matrix_data.hpp index e1da8106089..7513dcdc0d8 100644 --- a/include/ginkgo/core/base/matrix_data.hpp +++ b/include/ginkgo/core/base/matrix_data.hpp @@ -63,18 +63,16 @@ struct input_triple { }; -template = nullptr> -typename std::enable_if(), ValueType>::type +template +typename std::enable_if::value, ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { return dist(gen); } -template = nullptr> -typename std::enable_if(), ValueType>::type +template +typename std::enable_if::value, ValueType>::type get_rand_value(Distribution &&dist, Generator &&gen) { return ValueType(dist(gen), dist(gen)); diff --git a/include/ginkgo/core/base/utils.hpp b/include/ginkgo/core/base/utils.hpp index d9023fa5150..11434a30b9c 100644 --- a/include/ginkgo/core/base/utils.hpp +++ b/include/ginkgo/core/base/utils.hpp @@ -119,10 +119,13 @@ struct have_ownership_impl> : std::true_type {}; template struct have_ownership_impl> : std::true_type {}; +template +using have_ownership_s = have_ownership_impl::type>; + template constexpr bool have_ownership() { - return have_ownership_impl::type>::value; + return have_ownership_s::value; } @@ -233,18 +236,6 @@ inline typename std::remove_reference::type &&give( } -/** - * Create an overload function. - * - * This is a helper function which makes the MSVC can recognize - * enable_if function. - * - * @param Index the index of overload function. - */ -template -using overload = std::integral_constant *; - - /** * Returns a non-owning (plain) pointer to the object pointed to by `p`. * @@ -255,8 +246,8 @@ using overload = std::integral_constant *; * @note This is the overload for owning (smart) pointers, that behaves the * same as calling .get() on the smart pointer. */ -template = nullptr> -inline typename std::enable_if(), +template +inline typename std::enable_if::value, detail::pointee *>::type lend(const Pointer &p) { @@ -273,8 +264,8 @@ lend(const Pointer &p) * @note This is the overload for non-owning (plain) pointers, that just * returns `p`. */ -template = nullptr> -inline typename std::enable_if(), +template +inline typename std::enable_if::value, detail::pointee *>::type lend(const Pointer &p) { From c1003a80f707d798a68d0c3ef3077e8b76a28eea Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Wed, 25 Sep 2019 17:19:30 +0200 Subject: [PATCH 13/18] enable generate_ginkgo_header when has bash --- CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8b786ff63fe..7aac29b9a50 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -156,12 +156,16 @@ if(GINKGO_DEVEL_TOOLS) COMMAND ${Ginkgo_SOURCE_DIR}/dev_tools/scripts/add_license.sh WORKING_DIRECTORY ${Ginkgo_SOURCE_DIR}) add_dependencies(format add_license) +endif() - # Generate the global `ginkgo/ginkgo.hpp` header with every call of make +# Generate the global `ginkgo/ginkgo.hpp` header with every call of make +find_program(BASH bash) +if(NOT "${BASH}" STREQUAL "BASH-NOTFOUND") add_custom_target(generate_ginkgo_header ALL COMMAND ${Ginkgo_SOURCE_DIR}/dev_tools/scripts/update_ginkgo_header.sh WORKING_DIRECTORY ${Ginkgo_SOURCE_DIR}) endif() +unset(BASH CACHE) # Installation From 73deac15684e45bbe35fd1876b641d8ffb1da78c Mon Sep 17 00:00:00 2001 From: yhmtsai Date: Tue, 1 Oct 2019 20:45:44 +0200 Subject: [PATCH 14/18] add test read complex into real format --- core/test/base/mtx_io.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/core/test/base/mtx_io.cpp b/core/test/base/mtx_io.cpp index e8b41710a96..aa733b74781 100644 --- a/core/test/base/mtx_io.cpp +++ b/core/test/base/mtx_io.cpp @@ -39,6 +39,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +#include #include @@ -256,6 +257,22 @@ TEST(MtxReader, ReadsSparseComplexHermitianMtx) } +TEST(MtxReader, FailsWhenReadingSparseComplexMtxToRealMtx) +{ + using cpx = std::complex; + using tpl = gko::matrix_data::nonzero_type; + std::istringstream iss( + "%%MatrixMarket matrix coordinate complex general\n" + "2 3 4\n" + "1 1 1.0 2.0\n" + "2 2 5.0 3.0\n" + "1 2 3.0 1.0\n" + "1 3 2.0 4.0\n"); + + ASSERT_THROW((gko::read_raw(iss)), gko::StreamError); +} + + TEST(MatrixData, WritesRealMatrixToMatrixMarketArray) { // clang-format off From e34c1814b5b45278ad0ff7cfbf972d3567968d9f Mon Sep 17 00:00:00 2001 From: yhmtsai Date: Wed, 2 Oct 2019 18:10:46 +0200 Subject: [PATCH 15/18] update README and INSTALL --- INSTALL.md | 8 ++++++++ README.md | 25 ++++++++++++++++++++---- cmake/build_helpers.cmake | 2 +- cuda/base/math.hpp | 9 +++++---- reference/test/matrix/hybrid_kernels.cpp | 2 +- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index a7026e803f3..cc877ca63e5 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -81,6 +81,14 @@ Ginkgo adds the following additional switches to control what is being built: this option see the [`ARCHITECTURES` specification list](https://github.com/ginkgo-project/CudaArchitectureSelector/blob/master/CudaArchitectureSelector.cmake#L58) section in the documentation of the CudaArchitectureSelector CMake module. +* `-DGINKGO_WINDOWS_SHARED_LIBRARY_RELPATH=` where is a relative + path with `PROJECT_BINARY_DIR`. Users must to add the absoulte path + (`PROJECT_BINARY_DIR`/`GINKGO_WINDOWS_SHARED_LIBRARY_RELPATH`) into the + environment variable PATH when building shared libraries and executable + program, default is `windows_shared_library`. +* `-DGINKGO_CHECK_PATH={ON, OFF}` checks the environment variable PATH is valid. + It is valid only when building shared libraries and executable program, + default is `ON` For example, to build everything (in debug mode), use: diff --git a/README.md b/README.md index f7a6be43387..ae7f2fc0c87 100644 --- a/README.md +++ b/README.md @@ -53,10 +53,27 @@ following: ### Windows -Windows is currently not supported, but we are working on porting the library -there. If you are interested in helping us with this effort, feel free to -contact one of the developers. (The library itself doesn't use any non-standard -C++ features, so most of the effort here is in modifying the build system.) +The prequirement needs to be verified +* _cmake 3.9+_ +* C++11 compliant 64-bits compiler: + * _MinGW : gcc 5.3+, 6.3+, 7.3+, 8.1+_ + * _CygWin : gcc 5.3+, 6.3+, 7.3+, 8.1+_ + * _Microsoft Visual Studio : VS 2017 15.7+_ + +__NOTE:__ Need to add `--autocrlf=input` after `git clone` in _CygWin_. + +The Ginkgo CUDA module has the following __additional__ requirements: + +* _CUDA 9.0+_ +* _Microsoft Visual Studio_ +* Any host compiler restrictions your version of CUDA may impose also apply + here. For the newest CUDA version, this information can be found in the + [CUDA installation guide for Windows](https://docs.nvidia.com/cuda/cuda-installation-guide-microsoft-windows/index.html) + +The Ginkgo OMP module has the following __additional__ requirements: +* _MinGW_ or _Cygwin_ + +__NOTE:__ _Microsoft Visual Studio_ only supports OpenMP 2.0, so it can not compile the ginkgo OMP module. __NOTE:__ Some restrictions will also apply on the version of C and C++ standard libraries installed on the system. This needs further investigation. diff --git a/cmake/build_helpers.cmake b/cmake/build_helpers.cmake index 450bef6cca8..8641a85a4a5 100644 --- a/cmake/build_helpers.cmake +++ b/cmake/build_helpers.cmake @@ -18,7 +18,7 @@ function(ginkgo_compile_features name) set_property(TARGET "${name}" PROPERTY CXX_INCLUDE_WHAT_YOU_USE ${GINKGO_IWYU_PATH}) endif() if(GINKGO_CHANGED_SHARED_LIBRARY) - # Put all shared libraries and corresponding imported libraries into the specficed path + # Put all shared libraries and corresponding imported libraries into the specified path set_property(TARGET "${name}" PROPERTY RUNTIME_OUTPUT_DIRECTORY "${GINKGO_WINDOWS_SHARED_LIBRARY_PATH}") set_property(TARGET "${name}" PROPERTY diff --git a/cuda/base/math.hpp b/cuda/base/math.hpp index b94351b7595..bb425214a78 100644 --- a/cuda/base/math.hpp +++ b/cuda/base/math.hpp @@ -99,10 +99,11 @@ __device__ GKO_INLINE std::complex one>() // as a __device__ function, so it results in a compiler error. // Here, `isfinite` is written by hand, which might not be as performant as the // intrinsic function from CUDA, but it compiles and works. -#if defined(__CUDA_ARCH__) && (defined(_MSC_VER) || \ - (defined(__CUDACC_VER_MAJOR__) && defined(__CUDACC_VER_MINOR__) && \ - (__CUDACC_VER_MAJOR__ * 1000 + __CUDACC_VER_MINOR__) < 9002 && \ - (defined(__clang__) || defined(__ICC) || defined(__ICL)))) +#if defined(__CUDA_ARCH__) && \ + (defined(_MSC_VER) || \ + (defined(__CUDACC_VER_MAJOR__) && defined(__CUDACC_VER_MINOR__) && \ + (__CUDACC_VER_MAJOR__ * 1000 + __CUDACC_VER_MINOR__) < 9002 && \ + (defined(__clang__) || defined(__ICC) || defined(__ICL)))) namespace detail { diff --git a/reference/test/matrix/hybrid_kernels.cpp b/reference/test/matrix/hybrid_kernels.cpp index 1096b8cb6e9..c2512c5afd9 100644 --- a/reference/test/matrix/hybrid_kernels.cpp +++ b/reference/test/matrix/hybrid_kernels.cpp @@ -96,7 +96,7 @@ class Hybrid : public ::testing::Test { auto v = m->get_const_values(); auto c = m->get_const_col_idxs(); auto r = m->get_const_row_ptrs(); - + ASSERT_EQ(m->get_size(), gko::dim<2>(2, 3)); ASSERT_EQ(m->get_num_stored_elements(), 4); EXPECT_EQ(r[0], 0); From d012d50e6d7aebc73dd16375d13c55f33a23e97d Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Tue, 15 Oct 2019 12:14:17 +0200 Subject: [PATCH 16/18] keep the same cudart behaviour as the previous one --- benchmark/spmv/CMakeLists.txt | 5 ----- cuda/CMakeLists.txt | 22 ++++++++++------------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/benchmark/spmv/CMakeLists.txt b/benchmark/spmv/CMakeLists.txt index 9cb437dcfe8..0653bb8b509 100644 --- a/benchmark/spmv/CMakeLists.txt +++ b/benchmark/spmv/CMakeLists.txt @@ -2,11 +2,6 @@ add_executable(spmv spmv.cpp) target_link_libraries(spmv ginkgo gflags rapidjson) if (GINKGO_BUILD_CUDA) target_compile_definitions(spmv PRIVATE HAS_CUDA=1) - if("${CUDA_RUNTIME_LIBS}" MATCHES "Threads::Threads") - # need to import Threads - set(THREADS_PREFER_PTHREAD_FLAG ON) - find_package(Threads REQUIRED) - endif() target_link_libraries(spmv ginkgo ${CUDA_RUNTIME_LIBS} ${CUBLAS} ${CUSPARSE}) target_include_directories(spmv SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS}) diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index a7329f704ad..f5e6515897b 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -34,25 +34,23 @@ set(CMAKE_CUDA_COMPILER_VERSION ${CMAKE_CUDA_COMPILER_VERSION} PARENT_SCOPE) set(CUDA_INCLUDE_DIRS ${CUDA_INCLUDE_DIRS} PARENT_SCOPE) -# nvcc uses static cudartlibrary by default +# MSVC nvcc uses static cudartlibrary by default, and other platforms use shared cudartlibrary. # add `-cudart shared` or `-cudart=shared` according system into CMAKE_CUDA_FLAGS/GINKGO_CUDA_COMPILER_FLAGS -# to force nvcc to use dynamic cudart library. +# to force nvcc to use dynamic cudart library in MSVC. find_library(CUDA_RUNTIME_LIBS_DYNAMIC cudart HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) find_library(CUDA_RUNTIME_LIBS_STATIC cudart_static HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) - -if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart(=| )shared" OR "${GINKGO_CUDA_COMPILER_FLAGS}" MATCHES "-cudart(=| )shared") - set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_DYNAMIC}" CACHE STRING "Path to a library" FORCE) -else() - set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC}" CACHE STRING "Path to a library" FORCE) - if(NOT MSVC) - set(THREADS_PREFER_PTHREAD_FLAG ON) - find_package(Threads REQUIRED) - # link cudart_static need rt, pthread, and dl - set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC};rt;Threads::Threads;-Wl,--no-as-needed;dl" CACHE STRING "Path to a library" FORCE) +if(MSVC) + if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart(=| )shared" OR "${GINKGO_CUDA_COMPILER_FLAGS}" MATCHES "-cudart(=| )shared") + set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_DYNAMIC}" CACHE STRING "Path to a library" FORCE) + else() + set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC}" CACHE STRING "Path to a library" FORCE) endif() +else() + set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_DYNAMIC}" CACHE STRING "Path to a library" FORCE) endif() + find_library(CUBLAS cublas HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) find_library(CUSPARSE cusparse From 65274205cd2ac0cb97209e193c5cf10ce92b43d4 Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Wed, 16 Oct 2019 09:32:20 +0200 Subject: [PATCH 17/18] reviews updating --- CMakeLists.txt | 9 +++++---- INSTALL.md | 10 +++++++--- cmake/build_helpers.cmake | 10 +++++----- cmake/package_helpers.cmake | 4 +--- cuda/CMakeLists.txt | 4 ++-- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7aac29b9a50..71e4cc2c682 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -112,12 +112,12 @@ configure_file(${Ginkgo_SOURCE_DIR}/include/ginkgo/config.hpp.in # This is modified from https://gitlab.kitware.com/cmake/community/wikis/FAQ#dynamic-replace if(MSVC) if(BUILD_SHARED_LIBS) - ginkgo_turn_to_windows_dynamic("CXX") - ginkgo_turn_to_windows_dynamic("C") + ginkgo_switch_to_windows_dynamic("CXX") + ginkgo_switch_to_windows_dynamic("C") set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS TRUE) else() - ginkgo_turn_to_windows_static("CXX") - ginkgo_turn_to_windows_static("C") + ginkgo_switch_to_windows_static("CXX") + ginkgo_switch_to_windows_static("C") set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS FALSE) endif() endif() @@ -128,6 +128,7 @@ ginkgo_find_package(GTest "GTest::GTest;GTest::Main" FALSE 1.8.1) ginkgo_find_package(gflags gflags FALSE 2.2.2) ginkgo_find_package(RapidJSON rapidjson TRUE 1.1.0) add_subdirectory(third_party) # Third-party tools and libraries + # Needs to be first in order for `CMAKE_CUDA_DEVICE_LINK_EXECUTABLE` to be # propagated to the other parts of Ginkgo in case of building as static libraries if(GINKGO_BUILD_CUDA) diff --git a/INSTALL.md b/INSTALL.md index cc877ca63e5..7ba95897206 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -8,6 +8,10 @@ Use the standard cmake build procedure: mkdir build; cd build cmake -G "Unix Makefiles" [OPTIONS] .. && make ``` +Use `cmake --build .` in some systems like MinGW or Microsoft Visual Studio which do not use `make`. + +For Microsoft Visual Studio, use `cmake --build . --config ` to decide the build type. The possible options are `Debug`, `Release`, `RelWithDebInfo` and `MinSizeRel`. + Replace `[OPTIONS]` with desired cmake options for your build. Ginkgo adds the following additional switches to control what is being built: @@ -82,12 +86,12 @@ Ginkgo adds the following additional switches to control what is being built: [`ARCHITECTURES` specification list](https://github.com/ginkgo-project/CudaArchitectureSelector/blob/master/CudaArchitectureSelector.cmake#L58) section in the documentation of the CudaArchitectureSelector CMake module. * `-DGINKGO_WINDOWS_SHARED_LIBRARY_RELPATH=` where is a relative - path with `PROJECT_BINARY_DIR`. Users must to add the absoulte path + path built with `PROJECT_BINARY_DIR`. Users must add the absolute path (`PROJECT_BINARY_DIR`/`GINKGO_WINDOWS_SHARED_LIBRARY_RELPATH`) into the environment variable PATH when building shared libraries and executable program, default is `windows_shared_library`. -* `-DGINKGO_CHECK_PATH={ON, OFF}` checks the environment variable PATH is valid. - It is valid only when building shared libraries and executable program, +* `-DGINKGO_CHECK_PATH={ON, OFF}` checks if the environment variable PATH is valid. + It is checked only when building shared libraries and executable program, default is `ON` For example, to build everything (in debug mode), use: diff --git a/cmake/build_helpers.cmake b/cmake/build_helpers.cmake index 8641a85a4a5..76125a761a6 100644 --- a/cmake/build_helpers.cmake +++ b/cmake/build_helpers.cmake @@ -73,7 +73,7 @@ function(ginkgo_check_shared_library name) endif() endfunction() -function(ginkgo_turn_windows_link lang from to) +function(ginkgo_switch_windows_link lang from to) foreach(flag_var "CMAKE_${lang}_FLAGS" "CMAKE_${lang}_FLAGS_DEBUG" "CMAKE_${lang}_FLAGS_RELEASE" "CMAKE_${lang}_FLAGS_MINSIZEREL" "CMAKE_${lang}_FLAGS_RELWITHDEBINFO" @@ -88,10 +88,10 @@ function(ginkgo_turn_windows_link lang from to) endforeach() endfunction() -macro(ginkgo_turn_to_windows_static lang) - ginkgo_turn_windows_link(${lang} "MD" "MT") +macro(ginkgo_switch_to_windows_static lang) + ginkgo_switch_windows_link(${lang} "MD" "MT") endmacro() -macro(ginkgo_turn_to_windows_dynamic lang) - ginkgo_turn_windows_link(${lang} "MT" "MD") +macro(ginkgo_switch_to_windows_dynamic lang) + ginkgo_switch_windows_link(${lang} "MT" "MD") endmacro() \ No newline at end of file diff --git a/cmake/package_helpers.cmake b/cmake/package_helpers.cmake index 67414aa48b1..74208b86e09 100644 --- a/cmake/package_helpers.cmake +++ b/cmake/package_helpers.cmake @@ -119,9 +119,7 @@ macro(ginkgo_add_external_target new_target external_name includedir libdir buil # Since we do not really manage other build types, let's globally use the DEBUG symbols if(MSVC) # Only Debug build uses MDd or MTd, and others use MD or MT. - # MSVC would like to use same runtime library, so we use Debug third-party in Debug and Release third-party in Release. - set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION_DEBUG - ${${external_name}_LIBRARY_DEBUG}) + # MSVC would like to use same runtime library, so we use Debug third-party in Debug and Release third-party in others. set_target_properties(${new_target} PROPERTIES IMPORTED_LOCATION ${${external_name}_LIBRARY_RELEASE}) else() diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index f5e6515897b..8ad05f58386 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -17,9 +17,9 @@ endif() # This is modified from https://gitlab.kitware.com/cmake/community/wikis/FAQ#dynamic-replace if(MSVC) if(BUILD_SHARED_LIBS) - ginkgo_turn_to_windows_dynamic("CUDA") + ginkgo_switch_to_windows_dynamic("CUDA") else() - ginkgo_turn_to_windows_static("CUDA") + ginkgo_switch_to_windows_static("CUDA") endif() endif() From 570aa8c88e4bc20d0c460e3c7c95948391c2c4db Mon Sep 17 00:00:00 2001 From: "Yuhsiang M. Tsai" Date: Thu, 17 Oct 2019 15:28:22 +0200 Subject: [PATCH 18/18] only handle cudart option in CMAKE_CUDA_FLAGS --- cuda/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cuda/CMakeLists.txt b/cuda/CMakeLists.txt index 8ad05f58386..142ed0506fb 100644 --- a/cuda/CMakeLists.txt +++ b/cuda/CMakeLists.txt @@ -35,14 +35,14 @@ set(CUDA_INCLUDE_DIRS ${CUDA_INCLUDE_DIRS} PARENT_SCOPE) # MSVC nvcc uses static cudartlibrary by default, and other platforms use shared cudartlibrary. -# add `-cudart shared` or `-cudart=shared` according system into CMAKE_CUDA_FLAGS/GINKGO_CUDA_COMPILER_FLAGS +# add `-cudart shared` or `-cudart=shared` according system into CMAKE_CUDA_FLAGS # to force nvcc to use dynamic cudart library in MSVC. find_library(CUDA_RUNTIME_LIBS_DYNAMIC cudart HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) find_library(CUDA_RUNTIME_LIBS_STATIC cudart_static HINT ${CMAKE_CUDA_IMPLICIT_LINK_DIRECTORIES}) if(MSVC) - if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart(=| )shared" OR "${GINKGO_CUDA_COMPILER_FLAGS}" MATCHES "-cudart(=| )shared") + if("${CMAKE_CUDA_FLAGS}" MATCHES "-cudart(=| )shared") set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_DYNAMIC}" CACHE STRING "Path to a library" FORCE) else() set(CUDA_RUNTIME_LIBS "${CUDA_RUNTIME_LIBS_STATIC}" CACHE STRING "Path to a library" FORCE)