Merge pull request cms-sw#43281 from ericcano/SoA_Fixups

SoA fixes (2nd round): fix and add broken / missing SoA (const) row-to-row assignment.

Author: cmsbuild

DataFormats/SoATemplate/README.md

@@ -135,7 +135,7 @@ The buffer of the proper size is allocated, and the layout is populated with:
 // Allocation of aligned
 size_t elements = 100;
 using AlignedBuffer = std::unique_ptr<std::byte, decltype(std::free) *>;
-AlignedBuffer h_buf (reinterpret_cast<std::byte*>(aligned_alloc(SoA1LayoutAligned::byteAlignment, SoA1LayoutAligned::computeDataSize(elements))), std::free);
+AlignedBuffer h_buf (reinterpret_cast<std::byte*>(aligned_alloc(SoA1LayoutAligned::alignment, SoA1LayoutAligned::computeDataSize(elements))), std::free);
 SoA1LayoutAligned soaLayout(h_buf.get(), elements);
 ```
 

DataFormats/SoATemplate/interface/SoACommon.h

@@ -299,7 +299,7 @@ namespace cms::soa {
 
     SOA_HOST_DEVICE SOA_INLINE RefToConst operator()() const {
       // PtrToConst type will add the restrict qualifyer if needed
-      PtrToConst col = col_();
+      PtrToConst col = col_;
       return col[idx_];
     }
 

DataFormats/SoATemplate/interface/SoAView.h

@@ -604,6 +604,11 @@ namespace cms::soa {
       element& operator=(const element& _soa_impl_other) {                                                             \
         _ITERATE_ON_ALL(_DECLARE_VIEW_ELEMENT_VALUE_COPY, ~, VALUE_LIST)                                               \
         return *this;                                                                                                  \
+      }                                                                                                                \
+      SOA_HOST_DEVICE SOA_INLINE                                                                                       \
+      element& operator=(const const_element& _soa_impl_other) {                                                       \
+        _ITERATE_ON_ALL(_DECLARE_VIEW_ELEMENT_VALUE_COPY, ~, VALUE_LIST)                                               \
+        return *this;                                                                                                  \
       }
 // clang-format on
 

DataFormats/SoATemplate/test/BuildFile.xml

@@ -16,6 +16,12 @@
   </bin>
 </iftool>
 
+<bin file="SoAUnitTests.cc" name="SoAUnitTests">
+  <use name="boost"/>
+  <use name="catch2"/>
+</bin>
+
+
 <!-- This test triggers a bug in ROOT, so it's kept disabled
 <bin file="SoAStreamer_t.cpp" name="SoAStreamer_t">
   <use name="root"/>

DataFormats/SoATemplate/test/SoAUnitTests.cc

@@ -0,0 +1,103 @@
+#include <memory>
+#include <tuple>
+
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"
+#include "DataFormats/SoATemplate/interface/SoALayout.h"
+
+// clang-format off
+GENERATE_SOA_LAYOUT(SimpleLayoutTemplate,
+  SOA_COLUMN(float, x),
+  SOA_COLUMN(float, y),
+  SOA_COLUMN(float, z),
+  SOA_COLUMN(float, t))
+// clang-format on
+
+using SimpleLayout = SimpleLayoutTemplate<>;
+
+TEST_CASE("SoATemplate") {
+  const std::size_t slSize = 10;
+  const std::size_t slBufferSize = SimpleLayout::computeDataSize(slSize);
+  std::unique_ptr<std::byte, decltype(std::free) *> slBuffer{
+      reinterpret_cast<std::byte *>(aligned_alloc(SimpleLayout::alignment, slBufferSize)), std::free};
+  SimpleLayout sl{slBuffer.get(), slSize};
+  SECTION("Row wide copies, row") {
+    SimpleLayout::View slv{sl};
+    SimpleLayout::ConstView slcv{sl};
+    auto slv0 = slv[0];
+    slv0.x() = 1;
+    slv0.y() = 2;
+    slv0.z() = 3;
+    slv0.t() = 5;
+    // Fill up
+    for (SimpleLayout::View::size_type i = 1; i < slv.metadata().size(); ++i) {
+      auto slvi = slv[i];
+      slvi = slv[i - 1];
+      auto slvix = slvi.x();
+      slvi.x() += slvi.y();
+      slvi.y() += slvi.z();
+      slvi.z() += slvi.t();
+      slvi.t() += slvix;
+    }
+    // Verification and const view access
+    float x = 1, y = 2, z = 3, t = 5;
+    for (SimpleLayout::View::size_type i = 0; i < slv.metadata().size(); ++i) {
+      auto slvi = slv[i];
+      auto slcvi = slcv[i];
+      REQUIRE(slvi.x() == x);
+      REQUIRE(slvi.y() == y);
+      REQUIRE(slvi.z() == z);
+      REQUIRE(slvi.t() == t);
+      REQUIRE(slcvi.x() == x);
+      REQUIRE(slcvi.y() == y);
+      REQUIRE(slcvi.z() == z);
+      REQUIRE(slcvi.t() == t);
+      auto tx = x;
+      x += y;
+      y += z;
+      z += t;
+      t += tx;
+    }
+  }
+
+  SECTION("Row initializer, const view access, restrict disabling") {
+    // With two views, we are creating (artificially) aliasing and should warn the compiler by turning restrict qualifiers off.
+    using View = SimpleLayout::ViewTemplate<cms::soa::RestrictQualify::disabled, cms::soa::RangeChecking::Default>;
+    using ConstView =
+        SimpleLayout::ConstViewTemplate<cms::soa::RestrictQualify::disabled, cms::soa::RangeChecking::Default>;
+    View slv{sl};
+    ConstView slcv{sl};
+    auto slv0 = slv[0];
+    slv0 = {7, 11, 13, 17};
+    // Fill up
+    for (SimpleLayout::View::size_type i = 1; i < slv.metadata().size(); ++i) {
+      auto slvi = slv[i];
+      // Copy the const view
+      slvi = slcv[i - 1];
+      auto slvix = slvi.x();
+      slvi.x() += slvi.y();
+      slvi.y() += slvi.z();
+      slvi.z() += slvi.t();
+      slvi.t() += slvix;
+    }
+    // Verification and const view access
+    auto [x, y, z, t] = std::make_tuple(7.0, 11.0, 13.0, 17.0);
+    for (SimpleLayout::View::size_type i = 0; i < slv.metadata().size(); ++i) {
+      auto slvi = slv[i];
+      auto slcvi = slcv[i];
+      REQUIRE(slvi.x() == x);
+      REQUIRE(slvi.y() == y);
+      REQUIRE(slvi.z() == z);
+      REQUIRE(slvi.t() == t);
+      REQUIRE(slcvi.x() == x);
+      REQUIRE(slcvi.y() == y);
+      REQUIRE(slcvi.z() == z);
+      REQUIRE(slcvi.t() == t);
+      auto tx = x;
+      x += y;
+      y += z;
+      z += t;
+      t += tx;
+    }
+  }
+}