rework alignment for empty ranges (#303)

Author: rscohn2

Diff for: include/dr/mhp/algorithms/copy.hpp

@@ -10,6 +10,10 @@ namespace dr::mhp {
 
 /// Copy
 void copy(dr::distributed_range auto &&in, dr::distributed_iterator auto out) {
+  if (rng::empty(in)) {
+    return;
+  }
+
   if (aligned(in, out)) {
     dr::drlog.debug("copy: parallel execution\n");
     for (const auto &&[in_seg, out_seg] :

Diff for: include/dr/mhp/algorithms/for_each.hpp

@@ -20,6 +20,9 @@ namespace dr::mhp {
 /// Collective for_each on distributed range
 void for_each(dr::distributed_range auto &&dr, auto op) {
   dr::drlog.debug("for_each: parallel execution\n");
+  if (rng::empty(dr)) {
+    return;
+  }
   assert(aligned(dr));
 
   for (const auto &s : local_segments(dr)) {

Diff for: include/dr/mhp/algorithms/reduce.hpp

@@ -36,11 +36,16 @@ auto reduce(std::size_t root, bool root_provided, DR &&dr, auto &&binary_op) {
   using value_type = rng::range_value_t<DR>;
   auto comm = default_comm();
 
+  if (rng::empty(dr)) {
+    return rng::range_value_t<DR>{};
+  }
+
   if (aligned(dr)) {
     dr::drlog.debug("Parallel reduce\n");
 
     // Reduce the local segments
     auto reduce = [=](auto &&r) {
+      assert(rng::size(r) > 0);
       if (mhp::use_sycl()) {
         dr::drlog.debug("  with DPL\n");
         return dpl_reduce(r, binary_op);
@@ -84,6 +89,13 @@ T reduce(std::size_t root, bool root_provided, DR &&dr, T init,
   return binary_op(init, reduce(root, root_provided, dr, binary_op));
 }
 
+inline void
+#if defined(__GNUC__) && !defined(__clang__)
+    __attribute__((optimize(0)))
+#endif
+    no_optimize(auto x) {
+}
+
 }; // namespace dr::mhp::__detail
 
 namespace dr::mhp {
@@ -124,9 +136,20 @@ template <typename T, dr::distributed_range DR> auto reduce(DR &&dr, T init) {
 template <dr::distributed_range DR> auto reduce(std::size_t root, DR &&dr) {
   return __detail::reduce(root, true, std::forward<DR>(dr), std::plus<>{});
 }
+
 /// Collective reduction on a distributed range
 template <dr::distributed_range DR> auto reduce(DR &&dr) {
-  return __detail::reduce(0, false, std::forward<DR>(dr), std::plus<>{});
+  auto x = __detail::reduce(0, false, std::forward<DR>(dr), std::plus<>{});
+
+  // The code below avoids an issue where DotProduct_ZipReduce_DR
+  // fails with gcc11.  From debugging, I can see that the call to
+  // __detail::reduce above computes the correct value, but this
+  // function returns a bad value. My theory is that the problem is
+  // related to tail call optimization and the function below disables
+  // the optimization.
+  __detail::no_optimize(x);
+
+  return x;
 }
 
 //

Diff for: include/dr/mhp/algorithms/transform.hpp

@@ -19,6 +19,9 @@ namespace dr::mhp {
 
 void transform(dr::distributed_range auto &&in,
                dr::distributed_iterator auto out, auto op) {
+  if (rng::empty(in)) {
+    return;
+  }
   assert(aligned(in, out));
 
   auto zip = mhp::views::zip(in, rng::subrange(out, out + rng::size(in)));

Diff for: include/dr/mhp/alignment.hpp

@@ -16,19 +16,19 @@ concept has_segments = requires(T &t) { dr::ranges::segments(t); };
 template <typename T>
 concept no_segments = !has_segments<T>;
 
-auto sub_aligned(bool non_empty, has_segments auto &&r) {
-  if (non_empty && dr::ranges::segments(r).empty()) {
+auto sub_aligned(has_segments auto &&r) {
+  if (rng::empty(dr::ranges::segments(r))) {
     dr::drlog.debug("unaligned: empty segments\n");
     return false;
   } else {
     return true;
   }
 }
 
-auto sub_aligned(bool non_empty, auto &&r) { return true; }
+auto sub_aligned(auto &&r) { return true; }
 
 // iter1 is aligned with iter2, and iter2 is aligned with the rest
-bool sub_aligned(bool non_empty, has_segments auto &&r1, has_segments auto &&r2,
+bool sub_aligned(has_segments auto &&r1, has_segments auto &&r2,
                  auto &&...rest) {
   auto z = rng::views::zip(dr::ranges::segments(r1), dr::ranges::segments(r2));
   auto i = rng::distance(z) - 1;
@@ -48,25 +48,23 @@ bool sub_aligned(bool non_empty, has_segments auto &&r1, has_segments auto &&r2,
     i--;
   }
 
-  if (!rng::empty(dr::ranges::segments(r1))) {
-    non_empty = true;
-  }
-  return sub_aligned(non_empty, r2, rest...);
+  return sub_aligned(r2, rest...);
 }
 
 // Skip local iterators
-bool sub_aligned(bool non_empty, no_segments auto &&r1, has_segments auto &&r2,
-                 auto... rest) {
-  return sub_aligned(non_empty, r2, rest...);
+bool sub_aligned(no_segments auto &&r1, has_segments auto &&r2, auto... rest) {
+  return sub_aligned(r2, rest...);
 }
 
-bool sub_aligned(bool non_empty, has_segments auto &&r1, no_segments auto &&r2,
+bool sub_aligned(has_segments auto &&r1, no_segments auto &&r2,
                  auto &&...rest) {
-  return sub_aligned(non_empty, r1, rest...);
+  return sub_aligned(r1, rest...);
 }
 
+// This was added to allow passing state down the call tree, but it is
+// no longer needed. I did not delete it in case we need it again.
 template <typename... Args> bool aligned(Args &&...args) {
-  return sub_aligned(false, std::forward<Args>(args)...);
+  return sub_aligned(std::forward<Args>(args)...);
 }
 
 } // namespace dr::mhp

Diff for: include/dr/mhp/views/zip.hpp

@@ -230,6 +230,8 @@ template <__detail::zipable... Rs> class zip_view : public rng::view_base {
     return std::apply(make_end, base_);
   }
   auto size() const { return rng::size(rng_zip_); }
+  auto empty() const { return begin() == end(); }
+
   auto operator[](difference_type n) const { return rng_zip_[n]; }
 
   auto base() const { return base_; }

Diff for: scripts/dr-style.py

@@ -113,6 +113,10 @@
         r'\.size\(\)',
         'use rng::size()',
     ),
+    (
+        r'\.empty\(\)',
+        'use rng::empty()',
+    ),
     (
         r'std::begin\(',
         'use rng::begin()',

Diff for: test/gtest/common/zip_local.cpp

@@ -56,6 +56,11 @@ TEST_F(ZipLocal, Begin) {
   EXPECT_EQ(*r.begin(), *z.begin());
 }
 
+TEST_F(ZipLocal, Empty) {
+  EXPECT_FALSE(test_zip(ops[0]).empty());
+  EXPECT_TRUE(test_zip(rng::views::take(ops[0], 0)).empty());
+}
+
 TEST_F(ZipLocal, End) {
   auto z = test_zip(ops[0], ops[1]);
   auto r = rng::views::zip(ops[0], ops[1]);

Diff for: test/gtest/mhp/CMakeLists.txt

@@ -44,7 +44,8 @@ add_executable(
 add_executable(
   mhp-quick-test
   mhp-tests.cpp
-  ../common/counted.cpp
+  alignment.cpp
+  ../common/zip_local.cpp
 )
 
 target_compile_definitions(mhp-quick-test PRIVATE QUICK_TEST)

Diff for: test/gtest/mhp/alignment.cpp

@@ -33,26 +33,6 @@ TEST(Alignment, OffsetBy1) {
   }
 }
 
-TEST(Alignment, EmptyRangeRange) {
-  Ops2<DV> ops(10);
-  EXPECT_TRUE(dr::mhp::aligned(
-      rng::subrange(ops.dist_vec0.begin() + 10, ops.dist_vec0.begin() + 10),
-      rng::subrange(ops.dist_vec1.begin() + 10, ops.dist_vec1.begin() + 10)));
-}
-
-TEST(Alignment, EmptyRangeIter) {
-  Ops2<DV> ops(10);
-  EXPECT_TRUE(dr::mhp::aligned(
-      rng::subrange(ops.dist_vec0.begin() + 10, ops.dist_vec0.begin() + 10),
-      ops.dist_vec1.begin() + 10));
-}
-
-TEST(Alignment, EmptyIterIter) {
-  Ops2<DV> ops(10);
-  EXPECT_TRUE(
-      dr::mhp::aligned(ops.dist_vec1.begin() + 10, ops.dist_vec1.begin() + 10));
-}
-
 TEST(Alignment, Subrange) {
   Ops2<DV> ops(10);
   auto is_aligned = dr::mhp::aligned(
@@ -72,3 +52,21 @@ TEST(Alignment, Iota2) {
   Ops1<DV> ops(10);
   EXPECT_TRUE(dr::mhp::aligned(ops.dist_vec, rng::views::iota(100)));
 }
+
+TEST(Alignment, ZipAligned) {
+  Ops2<DV> ops(10);
+  EXPECT_TRUE(
+      dr::mhp::aligned(dr::mhp::views::zip(ops.dist_vec0, ops.dist_vec1)));
+}
+
+TEST(Alignment, ZipMisaligned) {
+  Ops2<DV> ops(10);
+  auto is_aligned = dr::mhp::aligned(dr::mhp::views::zip(
+      dr::mhp::views::drop(ops.dist_vec0, 1), ops.dist_vec1));
+  if (comm_size == 1) {
+    // If there is a single segment, then it is aligned
+    EXPECT_TRUE(is_aligned);
+  } else {
+    EXPECT_FALSE(is_aligned);
+  }
+}