Fix warnings in HIP branch. (#41)

* Fix warnings in HIP branch.
ORNL · Oct 31, 2023 · 7ea6515 · 7ea6515
1 parent 3b1b782
commit 7ea6515
Show file tree

Hide file tree

Showing 13 changed files with 40 additions and 28 deletions.
diff --git a/resolve/LinSolverDirectKLU.cpp b/resolve/LinSolverDirectKLU.cpp
@@ -157,7 +157,8 @@ namespace ReSolve
   {
     if (Numeric_ != nullptr){
       P_ = new index_type[A_->getNumRows()];
-      std::memcpy(P_, Numeric_->Pnum, A_->getNumRows() * sizeof(index_type));
+      size_t nrows = static_cast<size_t>(A_->getNumRows());
+      std::memcpy(P_, Numeric_->Pnum, nrows * sizeof(index_type));
       return P_;
     } else {
       return nullptr;
@@ -169,7 +170,8 @@ namespace ReSolve
   {
     if (Numeric_ != nullptr){
       Q_ = new index_type[A_->getNumRows()];
-      std::memcpy(Q_, Symbolic_->Q, A_->getNumRows() * sizeof(index_type));
+      size_t nrows = static_cast<size_t>(A_->getNumRows());
+      std::memcpy(Q_, Symbolic_->Q, nrows * sizeof(index_type));
       return Q_;
     } else {
       return nullptr;

diff --git a/resolve/MemoryUtils.hpp b/resolve/MemoryUtils.hpp
@@ -44,6 +44,15 @@ namespace ReSolve
 
       template <typename I, typename T>
       int copyArrayHostToDevice(T* dst, const T* src, I n);
+
+      /// Implemented here as it is always needed
+      template <typename I, typename T>
+      int copyArrayHostToHost(T* dst, const T* src, I n)
+      {
+        size_t nelements = static_cast<size_t>(n);
+        memcpy(dst, src, nelements * sizeof(T));
+        return 0;
+      }
   };
 
 } // namespace ReSolve

diff --git a/resolve/matrix/Coo.cpp b/resolve/matrix/Coo.cpp
@@ -113,9 +113,9 @@ namespace ReSolve
 
     switch(control)  {
       case 0: //cpu->cpu
-        std::memcpy(h_row_data_, row_data, (nnz_current) * sizeof(index_type));
-        std::memcpy(h_col_data_, col_data, (nnz_current) * sizeof(index_type));
-        std::memcpy(h_val_data_, val_data, (nnz_current) * sizeof(real_type));
+        mem_.copyArrayHostToHost(h_row_data_, row_data, nnz_current);
+        mem_.copyArrayHostToHost(h_col_data_, col_data, nnz_current);
+        mem_.copyArrayHostToHost(h_val_data_, val_data, nnz_current);
         h_data_updated_ = true;
         owns_cpu_data_ = true;
         owns_cpu_vals_ = true;

diff --git a/resolve/matrix/Csc.cpp b/resolve/matrix/Csc.cpp
@@ -109,9 +109,9 @@ namespace ReSolve
 
     switch(control)  {
       case 0: //cpu->cpu
-        std::memcpy(h_col_data_, col_data, (n_ + 1) * sizeof(index_type));
-        std::memcpy(h_row_data_, row_data, (nnz_current) * sizeof(index_type));
-        std::memcpy(h_val_data_, val_data, (nnz_current) * sizeof(real_type));
+        mem_.copyArrayHostToHost(h_col_data_, col_data,      n_ + 1);
+        mem_.copyArrayHostToHost(h_row_data_, row_data, nnz_current);
+        mem_.copyArrayHostToHost(h_val_data_, val_data, nnz_current);
         h_data_updated_ = true;
         owns_cpu_data_ = true;
         owns_cpu_vals_ = true;

diff --git a/resolve/matrix/Csr.cpp b/resolve/matrix/Csr.cpp
@@ -111,9 +111,9 @@ namespace ReSolve
     //copy	
     switch(control)  {
       case 0: //cpu->cpu
-        std::memcpy(h_row_data_, row_data, (n_ + 1) * sizeof(index_type));
-        std::memcpy(h_col_data_, col_data, (nnz_current) * sizeof(index_type));
-        std::memcpy(h_val_data_, val_data, (nnz_current) * sizeof(real_type));
+        mem_.copyArrayHostToHost(h_row_data_, row_data,      n_ + 1);
+        mem_.copyArrayHostToHost(h_col_data_, col_data, nnz_current);
+        mem_.copyArrayHostToHost(h_val_data_, val_data, nnz_current);
         h_data_updated_ = true;
         owns_cpu_data_ = true;
         owns_cpu_vals_ = true;

diff --git a/resolve/matrix/MatrixHandlerCpu.hpp b/resolve/matrix/MatrixHandlerCpu.hpp
@@ -50,7 +50,7 @@ namespace ReSolve {
       LinAlgWorkspaceCpu* workspace_{nullptr};
       bool values_changed_{true}; ///< needed for matvec
 
-      MemoryHandler mem_; ///< Device memory manager object
+      // MemoryHandler mem_; ///< Device memory manager object not used for now
   };
 
 } // namespace ReSolve

diff --git a/resolve/matrix/Sparse.cpp b/resolve/matrix/Sparse.cpp
@@ -228,7 +228,7 @@ namespace ReSolve { namespace matrix {
 
     switch(control)  {
       case 0: //cpu->cpu
-        std::memcpy(h_val_data_, new_vals, (nnz_current) * sizeof(real_type));
+        mem_.copyArrayHostToHost(h_val_data_, new_vals, nnz_current);
         h_data_updated_ = true;
         owns_cpu_vals_ = true;
         break;

diff --git a/resolve/utilities/logger/Logger.cpp b/resolve/utilities/logger/Logger.cpp
@@ -59,7 +59,7 @@ namespace ReSolve
      */
     void Logger::updateVerbosity(std::vector<std::ostream*>& output_streams)
     {
-      for (int i = NONE; i <= EVERYTHING; ++i)
+      for (std::size_t i = NONE; i <= EVERYTHING; ++i)
       {
         output_streams[i] = i > verbosity_ ? &nullstream_ : logger_;
       }

diff --git a/resolve/vector/Vector.cpp b/resolve/vector/Vector.cpp
@@ -104,7 +104,7 @@ namespace ReSolve { namespace vector {
 
     switch(control)  {
       case 0: //cpu->cpu
-        std::memcpy(h_data_, data, (n_current_ * k_) * sizeof(real_type));
+        mem_.copyArrayHostToHost(h_data_, data, n_current_ * k_);
         owns_cpu_data_ = true;
         cpu_updated_ = true;
         gpu_updated_ = false;
@@ -322,7 +322,7 @@ namespace ReSolve { namespace vector {
     } else {
       real_type* data = this->getData(i, memspaceOut);
       if (memspaceOut == "cpu") {
-        std::memcpy(dest, data, n_current_ * sizeof(real_type));
+        mem_.copyArrayHostToHost(dest, data, n_current_);
       } else {
       if ((memspaceOut == "cuda") || (memspaceOut == "hip")) {
           mem_.copyArrayDeviceToDevice(dest, data, n_current_);
@@ -338,7 +338,7 @@ namespace ReSolve { namespace vector {
   {
     real_type* data = this->getData(memspaceOut);
     if (memspaceOut == "cpu") {
-      std::memcpy(dest, data, n_current_ * k_ * sizeof(real_type));
+      mem_.copyArrayHostToHost(dest, data, n_current_ * k_);
     } else {
       if ((memspaceOut == "cuda") || (memspaceOut == "hip")) {
         mem_.copyArrayDeviceToDevice(dest, data, n_current_ * k_);

diff --git a/resolve/workspace/LinAlgWorkspaceCpu.cpp b/resolve/workspace/LinAlgWorkspaceCpu.cpp
@@ -1,3 +1,4 @@
+#include <cstddef>
 #include "LinAlgWorkspaceCpu.hpp"
 
 namespace ReSolve

diff --git a/resolve/workspace/LinAlgWorkspaceCpu.hpp b/resolve/workspace/LinAlgWorkspaceCpu.hpp
@@ -12,7 +12,7 @@ namespace ReSolve
       ~LinAlgWorkspaceCpu();
       void initializeHandles();
     private:
-      MemoryHandler mem_;
+      // MemoryHandler mem_; ///< Memory handler not needed for now
   };
 
 }
diff --git a/resolve/workspace/LinAlgWorkspaceHIP.hpp b/resolve/workspace/LinAlgWorkspaceHIP.hpp
@@ -46,7 +46,7 @@ namespace ReSolve
       //info - but we need info
       rocsparse_mat_info  info_A_;
 
-      MemoryHandler mem_;
+      // MemoryHandler mem_; ///< Memory handler not needed for now
   };
 
 } // namespace ReSolve
diff --git a/tests/unit/matrix/MatrixHandlerTests.hpp b/tests/unit/matrix/MatrixHandlerTests.hpp
@@ -125,39 +125,39 @@ class MatrixHandlerTests : TestBase
 
     // std::cout << N << "\n";
 
+    // First compute number of nonzeros
     index_type NNZ = 0;
     for (index_type i = 0; i < N; ++i)
     {
-      NNZ += static_cast<index_type>(data[i%5].size());
+      size_t reminder = static_cast<size_t>(i%5);
+      NNZ += static_cast<index_type>(data[reminder].size());
     }
-    // std::cout << NNZ << "\n";
 
+    // Allocate NxN CSR matrix with NNZ nonzeros
     matrix::Csr* A = new matrix::Csr(N, N, NNZ);
     A->allocateMatrixData("cpu");
 
     index_type* rowptr = A->getRowData("cpu");
     index_type* colidx = A->getColData("cpu");
     real_type* val     = A->getValues("cpu"); 
 
+    // Populate CSR matrix using same row pattern as for NNZ calculation
     rowptr[0] = 0;
-    index_type i = 0;
-    for (i=0; i < N; ++i)
+    for (index_type i=0; i < N; ++i)
     {
-      const std::vector<real_type>& row_sample = data[i%5];
+      size_t reminder = static_cast<size_t>(i%5);
+      const std::vector<real_type>& row_sample = data[reminder];
       index_type nnz_per_row = static_cast<index_type>(row_sample.size());
-      // std::cout << nnz_per_row << "\n";
 
       rowptr[i+1] = rowptr[i] + nnz_per_row;
       for (index_type j = rowptr[i]; j < rowptr[i+1]; ++j)
       {
         colidx[j] = (j - rowptr[i]) * N/nnz_per_row + (N%(N/nnz_per_row));
         // evenly distribute nonzeros ^^^^             ^^^^^^^^ perturb offset
-        val[j] = row_sample[j - rowptr[i]];
-        // std::cout << i << " " << colidx[j] << "  " << val[j] << "\n";
+        val[j] = row_sample[static_cast<size_t>(j - rowptr[i])];
       }
     }
     A->setUpdated("cpu");
-    // std::cout << rowptr[i] << "\n";
 
     if ((memspace == "cuda") || (memspace == "hip")) {
       A->copyData(memspace);