[PATCH 4/4] libstdc++: Rearrange some range adaptors' data members

Patrick Palka ppalka@redhat.com
Mon Sep 28 13:11:34 GMT 2020


On Mon, 28 Sep 2020, Jonathan Wakely wrote:

> On 28/09/20 00:48 -0400, Patrick Palka via Libstdc++ wrote:
> > Since the standard range adaptors are specified to derive from the empty
> > class view_base, making their first data member store the underlying
> > view is suboptimal, for if the underlying view also derives from
> > view_base then the two view_base subobjects will be adjacent, thus
> > preventing the compiler from applying the empty base optimization to
> > elide away the storage for these two empty bases.
> > 
> > This patch improves the situation by declaring the _M_base data member
> > last instead of first in each range adaptor that has more than one data
> > member, so that the empty base optimization can apply more often.
> > 
> > Tested on x86_64-pc-linux-gnu with and wihout -m32.
> > 
> > libstdc++-v3/ChangeLog:
> > 
> > 	* include/std/ranges (filter_view::_M_base): Declare this data
> > 	member last.
> > 	(transform_view::_M_base): Likewise.
> > 	(take_view::_M_base): Likewise.
> > 	(take_while_view::_M_base): Likewise.
> > 	(drop_view::_M_base): Likewise.
> > 	(drop_while_view::_M_base): Likewise.
> > 	(join_view::_M_base): Likewise.
> > 	(split_view::_M_base): Likewise.
> > 	* testsuite/std/ranges/adaptors/sizeof.cc: Adjust expected
> > 	sizes.
> > ---
> > libstdc++-v3/include/std/ranges                | 17 ++++++++---------
> > .../testsuite/std/ranges/adaptors/sizeof.cc    | 18 +++++++++---------
> > 2 files changed, 17 insertions(+), 18 deletions(-)
> > 
> > diff --git a/libstdc++-v3/include/std/ranges
> > b/libstdc++-v3/include/std/ranges
> > index 964a2b616a6..6fd8a85c2bf 100644
> > --- a/libstdc++-v3/include/std/ranges
> > +++ b/libstdc++-v3/include/std/ranges
> > @@ -1250,9 +1250,9 @@ namespace views
> > 	{ return __y.__equal(__x); }
> >       };
> > 
> > -      _Vp _M_base = _Vp();
> >       [[no_unique_address]] __detail::__box<_Pred> _M_pred;
> >       [[no_unique_address]] __detail::_CachedPosition<_Vp> _M_cached_begin;
> > +      _Vp _M_base = _Vp();
> 
> The constructor's mem-initializer-list needs to be reordered, to avoid
> warnings with -Wsystem-headers.

Oops, fixed thusly.  I noticed meanwhile that I forgot to move
reverse_view's _M_base member, so the below patch adds that as well.

Tested on x86_64-pc-linux-gnu, and also verified that there are no new
-Wsystem-headers warnings.

-- >8 --

libstdc++-v3/ChangeLog:

	* include/std/ranges (filter_view): Declare data member _M_base
	last instead of first, and adjust constructors' member
	initializer lists accordingly.
	(transform_view): Likewise.
	(take_view): Likewise.
	(take_while_view): Likewise.
	(drop_view): Likewise.
	(drop_while_view): Likewise.
	(join_view): Likewise.
	(split_view): Likewise (and tweak formatting of the declaration
	of _M_current).
	(reverse_view): Likewise.
	* testsuite/std/ranges/adaptors/sizeof.cc: Update expected
	sizes.
---
 libstdc++-v3/include/std/ranges               | 47 +++++++++----------
 .../testsuite/std/ranges/adaptors/sizeof.cc   | 20 ++++----
 2 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 964a2b616a6..7fd5d5110ed 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1250,16 +1250,16 @@ namespace views
 	{ return __y.__equal(__x); }
       };
 
-      _Vp _M_base = _Vp();
       [[no_unique_address]] __detail::__box<_Pred> _M_pred;
       [[no_unique_address]] __detail::_CachedPosition<_Vp> _M_cached_begin;
+      _Vp _M_base = _Vp();
 
     public:
       filter_view() = default;
 
       constexpr
       filter_view(_Vp __base, _Pred __pred)
-	: _M_base(std::move(__base)), _M_pred(std::move(__pred))
+	: _M_pred(std::move(__pred)), _M_base(std::move(__base))
       { }
 
       constexpr _Vp
@@ -1588,15 +1588,15 @@ namespace views
 	  friend _Sentinel<!_Const>;
 	};
 
-      _Vp _M_base = _Vp();
       [[no_unique_address]] __detail::__box<_Fp> _M_fun;
+      _Vp _M_base = _Vp();
 
     public:
       transform_view() = default;
 
       constexpr
       transform_view(_Vp __base, _Fp __fun)
-	: _M_base(std::move(__base)), _M_fun(std::move(__fun))
+	: _M_fun(std::move(__fun)), _M_base(std::move(__base))
       { }
 
       constexpr _Vp
@@ -1695,15 +1695,15 @@ namespace views
 	  friend _Sentinel<!_Const>;
 	};
 
-      _Vp _M_base = _Vp();
       range_difference_t<_Vp> _M_count = 0;
+      _Vp _M_base = _Vp();
 
     public:
       take_view() = default;
 
       constexpr
       take_view(_Vp base, range_difference_t<_Vp> __count)
-	: _M_base(std::move(base)), _M_count(std::move(__count))
+	: _M_count(std::move(__count)), _M_base(std::move(base))
       { }
 
       constexpr _Vp
@@ -1842,17 +1842,16 @@ namespace views
 	  friend _Sentinel<!_Const>;
 	};
 
-      _Vp _M_base = _Vp();
       [[no_unique_address]] __detail::__box<_Pred> _M_pred;
+      _Vp _M_base = _Vp();
 
     public:
       take_while_view() = default;
 
       constexpr
       take_while_view(_Vp base, _Pred __pred)
-	: _M_base(std::move(base)), _M_pred(std::move(__pred))
-      {
-      }
+	: _M_pred(std::move(__pred)), _M_base(std::move(base))
+      { }
 
       constexpr _Vp
       base() const& requires copy_constructible<_Vp>
@@ -1902,8 +1901,8 @@ namespace views
     class drop_view : public view_interface<drop_view<_Vp>>
     {
     private:
-      _Vp _M_base = _Vp();
       range_difference_t<_Vp> _M_count = 0;
+      _Vp _M_base = _Vp();
 
       // ranges::next(begin(base), count, end(base)) is O(1) if _Vp satisfies
       // both random_access_range and sized_range. Otherwise, cache its result.
@@ -1919,7 +1918,7 @@ namespace views
 
       constexpr
       drop_view(_Vp __base, range_difference_t<_Vp> __count)
-	: _M_base(std::move(__base)), _M_count(__count)
+	: _M_count(__count), _M_base(std::move(__base))
       { __glibcxx_assert(__count >= 0); }
 
       constexpr _Vp
@@ -2002,16 +2001,16 @@ namespace views
     class drop_while_view : public view_interface<drop_while_view<_Vp, _Pred>>
     {
     private:
-      _Vp _M_base = _Vp();
       [[no_unique_address]] __detail::__box<_Pred> _M_pred;
       [[no_unique_address]] __detail::_CachedPosition<_Vp> _M_cached_begin;
+      _Vp _M_base = _Vp();
 
     public:
       drop_while_view() = default;
 
       constexpr
       drop_while_view(_Vp __base, _Pred __pred)
-	: _M_base(std::move(__base)), _M_pred(std::move(__pred))
+	: _M_pred(std::move(__pred)), _M_base(std::move(__base))
       { }
 
       constexpr _Vp
@@ -2300,12 +2299,11 @@ namespace views
 	  friend _Sentinel<!_Const>;
 	};
 
-      _Vp _M_base = _Vp();
-
       // XXX: _M_inner is "present only when !is_reference_v<_InnerRange>"
       [[no_unique_address]]
 	__detail::__maybe_present_t<!is_reference_v<_InnerRange>,
 				    views::all_t<_InnerRange>> _M_inner;
+      _Vp _M_base = _Vp();
 
     public:
       join_view() = default;
@@ -2680,13 +2678,12 @@ namespace views
 	  { ranges::iter_swap(__x._M_i_current(), __y._M_i_current()); }
 	};
 
-      _Vp _M_base = _Vp();
       _Pattern _M_pattern = _Pattern();
-
       // XXX: _M_current is "present only if !forward_range<V>"
       [[no_unique_address]]
-	__detail::__maybe_present_t<!forward_range<_Vp>, iterator_t<_Vp>>
-	  _M_current;
+	__detail::__maybe_present_t<!forward_range<_Vp>,
+				    iterator_t<_Vp>> _M_current;
+      _Vp _M_base = _Vp();
 
 
     public:
@@ -2694,7 +2691,7 @@ namespace views
 
       constexpr
       split_view(_Vp __base, _Pattern __pattern)
-	: _M_base(std::move(__base)), _M_pattern(std::move(__pattern))
+	: _M_pattern(std::move(__pattern)), _M_base(std::move(__base))
       { }
 
       template<input_range _Range>
@@ -2702,8 +2699,8 @@ namespace views
 	  && constructible_from<_Pattern, single_view<range_value_t<_Range>>>
 	constexpr
 	split_view(_Range&& __r, range_value_t<_Range> __e)
-	  : _M_base(views::all(std::forward<_Range>(__r))),
-	    _M_pattern(std::move(__e))
+	  : _M_pattern(std::move(__e)),
+	    _M_base(views::all(std::forward<_Range>(__r)))
 	{ }
 
       constexpr _Vp
@@ -2892,14 +2889,14 @@ namespace views
     class reverse_view : public view_interface<reverse_view<_Vp>>
     {
     private:
-      _Vp _M_base = _Vp();
-
       static constexpr bool _S_needs_cached_begin
 	= !common_range<_Vp> && !random_access_range<_Vp>;
+
       [[no_unique_address]]
 	__detail::__maybe_present_t<_S_needs_cached_begin,
 				    __detail::_CachedPosition<_Vp>>
 				      _M_cached_begin;
+      _Vp _M_base = _Vp();
 
     public:
       reverse_view() = default;
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
index 2182981b7a2..4d6f78b36b1 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
@@ -33,20 +33,20 @@ using V = ranges::subrange<int*, int*>;
 constexpr auto ptr = sizeof(int*);
 static_assert(sizeof(V) == 2*ptr);
 
-static_assert(sizeof(ranges::take_view<V>) == 4*ptr);
-static_assert(sizeof(ranges::drop_view<V>) == 4*ptr);
+static_assert(sizeof(ranges::take_view<V>) == 3*ptr);
+static_assert(sizeof(ranges::drop_view<V>) == 3*ptr);
 
-static_assert(sizeof(ranges::filter_view<V, decltype(&pred_f)>) == 5*ptr);
-static_assert(sizeof(ranges::take_while_view<V, decltype(&pred_f)>) == 4*ptr);
-static_assert(sizeof(ranges::drop_while_view<V, decltype(&pred_f)>) == 5*ptr);
-static_assert(sizeof(ranges::transform_view<V, decltype(&func_f)>) == 4*ptr);
+static_assert(sizeof(ranges::filter_view<V, decltype(&pred_f)>) == 4*ptr);
+static_assert(sizeof(ranges::take_while_view<V, decltype(&pred_f)>) == 3*ptr);
+static_assert(sizeof(ranges::drop_while_view<V, decltype(&pred_f)>) == 4*ptr);
+static_assert(sizeof(ranges::transform_view<V, decltype(&func_f)>) == 3*ptr);
 
-static_assert(sizeof(ranges::filter_view<V, decltype(pred_l)>) == 4*ptr);
+static_assert(sizeof(ranges::filter_view<V, decltype(pred_l)>) == 3*ptr);
 static_assert(sizeof(ranges::take_while_view<V, decltype(pred_l)>) == 3*ptr);
-static_assert(sizeof(ranges::drop_while_view<V, decltype(pred_l)>) == 4*ptr);
+static_assert(sizeof(ranges::drop_while_view<V, decltype(pred_l)>) == 3*ptr);
 static_assert(sizeof(ranges::transform_view<V, decltype(func_l)>) == 3*ptr);
 
-static_assert(sizeof(ranges::split_view<V, std::string_view>) == 5*ptr);
+static_assert(sizeof(ranges::split_view<V, std::string_view>) == 4*ptr);
 
 static_assert
- (sizeof(ranges::reverse_view<ranges::filter_view<V, decltype(pred_l)>>) == 5*ptr);
+ (sizeof(ranges::reverse_view<ranges::filter_view<V, decltype(pred_l)>>) == 4*ptr);
-- 
2.28.0.618.g9bc233ae1c



More information about the Libstdc++ mailing list