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

via libcxx-commits libcxx-commits at lists.llvm.org
Sat Aug 24 00:00:55 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/7] [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/7] 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/7] 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/7] 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/7] 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/7] 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/7] 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());



More information about the libcxx-commits mailing list