[libcxx-commits] [libcxx] [libcxx] Added segment iterator for fill (PR #104680)

via libcxx-commits libcxx-commits at lists.llvm.org
Sun Aug 25 21:57:56 PDT 2024


https://github.com/NoumanAmir657 updated https://github.com/llvm/llvm-project/pull/104680

>From 2782a4cf1b62a7db53d7cb6a3543db0259f4ef5e Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Sat, 17 Aug 2024 23:18:52 +0500
Subject: [PATCH 1/9] [libcxx] Added segment iterator for fill

---
 libcxx/include/__algorithm/fill.h | 53 +++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index 1ce3eadb013d05..3ba37e7260a4a2 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -21,25 +21,66 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 // fill isn't specialized for std::memset, because the compiler already optimizes the loop to a call to std::memset.
 
-template <class _ForwardIterator, class _Tp>
+template <
+    class _ForwardIterator,
+    class _Tp,
+    __enable_if_t<
+        is_same<typename iterator_traits<_ForwardIterator>::iterator_category, forward_iterator_tag>::value ||
+            is_same<typename iterator_traits<_ForwardIterator>::iterator_category, bidirectional_iterator_tag>::value,
+        int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-__fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, forward_iterator_tag) {
+__fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
   for (; __first != __last; ++__first)
     *__first = __value;
 }
 
-template <class _RandomAccessIterator, class _Tp>
+template <class _RandomAccessIterator,
+          class _Tp,
+          __enable_if_t<(is_same<typename iterator_traits<_RandomAccessIterator>::iterator_category,
+                                 random_access_iterator_tag>::value ||
+                         is_same<typename iterator_traits<_RandomAccessIterator>::iterator_category,
+                                 contiguous_iterator_tag>::value) &&
+                            !__is_segmented_iterator<_RandomAccessIterator>::value,
+                        int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-__fill(_RandomAccessIterator __first, _RandomAccessIterator __last, const _Tp& __value, random_access_iterator_tag) {
+__fill(_RandomAccessIterator __first, _RandomAccessIterator __last, const _Tp& __value) {
   std::fill_n(__first, __last - __first, __value);
 }
 
+template <class _SegmentedIterator,
+          class _Tp,
+          __enable_if_t<__is_segmented_iterator<_SegmentedIterator>::value, int> = 0>
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
+__fill(_SegmentedIterator __first, _SegmentedIterator __last, const _Tp& __value) {
+  using _Traits = __segmented_iterator_traits<_SegmentedIterator>;
+
+  auto __sfirst = _Traits::__segment(__first);
+  auto __slast  = _Traits::__segment(__last);
+
+  // We are in a single segment, so we might not be at the beginning or end
+  if (__sfirst == __slast) {
+    __fill(_Traits::__local(__first), _Traits::__local(__last), __value);
+    return;
+  }
+
+  // We have more than one segment. Iterate over the first segment, since we might not start at the beginning
+  __fill(_Traits::__local(__first), _Traits::__end(__sfirst), __value);
+  ++__sfirst;
+  // iterate over the segments which are guaranteed to be completely in the range
+  while (__sfirst != __slast) {
+    __fill(_Traits::__begin(__sfirst), _Traits::__end(__sfirst), __value);
+    ++__sfirst;
+  }
+  // iterate over the last segment
+  __fill(_Traits::__begin(__sfirst), _Traits::__local(__last), __value);
+}
+
 template <class _ForwardIterator, class _Tp>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
-  std::__fill(__first, __last, __value, typename iterator_traits<_ForwardIterator>::iterator_category());
+  std::__fill(__first, __last, __value);
 }
 
 _LIBCPP_END_NAMESPACE_STD
 
-#endif // _LIBCPP___ALGORITHM_FILL_H
+#endif // _LIBCPP___ALGORITHM_FILL_H
\ No newline at end of file

>From 9caf495c7e6e17ffd9da59858f33246f825a0b02 Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Sat, 17 Aug 2024 23:37:14 +0500
Subject: [PATCH 2/9] Fixed newline

---
 libcxx/include/__algorithm/fill.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index 3ba37e7260a4a2..da2bda4713c373 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -83,4 +83,4 @@ fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
 
 _LIBCPP_END_NAMESPACE_STD
 
-#endif // _LIBCPP___ALGORITHM_FILL_H
\ No newline at end of file
+#endif // _LIBCPP___ALGORITHM_FILL_H

>From 37dab9d3dfc627b8ca7b14ad3475caab2c4f81cd Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Sun, 18 Aug 2024 00:57:05 +0500
Subject: [PATCH 3/9] Fixed the logic of templates

---
 libcxx/include/__algorithm/fill.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index da2bda4713c373..b8d893e2ee7339 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -12,6 +12,7 @@
 #include <__algorithm/fill_n.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
+#include <__iterator/segmented_iterator.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -24,10 +25,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 template <
     class _ForwardIterator,
     class _Tp,
-    __enable_if_t<
-        is_same<typename iterator_traits<_ForwardIterator>::iterator_category, forward_iterator_tag>::value ||
-            is_same<typename iterator_traits<_ForwardIterator>::iterator_category, bidirectional_iterator_tag>::value,
-        int> = 0>
+    __enable_if_t<!__has_random_access_iterator_category<_ForwardIterator>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 __fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
   for (; __first != __last; ++__first)
@@ -36,12 +34,8 @@ __fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
 
 template <class _RandomAccessIterator,
           class _Tp,
-          __enable_if_t<(is_same<typename iterator_traits<_RandomAccessIterator>::iterator_category,
-                                 random_access_iterator_tag>::value ||
-                         is_same<typename iterator_traits<_RandomAccessIterator>::iterator_category,
-                                 contiguous_iterator_tag>::value) &&
-                            !__is_segmented_iterator<_RandomAccessIterator>::value,
-                        int> = 0>
+          __enable_if_t<__has_random_access_iterator_category<_RandomAccessIterator>::value &&
+                        !__is_segmented_iterator<_RandomAccessIterator>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 __fill(_RandomAccessIterator __first, _RandomAccessIterator __last, const _Tp& __value) {
   std::fill_n(__first, __last - __first, __value);

>From 8bb83d67e8dd050be8b06d2a1b93327c4e2b0dfb Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Sun, 18 Aug 2024 01:42:46 +0500
Subject: [PATCH 4/9] Used std::for_each to remove redundant code

---
 libcxx/include/__algorithm/fill.h | 49 ++++++++++++++++---------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index b8d893e2ee7339..32439f03dc9cff 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -10,6 +10,7 @@
 #define _LIBCPP___ALGORITHM_FILL_H
 
 #include <__algorithm/fill_n.h>
+#include <__algorithm/for_each.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/segmented_iterator.h>
@@ -22,10 +23,9 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 // fill isn't specialized for std::memset, because the compiler already optimizes the loop to a call to std::memset.
 
-template <
-    class _ForwardIterator,
-    class _Tp,
-    __enable_if_t<!__has_random_access_iterator_category<_ForwardIterator>::value, int> = 0>
+template < class _ForwardIterator,
+           class _Tp,
+           __enable_if_t<!__has_random_access_iterator_category<_ForwardIterator>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 __fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
   for (; __first != __last; ++__first)
@@ -35,7 +35,8 @@ __fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
 template <class _RandomAccessIterator,
           class _Tp,
           __enable_if_t<__has_random_access_iterator_category<_RandomAccessIterator>::value &&
-                        !__is_segmented_iterator<_RandomAccessIterator>::value, int> = 0>
+                            !__is_segmented_iterator<_RandomAccessIterator>::value,
+                        int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 __fill(_RandomAccessIterator __first, _RandomAccessIterator __last, const _Tp& __value) {
   std::fill_n(__first, __last - __first, __value);
@@ -46,27 +47,29 @@ template <class _SegmentedIterator,
           __enable_if_t<__is_segmented_iterator<_SegmentedIterator>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 __fill(_SegmentedIterator __first, _SegmentedIterator __last, const _Tp& __value) {
-  using _Traits = __segmented_iterator_traits<_SegmentedIterator>;
+  // using _Traits = __segmented_iterator_traits<_SegmentedIterator>;
 
-  auto __sfirst = _Traits::__segment(__first);
-  auto __slast  = _Traits::__segment(__last);
+  // auto __sfirst = _Traits::__segment(__first);
+  // auto __slast  = _Traits::__segment(__last);
 
-  // We are in a single segment, so we might not be at the beginning or end
-  if (__sfirst == __slast) {
-    __fill(_Traits::__local(__first), _Traits::__local(__last), __value);
-    return;
-  }
+  // // We are in a single segment, so we might not be at the beginning or end
+  // if (__sfirst == __slast) {
+  //   __fill(_Traits::__local(__first), _Traits::__local(__last), __value);
+  //   return;
+  // }
 
-  // We have more than one segment. Iterate over the first segment, since we might not start at the beginning
-  __fill(_Traits::__local(__first), _Traits::__end(__sfirst), __value);
-  ++__sfirst;
-  // iterate over the segments which are guaranteed to be completely in the range
-  while (__sfirst != __slast) {
-    __fill(_Traits::__begin(__sfirst), _Traits::__end(__sfirst), __value);
-    ++__sfirst;
-  }
-  // iterate over the last segment
-  __fill(_Traits::__begin(__sfirst), _Traits::__local(__last), __value);
+  // // We have more than one segment. Iterate over the first segment, since we might not start at the beginning
+  // __fill(_Traits::__local(__first), _Traits::__end(__sfirst), __value);
+  // ++__sfirst;
+  // // iterate over the segments which are guaranteed to be completely in the range
+  // while (__sfirst != __slast) {
+  //   __fill(_Traits::__begin(__sfirst), _Traits::__end(__sfirst), __value);
+  //   ++__sfirst;
+  // }
+  // // iterate over the last segment
+  // __fill(_Traits::__begin(__sfirst), _Traits::__local(__last), __value);
+
+  std::for_each(__first, __last, [__value](_Tp& val) { val = __value; });
 }
 
 template <class _ForwardIterator, class _Tp>

>From 0e799684c5cbcb0c7ad60f137f9c57c714d9b660 Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Sun, 18 Aug 2024 01:53:47 +0500
Subject: [PATCH 5/9] Fixed case

---
 libcxx/include/__algorithm/fill.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index 32439f03dc9cff..df9eb6ed16aa23 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -69,7 +69,7 @@ __fill(_SegmentedIterator __first, _SegmentedIterator __last, const _Tp& __value
   // // iterate over the last segment
   // __fill(_Traits::__begin(__sfirst), _Traits::__local(__last), __value);
 
-  std::for_each(__first, __last, [__value](_Tp& val) { val = __value; });
+  std::for_each(__first, __last, [__value](_Tp& __val) { __val = __value; });
 }
 
 template <class _ForwardIterator, class _Tp>

>From 508659875b460f106310f78ca4c05d88c85e7dcb Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Thu, 22 Aug 2024 16:26:51 +0500
Subject: [PATCH 6/9] Added benchmark

---
 libcxx/include/__algorithm/fill.h             | 22 -------------------
 .../test/benchmarks/algorithms/fill.bench.cpp | 10 +++++++++
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index df9eb6ed16aa23..41f37c46fe471d 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -47,28 +47,6 @@ template <class _SegmentedIterator,
           __enable_if_t<__is_segmented_iterator<_SegmentedIterator>::value, int> = 0>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
 __fill(_SegmentedIterator __first, _SegmentedIterator __last, const _Tp& __value) {
-  // using _Traits = __segmented_iterator_traits<_SegmentedIterator>;
-
-  // auto __sfirst = _Traits::__segment(__first);
-  // auto __slast  = _Traits::__segment(__last);
-
-  // // We are in a single segment, so we might not be at the beginning or end
-  // if (__sfirst == __slast) {
-  //   __fill(_Traits::__local(__first), _Traits::__local(__last), __value);
-  //   return;
-  // }
-
-  // // We have more than one segment. Iterate over the first segment, since we might not start at the beginning
-  // __fill(_Traits::__local(__first), _Traits::__end(__sfirst), __value);
-  // ++__sfirst;
-  // // iterate over the segments which are guaranteed to be completely in the range
-  // while (__sfirst != __slast) {
-  //   __fill(_Traits::__begin(__sfirst), _Traits::__end(__sfirst), __value);
-  //   ++__sfirst;
-  // }
-  // // iterate over the last segment
-  // __fill(_Traits::__begin(__sfirst), _Traits::__local(__last), __value);
-
   std::for_each(__first, __last, [__value](_Tp& __val) { __val = __value; });
 }
 
diff --git a/libcxx/test/benchmarks/algorithms/fill.bench.cpp b/libcxx/test/benchmarks/algorithms/fill.bench.cpp
index 40f37425c394cf..313fc7f849cbe5 100644
--- a/libcxx/test/benchmarks/algorithms/fill.bench.cpp
+++ b/libcxx/test/benchmarks/algorithms/fill.bench.cpp
@@ -9,6 +9,7 @@
 #include <algorithm>
 #include <benchmark/benchmark.h>
 #include <vector>
+#include <deque>
 
 static void bm_fill_n(benchmark::State& state) {
   std::vector<bool> vec1(state.range());
@@ -37,6 +38,15 @@ static void bm_fill(benchmark::State& state) {
 }
 BENCHMARK(bm_fill)->DenseRange(1, 8)->Range(16, 1 << 20);
 
+static void bm_deque_fill(benchmark::State& state) {
+  std::deque<bool> vec1(state.range());
+  for (auto _ : state) {
+    benchmark::DoNotOptimize(vec1);
+    std::fill(vec1.begin(), vec1.end(), false);
+  }
+}
+BENCHMARK(bm_fill)->DenseRange(1, 8)->Range(16, 1 << 20);
+
 static void bm_ranges_fill(benchmark::State& state) {
   std::vector<bool> vec1(state.range());
   for (auto _ : state) {

>From 24fcb1d7f47c38f238e3022f6ed4a2d96793e69c Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Sat, 24 Aug 2024 12:00:13 +0500
Subject: [PATCH 7/9] Added call to benchmark

---
 libcxx/include/__algorithm/fill.h                | 1 +
 libcxx/test/benchmarks/algorithms/fill.bench.cpp | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index 41f37c46fe471d..954814ff4c00d1 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -59,3 +59,4 @@ fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_FILL_H
+
diff --git a/libcxx/test/benchmarks/algorithms/fill.bench.cpp b/libcxx/test/benchmarks/algorithms/fill.bench.cpp
index 313fc7f849cbe5..ccfb866c147b12 100644
--- a/libcxx/test/benchmarks/algorithms/fill.bench.cpp
+++ b/libcxx/test/benchmarks/algorithms/fill.bench.cpp
@@ -45,7 +45,7 @@ static void bm_deque_fill(benchmark::State& state) {
     std::fill(vec1.begin(), vec1.end(), false);
   }
 }
-BENCHMARK(bm_fill)->DenseRange(1, 8)->Range(16, 1 << 20);
+BENCHMARK(bm_deque_fill)->DenseRange(1, 8)->Range(16, 1 << 20);
 
 static void bm_ranges_fill(benchmark::State& state) {
   std::vector<bool> vec1(state.range());

>From 668139820e5f943912e1c7698d6619182aa733b0 Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Sat, 24 Aug 2024 12:24:48 +0500
Subject: [PATCH 8/9] Added release note

---
 libcxx/docs/ReleaseNotes/20.rst   | 3 +++
 libcxx/include/__algorithm/fill.h | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
index fe9f4c1973cdb4..f7e82d7b932cca 100644
--- a/libcxx/docs/ReleaseNotes/20.rst
+++ b/libcxx/docs/ReleaseNotes/20.rst
@@ -46,6 +46,9 @@ Improvements and New Features
 
 - The ``lexicographical_compare`` and ``ranges::lexicographical_compare`` algorithms have been optimized for trivially
   equality comparable types, resulting in a performance improvement of up to 40x.
+  
+- ``std::fill`` has been optimized for segmented iterators like ``std::deque::iterator`` in C++23 and
+  later, which can lead up to 40x performance improvements.
 
 
 Deprecations and Removals
diff --git a/libcxx/include/__algorithm/fill.h b/libcxx/include/__algorithm/fill.h
index 954814ff4c00d1..41f37c46fe471d 100644
--- a/libcxx/include/__algorithm/fill.h
+++ b/libcxx/include/__algorithm/fill.h
@@ -59,4 +59,3 @@ fill(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_FILL_H
-

>From 47a770f6dfdd572fb7d16399a1088d9e1f35c1bb Mon Sep 17 00:00:00 2001
From: nouman-10x <noumanamir453 at gmail.com>
Date: Mon, 26 Aug 2024 09:57:13 +0500
Subject: [PATCH 9/9] Added tests

---
 .../alg.modifying.operations/alg.fill/fill.pass.cpp    | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
index 481d565961b2b5..86cc7d46d15f65 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
@@ -17,6 +17,7 @@
 #include <array>
 #include <cassert>
 #include <vector>
+#include <deque>
 
 #include "test_macros.h"
 #include "test_iterators.h"
@@ -101,5 +102,14 @@ int main(int, char**) {
   static_assert(test());
 #endif
 
+  // check that segmented iterators work properly
+  int sizes[] = {0, 1, 2, 1023, 1024, 1025, 2047, 2048, 2049};
+  for (const int size : sizes) {
+    std::deque<bool> in(size, false);
+    std::deque<bool> expected(size, true);
+    std::fill(in.begin(), in.end(), true);
+    assert(in == expected);
+  }
+
   return 0;
 }



More information about the libcxx-commits mailing list