Change Accessor constructor order

Now, the size is always first in the constructor, and the data pointer is followed by the corresponding stride (if existing).
ginkgo-project · Mar 13, 2021 · f7f8540 · f7f8540
1 parent bb6a728
commit f7f8540
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 59 deletions.
diff --git a/accessor/block_col_major.hpp b/accessor/block_col_major.hpp
@@ -99,10 +99,10 @@ class block_col_major {
      *   `x_1 * stride_1 + x_2 * stride_2 * ... + x_(n-1) + x_n * stride_(n-1)`
      *                points to the element at (x_1, x_2, ..., x_n))
      */
-    constexpr GKO_ACC_ATTRIBUTES explicit block_col_major(data_type data,
-                                                          length_type size,
+    constexpr GKO_ACC_ATTRIBUTES explicit block_col_major(length_type size,
+                                                          data_type data,
                                                           stride_type stride)
-        : data{data}, lengths(size), stride(stride)
+        : lengths(size), data{data}, stride(stride)
     {}
 
     /**
@@ -112,10 +112,10 @@ class block_col_major {
      * @param data  pointer to the block of memory containing the data
      * @param lengths size / length of the accesses of each dimension
      */
-    constexpr GKO_ACC_ATTRIBUTES explicit block_col_major(data_type data,
-                                                          length_type size)
-        : data{data},
-          lengths(size),
+    constexpr GKO_ACC_ATTRIBUTES explicit block_col_major(length_type size,
+                                                          data_type data)
+        : lengths(size),
+          data{data},
           stride(helper::blk_col_major::default_stride_array(lengths))
     {}
 
@@ -129,7 +129,7 @@ class block_col_major {
     constexpr GKO_ACC_ATTRIBUTES range<const_accessor> to_const() const
     {
         // TODO Remove this functionality all together (if requested)
-        return range<const_accessor>(data, lengths, stride);
+        return range<const_accessor>(lengths, data, stride);
     }
 
     /**
@@ -165,10 +165,10 @@ class block_col_major {
     {
         return helper::validate_index_spans(lengths, spans...),
                range<block_col_major>{
-                   data + helper::blk_col_major::compute_index(
-                              lengths, stride, (index_span{spans}.begin)...),
                    length_type{
                        (index_span{spans}.end - index_span{spans}.begin)...},
+                   data + helper::blk_col_major::compute_index(
+                              lengths, stride, (index_span{spans}.begin)...),
                    stride};
     }
 
@@ -185,14 +185,14 @@ class block_col_major {
     }
 
     /**
-     * Reference to the underlying data.
+     * An array of dimension sizes.
      */
-    const data_type data;
+    const length_type lengths;
 
     /**
-     * An array of dimension sizes.
+     * Reference to the underlying data.
      */
-    const length_type lengths;
+    const data_type data;
 
     /**
      * Distance between consecutive 'layers' for each dimension

diff --git a/accessor/reduced_row_major.hpp b/accessor/reduced_row_major.hpp
@@ -264,7 +264,7 @@ class reduced_row_major {
 
 private:
     const dim_type size_;
-    storage_type *storage_;
+    storage_type *const storage_;
     const storage_stride_type stride_;
 };
 

diff --git a/accessor/row_major.hpp b/accessor/row_major.hpp
@@ -66,6 +66,10 @@ class row_major {
 
     static_assert(Dimensionality != 0,
                   "This accessor does not support a dimensionality of 0!");
+
+    /**
+     * Number of dimensions of the accessor.
+     */
     static constexpr size_type dimensionality = Dimensionality;
 
     /**
@@ -78,41 +82,37 @@ class row_major {
      */
     using data_type = value_type *;
 
-    /**
-     * Number of dimensions of the accessor.
-     */
-
     using const_accessor = row_major<const ValueType, Dimensionality>;
-    using stride_type = std::array<size_type, dimensionality - 1>;
     using length_type = std::array<size_type, dimensionality>;
+    using stride_type = std::array<size_type, dimensionality - 1>;
 
 protected:
     /**
      * Creates a row_major accessor.
      *
-     * @param data  pointer to the block of memory containing the data
      * @param lengths size / length of the accesses of each dimension
+     * @param data  pointer to the block of memory containing the data
      * @param stride  distance (in elements) between starting positions of
      *                the dimensions (i.e.
      *                `x_1 * stride_1 + x_2 * stride_2 * ... + x_n`
      *                points to the element at (x_1, x_2, ..., x_n))
      */
-    constexpr GKO_ACC_ATTRIBUTES explicit row_major(data_type data,
-                                                    length_type size,
+    constexpr GKO_ACC_ATTRIBUTES explicit row_major(length_type size,
+                                                    data_type data,
                                                     stride_type stride)
-        : data{data}, lengths(size), stride(stride)
+        : lengths(size), data{data}, stride(stride)
     {}
 
     /**
      * Creates a row_major accessor with a default stride (assumes no
      * padding)
      *
-     * @param data  pointer to the block of memory containing the data
      * @param lengths size / length of the accesses of each dimension
+     * @param data  pointer to the block of memory containing the data
      */
-    constexpr GKO_ACC_ATTRIBUTES explicit row_major(data_type data,
-                                                    length_type size)
-        : row_major{data, size,
+    constexpr GKO_ACC_ATTRIBUTES explicit row_major(length_type size,
+                                                    data_type data)
+        : row_major{size, data,
                     helper::compute_default_row_major_stride_array<
                         typename stride_type::value_type>(size)}
     {}
@@ -127,7 +127,7 @@ class row_major {
     constexpr GKO_ACC_ATTRIBUTES range<const_accessor> to_const() const
     {
         // TODO Remove this functionality all together (if requested)
-        return range<const_accessor>(data, lengths, stride);
+        return range<const_accessor>(lengths, data, stride);
     }
 
     /**
@@ -163,10 +163,10 @@ class row_major {
     {
         return helper::validate_index_spans(lengths, spans...),
                range<row_major>{
-                   data + helper::compute_row_major_index(
-                              lengths, stride, (index_span{spans}.begin)...),
                    length_type{
                        (index_span{spans}.end - index_span{spans}.begin)...},
+                   data + helper::compute_row_major_index(
+                              lengths, stride, (index_span{spans}.begin)...),
                    stride};
     }
 
@@ -183,14 +183,14 @@ class row_major {
     }
 
     /**
-     * Reference to the underlying data.
+     * An array of dimension sizes.
      */
-    const data_type data;
+    const length_type lengths;
 
     /**
-     * An array of dimension sizes.
+     * Reference to the underlying data.
      */
-    const length_type lengths;
+    const data_type data;
 
     /**
      * Distance between consecutive rows for each dimension (except the

diff --git a/accessor/scaled_reduced_row_major.hpp b/accessor/scaled_reduced_row_major.hpp
@@ -483,9 +483,9 @@ class scaled_reduced_row_major
 
 private:
     const dim_type size_;
-    storage_type *storage_;
+    storage_type *const storage_;
     const storage_stride_type storage_stride_;
-    scalar_type *scalar_;
+    scalar_type *const scalar_;
     const scalar_stride_type scalar_stride_;
 };
 

diff --git a/accessor/utils.hpp b/accessor/utils.hpp
@@ -162,9 +162,8 @@ struct are_all_integral : public std::true_type {};
 
 template <typename First, typename... Args>
 struct are_all_integral<First, Args...>
-    : public std::conditional<std::is_integral<std::decay_t<First>>::value,
-                              are_all_integral<Args...>,
-                              std::false_type>::type {};
+    : public std::conditional_t<std::is_integral<std::decay_t<First>>::value,
+                                are_all_integral<Args...>, std::false_type> {};
 
 
 }  // namespace acc

diff --git a/core/test/accessor/block_col_major.cpp b/core/test/accessor/block_col_major.cpp
@@ -83,9 +83,9 @@ class BlockColMajorAccessor3d : public ::testing::Test {
     // clang-format on
     const std::array<gko::acc::size_type, dimensionality> dim1{{2, 3, 4}};
     const std::array<gko::acc::size_type, dimensionality> dim2{{2, 2, 3}};
-    blk_col_major_range default_r{data, dim1};
+    blk_col_major_range default_r{dim1, data};
     blk_col_major_range custom_r{
-        data, dim2,
+        dim2, data,
         std::array<gko::acc::size_type, dimensionality - 1>{{12, 3}}};
 };
 

diff --git a/core/test/accessor/row_major.cpp b/core/test/accessor/row_major.cpp
@@ -66,26 +66,13 @@ class RowMajorAccessor : public ::testing::Test {
         5, 6, -3
     };
     // clang-format on
-    row_major_int_range r{data, dim_type{{3u, 2u}}, stride_type{{3u}}};
+    row_major_int_range r{dim_type{{3u, 2u}}, data, stride_type{{3u}}};
 };
 
 
-TEST_F(RowMajorAccessor, CanCreateWithDim)
-{
-    row_major_int_range r2{data, dim_type{{3, 2}}, stride_type{{3u}}};
-
-    EXPECT_EQ(r2(0, 0), 1);
-    EXPECT_EQ(r2(0, 1), 2);
-    EXPECT_EQ(r2(1, 0), 3);
-    EXPECT_EQ(r2(1, 1), 4);
-    EXPECT_EQ(r2(2, 0), 5);
-    EXPECT_EQ(r2(2, 1), 6);
-}
-
-
 TEST_F(RowMajorAccessor, CanCreateDefaultStride)
 {
-    row_major_int_range r2{data, dim_type{{3, 3}}};
+    row_major_int_range r2{dim_type{{3, 3}}, data};
 
     EXPECT_EQ(r2(0, 0), 1);
     EXPECT_EQ(r2(0, 1), 2);
@@ -177,9 +164,9 @@ class RowMajorAccessor3d : public ::testing::Test {
     // clang-format on
     const std::array<gko::acc::size_type, dimensionality> dim1{{2, 3, 4}};
     const std::array<gko::acc::size_type, dimensionality> dim2{{2, 2, 3}};
-    row_major_int_range default_r{data, dim1};
+    row_major_int_range default_r{dim1, data};
     row_major_int_range custom_r{
-        data, dim2, std::array<gko::size_type, dimensionality - 1>{{12, 4}}};
+        dim2, data, std::array<gko::size_type, dimensionality - 1>{{12, 4}}};
 };