Fix code and documentation issues according to the reviews.

INSTALL.md

@@ -115,7 +115,7 @@ generators. Other CMake generators are untested.
 Ginkgo provides a [HIP](https://github.com/ROCm-Developer-Tools/HIP) backend.
 This allows to compile optimized versions of the kernels for either AMD or
 NVIDIA GPUs. To build Ginkgo with this backend, use the CMake option
-`-DGINKGO_BUILD_HIP=on`.
+`-DGINKGO_BUILD_HIP=ON`.
 
 #### Correctly installing HIP toolkits and dependencies for Ginkgo
 In general, Ginkgo's HIP backend requires the following packages:
@@ -165,7 +165,7 @@ popd && popd
 # hipSPARSE
 git clone https://github.com/tcojean/hipSPARSE.git
 pushd hipSPARSE && mkdir build && pushd build
-cmake -DBUILD_CUDA=on .. && make install
+cmake -DBUILD_CUDA=ON .. && make install
 popd && popd
 ```
 

README.md

@@ -55,7 +55,7 @@ The Ginkgo HIP module has the following __additional__ requirements:
 
 * 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.
+ * _CUDA 9.0+_ backend. When using CUDA 10+, _cmake 3.12.2+_ is required.
 
 ### Windows
 

cmake/create_test.cmake

@@ -54,7 +54,7 @@ function(ginkgo_create_hip_test test_name)
  PRIVATE
  "$<BUILD_INTERFACE:${Ginkgo_BINARY_DIR}>"
 
- # Only `exception_helpers` requires thess so far, but it's much easier
+ # Only `exception_helpers` requires these so far, but it's much easier
  # to put these this way.
  ${HIPBLAS_INCLUDE_DIRS}
  ${HIPSPARSE_INCLUDE_DIRS}
@@ -75,7 +75,6 @@ function(ginkgo_create_hip_test test_name)
  if (GINKGO_HIP_PLATFORM MATCHES "hcc")
  target_link_libraries(${TEST_TARGET_NAME} PRIVATE "${GINKGO_RPATH_FOR_HIP}")
 
-
  if (GINKGO_CHECK_CIRCULAR_DEPS)
  target_link_libraries(${TEST_TARGET_NAME} PRIVATE "${GINKGO_CIRCULAR_DEPS_FLAGS}")
  endif()

core/base/mtx_io.cpp

@@ -139,6 +139,7 @@ class mtx_io {
  struct : entry_format {
  /**
  * reads entry from the input stream
+ *
  * @param is the input stream
  *
  * @return the matrix entry.
@@ -152,6 +153,7 @@ class mtx_io {
 
  /**
  * writes entry to the output stream
+ *
  * @param os the output stream
  * @param value the matrix entry to be written
  */
@@ -186,6 +188,7 @@ class mtx_io {
  struct : entry_format {
  /**
  * reads entry from the input stream
+ *
  * @param is the input stream
  *
  * @return the matrix entry.
@@ -197,6 +200,7 @@ class mtx_io {
 
  /**
  * writes entry to the output stream
+ *
  * @param os the output stream
  * @param value the matrix entry to be written
  */
@@ -237,6 +241,7 @@ class mtx_io {
  struct : entry_format {
  /**
  * reads entry from the input stream
+ *
  * @param dummy input stream
  *
  * @return the matrix entry(one).
@@ -248,6 +253,7 @@ class mtx_io {
 
  /**
  * writes entry to the output stream
+ *
  * @param dummy output stream
  * @param dummy matrix entry to be written
  */
@@ -284,6 +290,7 @@ class mtx_io {
  struct : storage_modifier {
  /**
  * get the reservation size
+ *
  * @param num_rows the number of rows
  * @param num_cols the number of columns
  * @param num_nonzeros the number of non-zeros
@@ -298,6 +305,7 @@ class mtx_io {
 
  /**
  * Insert an entry
+ *
  * @param row The row where the entry is to be inserted.
  * @param col The column where the entry is to be inserted.
  * @param entry the entry to be inserted.
@@ -337,6 +345,7 @@ class mtx_io {
 
  /**
  * Insert an entry
+ *
  * @param row The row where the entry is to be inserted.
  * @param col The column where the entry is to be inserted.
  * @param entry the entry to be inserted.
@@ -366,6 +375,7 @@ class mtx_io {
  struct : storage_modifier {
  /**
  * get the reservation size
+ *
  * @param num_rows
  * @param num_cols
  * @param num_nonzeros the number of non-zeros
@@ -380,6 +390,7 @@ class mtx_io {
 
  /**
  * Insert an entry
+ *
  * @param row The row where the entry is to be inserted.
  * @param col The column where the entry is to be inserted.
  * @param entry the entry to be inserted.
@@ -409,6 +420,7 @@ class mtx_io {
  struct : storage_modifier {
  /**
  * get the reservation size
+ *
  * @param num_rows
  * @param num_cols
  * @param num_nonzeros the number of non-zeros
@@ -423,6 +435,7 @@ class mtx_io {
 
  /**
  * Insert an entry
+ *
  * @param row The row where the entry is to be inserted.
  * @param col The column where the entry is to be inserted.
  * @param entry the entry to be inserted.
@@ -667,6 +680,7 @@ class mtx_io {
 
  /**
  * reads and parses the first line of the header
+ *
  * @param is the input stream
  *
  * @return the data containing the description
@@ -711,6 +725,7 @@ class mtx_io {
 
  /**
  * reads and parses the header
+ *
  * @param is The input stream to read the header from.
  *
  * @return the header data

cuda/CMakeLists.txt

@@ -107,7 +107,7 @@ target_include_directories(ginkgo_cuda
  SYSTEM PRIVATE ${CUDA_INCLUDE_DIRS})
 target_link_libraries(ginkgo_cuda PRIVATE ${CUDA_RUNTIME_LIBS} ${CUBLAS} ${CUSPARSE})
 
-# Need to link against ginkgo_cuda for the `raw_copy_to(HipExecutor ...)` method
+# Need to link against ginkgo_hip for the `raw_copy_to(HipExecutor ...)` method
 target_link_libraries(ginkgo_cuda PUBLIC ginkgo_hip)
 
 cas_target_cuda_architectures(ginkgo_cuda

cuda/components/uninitialized_array.hpp

@@ -42,15 +42,19 @@ namespace kernels {
 namespace cuda {
 
 
-template <typename ValueType, size_type size>
 /**
  * Stores an array with uninitialized contents.
+ *
+ * @tparam ValueType the type of values
+ * @tparam size the size of the array
  */
+template <typename ValueType, size_type size>
 class UninitializedArray {
 public:
  /**
  * Operator for casting an UninitializedArray into its constexpr value
  * pointer.
+ *
  * @return the constexpr pointer to the first entry of the array.
  */
  constexpr GKO_ATTRIBUTES operator ValueType *() const noexcept
@@ -61,14 +65,17 @@ class UninitializedArray {
  /**
  * Operator for casting an UninitializedArray into its non-const value
  * pointer.
+ *
  * @return the non-const pointer to the first entry of the array.
  */
  GKO_ATTRIBUTES operator ValueType *() noexcept { return &(*this)[0]; }
 
  /**
  * constexpr array access operator.
+ *
  * @param pos The array index. Using a value outside [0, size) is undefined
  * behavior.
+ *
  * @return a reference to the array entry at the given index.
  */
  constexpr GKO_ATTRIBUTES ValueType &operator[](size_type pos) const noexcept
@@ -78,8 +85,10 @@ class UninitializedArray {
 
  /**
  * Non-const array access operator.
+ *
  * @param pos The array index. Using a value outside [0, size) is undefined
  * behavior.
+ *
  * @return a reference to the array entry at the given index.
  */
  GKO_ATTRIBUTES ValueType &operator[](size_type pos) noexcept

examples/custom-stopping-criterion/build.sh

@@ -23,9 +23,9 @@ done
 
 # figure out correct compiler flags
 if ls ${THIS_DIR} | grep -F "libginkgo." >/dev/null; then
- LINK_FLAGS="-lpthread -lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference"
+ LINK_FLAGS="-lpthread -lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference -lginkgo_hip"
 else
- LINK_FLAGS="-lpthread -lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced"
+ LINK_FLAGS="-lpthread -lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced -lginkgo_hipd"
 fi
 if [ -z "${CXX}" ]; then
  CXX="c++"

examples/ilu-preconditioned-solver/build.sh

@@ -9,8 +9,8 @@ BUILD_DIR=$1
 THIS_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" &>/dev/null && pwd )
 
 # copy libraries
-LIBRARY_DIRS="core core/device_hooks reference omp cuda"
-LIBRARY_NAMES="ginkgo ginkgo_reference ginkgo_omp ginkgo_cuda"
+LIBRARY_DIRS="core core/device_hooks reference omp cuda hip"
+LIBRARY_NAMES="ginkgo ginkgo_reference ginkgo_omp ginkgo_cuda ginkgo_hip"
 SUFFIXES=".so .dylib .dll d.so d.dylib d.dll"
 for prefix in ${LIBRARY_DIRS}; do
  for name in ${LIBRARY_NAMES}; do
@@ -23,9 +23,9 @@ done
 
 # figure out correct compiler flags
 if ls ${THIS_DIR} | grep -F "libginkgo." >/dev/null; then
- LINK_FLAGS="-lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference"
+ LINK_FLAGS="-lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference -lginkgo_hip"
 else
- LINK_FLAGS="-lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced"
+ LINK_FLAGS="-lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced -lginkgo_hipd"
 fi
 if [ -z "${CXX}" ]; then
  CXX="c++"

examples/papi-logging/build.sh

@@ -23,9 +23,9 @@ done
 
 # figure out correct compiler flags
 if ls ${THIS_DIR} | grep -F "libginkgo." >/dev/null; then
- LINK_FLAGS="-lpapi -lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference"
+ LINK_FLAGS="-lpapi -lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference -lginkgo_hip"
 else
- LINK_FLAGS="-lpapi -lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced"
+ LINK_FLAGS="-lpapi -lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced -lginkgo_hipd"
 fi
 if [ -z "${CXX}" ]; then
  CXX="c++"

examples/performance-debugging/build.sh

@@ -9,8 +9,8 @@ BUILD_DIR=$1
 THIS_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" &>/dev/null && pwd )
 
 # copy libraries
-LIBRARY_DIRS="core core/device_hooks reference omp cuda"
-LIBRARY_NAMES="ginkgo ginkgo_reference ginkgo_omp ginkgo_cuda"
+LIBRARY_DIRS="core core/device_hooks reference omp cuda hip"
+LIBRARY_NAMES="ginkgo ginkgo_reference ginkgo_omp ginkgo_cuda ginkgo_hip"
 SUFFIXES=".so .dylib .dll d.so d.dylib d.dll"
 for prefix in ${LIBRARY_DIRS}; do
  for name in ${LIBRARY_NAMES}; do
@@ -23,9 +23,9 @@ done
 
 # figure out correct compiler flags
 if ls ${THIS_DIR} | grep -F "libginkgo." >/dev/null; then
- LINK_FLAGS="-lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference"
+ LINK_FLAGS="-lginkgo -lginkgo_omp -lginkgo_cuda -lginkgo_reference -lginkgo_hip"
 else
- LINK_FLAGS="-lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced"
+ LINK_FLAGS="-lginkgod -lginkgo_ompd -lginkgo_cudad -lginkgo_referenced -lginkgo_hipd"
 fi
 if [ -z "${CXX}" ]; then
  CXX="c++"

hip/CMakeLists.txt

@@ -142,8 +142,8 @@ if(GINKGO_HIP_PLATFORM MATCHES "hcc")
  # Ban `-hc` flag as INTERFACE_LINK_LIBRARIES since that is propagated when building
  # a static library, and it's definitely not a known option to any compiler.
  get_target_property(GINKGO_HCCRT_ILL hcc::hccrt INTERFACE_LINK_LIBRARIES)
- string(REPLACE "-hc " "" GINKGO_HCCRT_NEW_ILL GINKGO_HCCRT_ILL)
- set_target_properties(hcc::hccrt PROPERTIES INTERFACE_LINK_LIBRARIES "${GINKGO_HCC_NEW_ILL}")
+ string(REPLACE "-hc " "" GINKGO_HCCRT_NEW_ILL "${GINKGO_HCCRT_ILL}")
+ set_target_properties(hcc::hccrt PROPERTIES INTERFACE_LINK_LIBRARIES "${GINKGO_HCCRT_NEW_ILL}")
 
  target_link_libraries(ginkgo_hip PRIVATE hip::device)
 elseif(GINKGO_HIP_PLATFORM MATCHES "nvcc")