Review update.

+ Change behavior and name of the ginkgo_core_device library (to
  ginkgo_device).
+ Update the documentation in INSTALL.md and README.md and in
  cmake/install_helpers.cmake.
+ Fix some code style issues.

Co-authored-by: Yuhsiang Tsai <yhmtsai@gmail.com>
Co-authored-by: Pratik Nayak <pratikvn@protonmail.com>
Co-authored-by: Tobias Ribizel <ribizel@kit.edu>

INSTALL.md

@@ -192,14 +192,21 @@ Ginkgo's HIP backend adds a dependency to the following packages:
 All HIP installation paths can be configured through the use of environment
 variables or CMake variables. This way of configuring the paths is currently
 imposed by the `HIP` tool suite. The variables are the following:
-+ CMake `-DHIP_PATH=` or  environment `export HIP_PATH=`: sets the `HIP`
-  installation path. The default value is `/opt/rocm/hip`.
-+ CMake `-DHIPBLAS_PATH=` or  environment `export HIPBLAS_PATH=`: sets the
-  `hipBLAS` installation path. The default value is `/opt/rocm/hipblas`.
-+ CMake `-DHIPSPARSE_PATH=` or  environment `export HIPSPARSE_PATH=`: sets the
-  `hipSPARSE` installation path. The default value is `/opt/rocm/hipsparse`.
-+ CMake `-DHCC_PATH=` or  environment `export HCC_PATH=`: sets the `HCC`
-  installation path, for AMD backends. The default value is `/opt/rocm/hcc`.
++ CMake `-DROCM_PATH=` or environment `export ROCM_PATH=`: sets the `ROCM`
+  installation path. The default value is `/opt/rocm/`.
++ CMake `-DHIP_CLANG_PATH` or environment `export HIP_CLANG_PATH=`: sets the
+  `HIP` compatible `clang` binary path. The default value is
+  `${ROCM_PATH}/llvm/bin`.
++ CMake `-DHIP_PATH=` or environment `export HIP_PATH=`: sets the `HIP`
+  installation path. The default value is `${ROCM_PATH}/hip`.
++ CMake `-DHIPBLAS_PATH=` or environment `export HIPBLAS_PATH=`: sets the
+  `hipBLAS` installation path. The default value is `${ROCM_PATH}/hipblas`.
++ CMake `-DHIPSPARSE_PATH=` or environment `export HIPSPARSE_PATH=`: sets the
+  `hipSPARSE` installation path. The default value is `${ROCM_PATH}/hipsparse`.
++ CMake `-DROCRAND_PATH=` or environment `export ROCRAND_PATH=`: sets the
+  `rocRAND` installation path. The default value is `${ROCM_PATH}/rocrand`.
++ CMake `-DHIPRAND_PATH=` or environment `export HIPRAND_PATH=`: sets the
+  `hipRAND` installation path. The default value is `${ROCM_PATH}/hiprand`.
 + environment `export CUDA_PATH=`: where `hipcc` can find `CUDA` if it is not in
   the default `/usr/local/cuda` path.
 
@@ -216,9 +223,9 @@ export HIP_PLATFORM=nvcc
 ```
 
 When using `HIP_PLATFORM=hcc`, note that two `HIP` compilers can be set, the old
-`hcc` or since ROCm 3.5, `clang`. On newer installations, `clang` should be set
-by default, but if encountering any problem try to manually set the `HIP`
-compiler to `clang`:
+`hcc` or since ROCm 3.5, `clang`. Ginkgo is only compatible with the `clang`
+based installations. Although this setting should be automatically done, it is
+also possible to manually set the `HIP` compiler to `clang`:
 ```
 export HIP_COMPILER=clang
 ```
@@ -227,7 +234,6 @@ export HIP_COMPILER=clang
 Platform specific compilation flags can be given through the following
 CMake variables:
 + `-DGINKGO_HIP_COMPILER_FLAGS=`: compilation flags given to all platforms.
-+ `-DGINKGO_HIP_HCC_COMPILER_FLAGS=`: compilation flags given to AMD platforms.
 + `-DGINKGO_HIP_NVCC_COMPILER_FLAGS=`: compilation flags given to NVIDIA platforms.
 + `-DGINKGO_HIP_CLANG_COMPILER_FLAGS=`: compilation flags given to AMD clang compiler.
 
@@ -257,7 +263,9 @@ packages `GTEST`, `GFLAGS`, `RAPIDJSON` and `CAS`, it is possible to force
 Ginkgo to try to use an external version of a package. For this, Ginkgo provides
 two ways to find packages. To rely on the CMake `find_package` command, use the
 CMake option `-DGINKGO_USE_EXTERNAL_<package>=ON`. `HWLOC` works the opposite
-way, Ginkgo always looks for the system's `hwloc` first.
+way, Ginkgo always looks for the system's `hwloc` first. In addition, when
+installing Ginkgo, `hwloc` will be installed as well into the Ginkgo directory
+if there were no system `hwloc` detected.
 
 Note that, if the external packages were not installed to the default location,
 the CMake option `-DCMAKE_PREFIX_PATH=<path-list>` needs to be set to the

README.md

@@ -61,10 +61,10 @@ following:
 
 The Ginkgo HIP module has the following __additional__ requirements:
 
-* _ROCm 2.8+_
-*    the HIP, hipBLAS and hipSPARSE packages compiled with either:
-    * _AMD_ backend
-    * _CUDA 9.0+_ backend. When using CUDA 10+, _cmake 3.12.2+_ is required.
+* _ROCm 3.5+_
+*    the HIP, hipBLAS, hipSPARSE, hip/rocRAND packages compiled with either:
+    * _AMD_ backend (using the `clang` compiler)
+    * _CUDA 9.2+_ backend. When using CUDA 10+, _cmake 3.12.2+_ is required.
 
 ### Windows
 

cmake/Modules/FindHWLOC.cmake

@@ -51,7 +51,7 @@ find_library(HWLOC_LIBRARIES "hwloc"
     )
 
 if (HWLOC_INCLUDE_DIRS)
-        # Find the version of hwloc found
+    # Find the version of hwloc found
     if(NOT HWLOC_VERSION)
         file(READ "${HWLOC_INCLUDE_DIRS}/hwloc.h"
             HEADER_CONTENTS LIMIT 16384)

cmake/install_helpers.cmake

@@ -9,6 +9,13 @@ set(GINKGO_INSTALL_CONFIG_DIR "${CMAKE_INSTALL_LIBDIR}/cmake/Ginkgo")
 set(GINKGO_INSTALL_MODULE_DIR "${CMAKE_INSTALL_LIBDIR}/cmake/Ginkgo/Modules")
 
 function(ginkgo_install_library name subdir)
+    # This setting sets the directory of the library as the `INSTALL_RPATH`.
+    # This is required for `libginkgo.so` to find its dependencies such as
+    # `libginkgo_omp.so` etc. Note that with this setting it's not possible on
+    # Linux to control which Ginkgo libraries are found and where: if all the
+    # libraries are present in the directory, these will be used first and
+    # foremost and the environment variable `LD_LIBRARY_PATH` will have no
+    # effect.
     if (BUILD_SHARED_LIBS)
         if (APPLE)
             set(ORIGIN_OR_LOADER_PATH "@loader_path")

core/CMakeLists.txt

@@ -62,7 +62,7 @@ target_compile_options(ginkgo PRIVATE "${GINKGO_COMPILER_FLAGS}")
 # regardless of whether it is installed or added as a subdirectory
 add_library(Ginkgo::ginkgo ALIAS ginkgo)
 target_link_libraries(ginkgo
-    PUBLIC ginkgo_core_device ginkgo_omp ginkgo_cuda ginkgo_reference ginkgo_hip ginkgo_dpcpp)
+    PUBLIC ginkgo_device ginkgo_omp ginkgo_cuda ginkgo_reference ginkgo_hip ginkgo_dpcpp)
 # The PAPI dependency needs to be exposed to the user.
 if (GINKGO_HAVE_PAPI_SDE)
     target_link_libraries(ginkgo PUBLIC PAPI::PAPI)

core/device_hooks/CMakeLists.txt

@@ -2,7 +2,7 @@ if(NOT GINKGO_BUILD_CUDA)
     add_library(ginkgo_cuda
         $<TARGET_OBJECTS:ginkgo_cuda_device>
         cuda_hooks.cpp)
-    target_link_libraries(ginkgo_cuda PRIVATE ginkgo_core_device)
+    target_link_libraries(ginkgo_cuda PUBLIC ginkgo_device)
     ginkgo_compile_features(ginkgo_cuda)
     ginkgo_default_includes(ginkgo_cuda)
     ginkgo_install_library(ginkgo_cuda cuda)
@@ -12,7 +12,7 @@ if (NOT GINKGO_BUILD_DPCPP)
     add_library(ginkgo_dpcpp
         $<TARGET_OBJECTS:ginkgo_dpcpp_device>
         dpcpp_hooks.cpp)
-    target_link_libraries(ginkgo_dpcpp PRIVATE ginkgo_core_device)
+    target_link_libraries(ginkgo_dpcpp PUBLIC ginkgo_device)
     ginkgo_compile_features(ginkgo_dpcpp)
     ginkgo_default_includes(ginkgo_dpcpp)
     ginkgo_install_library(ginkgo_dpcpp dpcpp)
@@ -22,7 +22,7 @@ if(NOT GINKGO_BUILD_HIP)
     add_library(ginkgo_hip
         $<TARGET_OBJECTS:ginkgo_hip_device>
         hip_hooks.cpp)
-    target_link_libraries(ginkgo_hip PRIVATE ginkgo_core_device)
+    target_link_libraries(ginkgo_hip PUBLIC ginkgo_device)
     ginkgo_compile_features(ginkgo_hip)
     ginkgo_default_includes(ginkgo_hip)
     ginkgo_install_library(ginkgo_hip hip)
@@ -36,7 +36,7 @@ if (NOT GINKGO_BUILD_OMP)
     target_link_libraries(ginkgo_omp PRIVATE ginkgo_cuda)
     target_link_libraries(ginkgo_omp PRIVATE ginkgo_hip)
     target_link_libraries(ginkgo_omp PRIVATE ginkgo_dpcpp)
-    target_link_libraries(ginkgo_omp PRIVATE ginkgo_core_device)
+    target_link_libraries(ginkgo_omp PUBLIC ginkgo_device)
     ginkgo_default_includes(ginkgo_omp)
     ginkgo_install_library(ginkgo_omp omp)
 endif()
@@ -45,7 +45,7 @@ if (NOT GINKGO_BUILD_REFERENCE)
     add_library(ginkgo_reference
         $<TARGET_OBJECTS:ginkgo_reference_device>
         reference_hooks.cpp)
-    target_link_libraries(ginkgo_reference PRIVATE ginkgo_core_device)
+    target_link_libraries(ginkgo_reference PUBLIC ginkgo_device)
     ginkgo_compile_features(ginkgo_reference)
     ginkgo_default_includes(ginkgo_reference)
     ginkgo_install_library(ginkgo_reference reference)

cuda/CMakeLists.txt

@@ -160,7 +160,7 @@ ginkgo_compile_features(ginkgo_cuda)
 target_include_directories(ginkgo_cuda
     SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS})
 target_link_libraries(ginkgo_cuda PRIVATE ${CUDA_RUNTIME_LIBS} ${CUBLAS} ${CUSPARSE} ${CURAND})
-target_link_libraries(ginkgo_cuda PRIVATE ginkgo_core_device)
+target_link_libraries(ginkgo_cuda PUBLIC ginkgo_device)
 target_compile_options(ginkgo_cuda
         PRIVATE "$<$<COMPILE_LANGUAGE:CUDA>:${GINKGO_CUDA_ARCH_FLAGS}>")
 

devices/CMakeLists.txt

@@ -16,8 +16,8 @@ function(ginkgo_add_library name)
     set_target_properties(${name} PROPERTIES POSITION_INDEPENDENT_CODE ON)
 endfunction()
 
-ginkgo_add_library(ginkgo_core_device machine_topology.cpp)
-ginkgo_install_library(ginkgo_core_device devices)
+ginkgo_add_library(ginkgo_device machine_topology.cpp)
+ginkgo_install_library(ginkgo_device devices)
 
 add_subdirectory(cuda)
 add_subdirectory(dpcpp)

dpcpp/CMakeLists.txt

@@ -54,7 +54,7 @@ set(GINKGO_DPCPP_FLAGS ${GINKGO_COMPILER_FLAGS} -fsycl -fsycl-device-lib=libc)
 set(GINKGO_DPCPP_FLAGS ${GINKGO_DPCPP_FLAGS} PARENT_SCOPE)
 target_compile_options(ginkgo_dpcpp PRIVATE "${GINKGO_DPCPP_FLAGS}")
 target_compile_features(ginkgo_dpcpp PRIVATE cxx_std_17)
-target_link_libraries(ginkgo_dpcpp PRIVATE ginkgo_core_device)
+target_link_libraries(ginkgo_dpcpp PUBLIC ginkgo_device)
 
 ginkgo_default_includes(ginkgo_dpcpp)
 ginkgo_install_library(ginkgo_dpcpp dpcpp)

hip/CMakeLists.txt

@@ -120,7 +120,9 @@ list(APPEND CMAKE_PREFIX_PATH
 set(CMAKE_MODULE_PATH "${CMAKE_MODULE_PATH}" PARENT_SCOPE)
 set(CMAKE_PREFIX_PATH "${CMAKE_PREFIX_PATH}" PARENT_SCOPE)
 
-# NOTE: without this, HIP jacobi build takes a *very* long time
+# NOTE: without this, HIP jacobi build takes a *very* long time. The reason for
+# that is that these variables are seemingly empty by default, thus there is no
+# proper optimization applied to the HIP builds otherwise.
 set(HIP_HIPCC_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}" CACHE STRING "Flags used by the HIPCC compiler during DEBUG builds")
 set(HIP_HIPCC_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL}" CACHE STRING "Flags used by the HIPCC compiler during MINSIZEREL builds")
 set(HIP_HIPCC_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}" CACHE STRING "Flags used by the HIPCC compiler during RELEASE builds")
@@ -247,7 +249,7 @@ target_include_directories(ginkgo_hip
         ${HIPSPARSE_INCLUDE_DIRS}
         $<BUILD_INTERFACE:${ROCPRIM_INCLUDE_DIRS}>)
 
-target_link_libraries(ginkgo_hip PRIVATE ginkgo_core_device)
+target_link_libraries(ginkgo_hip PUBLIC ginkgo_device)
 target_link_libraries(ginkgo_hip PRIVATE roc::hipblas roc::hipsparse hip::hiprand roc::rocrand)
 
 target_compile_options(ginkgo_hip PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${GINKGO_COMPILER_FLAGS}>)

omp/CMakeLists.txt

@@ -57,7 +57,7 @@ target_link_libraries(ginkgo_omp PRIVATE ginkgo_cuda)
 target_link_libraries(ginkgo_omp PRIVATE ginkgo_hip)
 # Need to link against ginkgo_dpcpp for the `raw_copy_to(DpcppExecutor ...)` method
 target_link_libraries(ginkgo_omp PRIVATE ginkgo_dpcpp)
-target_link_libraries(ginkgo_omp PRIVATE ginkgo_core_device)
+target_link_libraries(ginkgo_omp PUBLIC ginkgo_device)
 
 ginkgo_default_includes(ginkgo_omp)
 ginkgo_install_library(ginkgo_omp omp)

reference/CMakeLists.txt

@@ -39,7 +39,7 @@ target_sources(ginkgo_reference
     stop/criterion_kernels.cpp
     stop/residual_norm_kernels.cpp)
 
-target_link_libraries(ginkgo_reference PRIVATE ginkgo_core_device)
+target_link_libraries(ginkgo_reference PUBLIC ginkgo_device)
 ginkgo_compile_features(ginkgo_reference)
 ginkgo_default_includes(ginkgo_reference)
 ginkgo_install_library(ginkgo_reference reference)

test_install/test_install.cpp

@@ -129,7 +129,6 @@ int main(int, char **)
         auto test = gko::name_demangling::get_static_type(testVar);
     }
 
-
     // core/base/polymorphic_object.hpp
     {
         auto test = gko::layout_type::array;

test_install/test_install.hip.cpp

@@ -134,7 +134,6 @@ int main(int, char **)
         gko::name_demangling::get_static_type(testVar);
     }
 
-
     // core/base/polymorphic_object.hpp
     {
         gko::PolymorphicObject *test;

test_install/test_install_cuda.cu

@@ -131,7 +131,6 @@ int main(int, char **)
         gko::name_demangling::get_static_type(testVar);
     }
 
-
     // core/base/polymorphic_object.hpp
     {
         gko::PolymorphicObject *test;