* std::vector<bool> code cleanup fixes optimizations
@ 2019-05-14 5:46 François Dumont
2019-06-24 19:31 ` François Dumont
0 siblings, 1 reply; 8+ messages in thread
From: François Dumont @ 2019-05-14 5:46 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2888 bytes --]
Hi
   This is the patch on vector<bool> to:
- Optimize sizeof in Versioned namespace mode. We could go one step
further by removing _M_p from _M_finish and just transform it into an
offset but it is a little bit more impacting for the code.
- Implement the swap optimization already done on main std::vector
template class.
- Fix move constructor so that it is noexcept no matter allocator move
constructor noexcept qualification
- Optimize move constructor with allocator when allocator type is always
equal.
- Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods. Those
are now defined only in pre-C++11 mode, I can't see any abi issue in
doing so.
   * include/bits/stl_bvector.h
   [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
   _Bit_type*.
   (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
   (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
   (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): Default.
(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
   (_Bvector_impl_data::_M_reset()): Likewise.
   (_Bvector_impl_data::_M_swap_data): New.
   (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement explicitely.
   (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&, _Bvector_impl&&)):
New.
   (_Bvector_base::_Bvector_base(_Bvector_base&&, const allocator_type&)):
   New, use latter.
   (vector::vector(vector&&, const allocator_type&, true_type)): New, use
   latter.
   (vector::vector(vector&&, const allocator_type&, false_type)): New.
   (vector::vector(vector&&, const allocator_type&)): Use latters.
   (vector::vector(const vector&, const allocator_type&)): Adapt.
   [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
   const allocator_type&)): Use _M_initialize_range.
   (vector::operator[](size_type)): Use iterator operator[].
   (vector::operator[](size_type) const): Use const_iterator operator[].
   (vector::swap(vector&)): Adapt.
   (vector::_M_initialize(size_type)): Add assertions on allocators.
   Use _M_swap_data.
   [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
   _InputIt)): Use _M_insert_range.
   [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
   [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
   * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
   * testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
   Add check.
Tested under Linux x86_64, normal and debug modes.
Ok to commit ?
François
[-- Attachment #2: bvector.patch --]
[-- Type: text/x-patch, Size: 12077 bytes --]
diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index 280d40f60c5..d6fcc2d848d 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -441,7 +441,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
struct _Bvector_impl_data
{
+#if !_GLIBCXX_INLINE_VERSION
_Bit_iterator _M_start;
+#else
+ // We don't need the offset field for the start, it's always zero.
+ struct {
+ _Bit_type* _M_p;
+ // Allow assignment from iterators (assume offset is zero):
+ void operator=(_Bit_iterator __it) { _M_p = __it._M_p; }
+ } _M_start;
+#endif
_Bit_iterator _M_finish;
_Bit_pointer _M_end_of_storage;
@@ -450,33 +459,38 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ }
#if __cplusplus >= 201103L
+ _Bvector_impl_data(const _Bvector_impl_data&) = default;
+ _Bvector_impl_data&
+ operator=(const _Bvector_impl_data&) = default;
+
_Bvector_impl_data(_Bvector_impl_data&& __x) noexcept
- : _M_start(__x._M_start), _M_finish(__x._M_finish)
- , _M_end_of_storage(__x._M_end_of_storage)
+ : _Bvector_impl_data(__x)
{ __x._M_reset(); }
void
_M_move_data(_Bvector_impl_data&& __x) noexcept
{
- this->_M_start = __x._M_start;
- this->_M_finish = __x._M_finish;
- this->_M_end_of_storage = __x._M_end_of_storage;
+ *this = __x;
__x._M_reset();
}
#endif
void
_M_reset() _GLIBCXX_NOEXCEPT
+ { *this = _Bvector_impl_data(); }
+
+ void
+ _M_swap_data(_Bvector_impl_data& __x) _GLIBCXX_NOEXCEPT
{
- _M_start = _M_finish = _Bit_iterator();
- _M_end_of_storage = _Bit_pointer();
+ // Do not use std::swap(_M_start, __x._M_start), etc as it loses
+ // information used by TBAA.
+ std::swap(*this, __x);
}
};
struct _Bvector_impl
: public _Bit_alloc_type, public _Bvector_impl_data
{
- public:
_Bvector_impl() _GLIBCXX_NOEXCEPT_IF(
is_nothrow_default_constructible<_Bit_alloc_type>::value)
: _Bit_alloc_type()
@@ -487,7 +501,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ }
#if __cplusplus >= 201103L
- _Bvector_impl(_Bvector_impl&&) = default;
+ // Not defaulted, to enforce noexcept(true) even when
+ // !is_nothrow_move_constructible<_Bit_alloc_type>.
+ _Bvector_impl(_Bvector_impl&& __x) noexcept
+ : _Bit_alloc_type(std::move(__x)), _Bvector_impl_data(std::move(__x))
+ { }
+
+ _Bvector_impl(_Bit_alloc_type&& __a, _Bvector_impl&& __x) noexcept
+ : _Bit_alloc_type(std::move(__a)), _Bvector_impl_data(std::move(__x))
+ { }
#endif
_Bit_type*
@@ -525,6 +547,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
#if __cplusplus >= 201103L
_Bvector_base(_Bvector_base&&) = default;
+
+ _Bvector_base(_Bvector_base&& __x, const allocator_type& __a)
+ : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
+ { }
#endif
~_Bvector_base()
@@ -661,14 +687,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Bit_alloc_traits::_S_select_on_copy(__x._M_get_Bit_allocator()))
{
_M_initialize(__x.size());
- _M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+ _M_copy_aligned(__x.begin(), __x.end(), begin());
}
#if __cplusplus >= 201103L
vector(vector&&) = default;
- vector(vector&& __x, const allocator_type& __a)
- noexcept(_Bit_alloc_traits::_S_always_equal())
+ private:
+ vector(vector&& __x, const allocator_type& __a, true_type) noexcept
+ : _Base(std::move(__x), __a)
+ { }
+
+ vector(vector&& __x, const allocator_type& __a, false_type)
: _Base(__a)
{
if (__x.get_allocator() == __a)
@@ -681,11 +711,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
}
}
+ public:
+ vector(vector&& __x, const allocator_type& __a)
+ noexcept(_Bit_alloc_traits::_S_always_equal())
+ : vector(std::move(__x), __a,
+ typename _Bit_alloc_traits::is_always_equal{})
+ { }
+
vector(const vector& __x, const allocator_type& __a)
: _Base(__a)
{
_M_initialize(__x.size());
- _M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+ _M_copy_aligned(__x.begin(), __x.end(), begin());
}
vector(initializer_list<bool> __l,
@@ -703,13 +740,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
vector(_InputIterator __first, _InputIterator __last,
const allocator_type& __a = allocator_type())
: _Base(__a)
- { _M_initialize_dispatch(__first, __last, __false_type()); }
+ {
+ _M_initialize_range(__first, __last,
+ std::__iterator_category(__first));
+ }
#else
template<typename _InputIterator>
vector(_InputIterator __first, _InputIterator __last,
const allocator_type& __a = allocator_type())
: _Base(__a)
{
+ // Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_initialize_dispatch(__first, __last, _Integral());
}
@@ -800,6 +841,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
assign(_InputIterator __first, _InputIterator __last)
{
+ // Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_assign_dispatch(__first, __last, _Integral());
}
@@ -888,17 +930,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
reference
operator[](size_type __n)
- {
- return *iterator(this->_M_impl._M_start._M_p
- + __n / int(_S_word_bit), __n % int(_S_word_bit));
- }
+ { return begin()[__n]; }
const_reference
operator[](size_type __n) const
- {
- return *const_iterator(this->_M_impl._M_start._M_p
- + __n / int(_S_word_bit), __n % int(_S_word_bit));
- }
+ { return begin()[__n]; }
protected:
void
@@ -965,10 +1001,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
swap(vector& __x) _GLIBCXX_NOEXCEPT
{
- std::swap(this->_M_impl._M_start, __x._M_impl._M_start);
- std::swap(this->_M_impl._M_finish, __x._M_impl._M_finish);
- std::swap(this->_M_impl._M_end_of_storage,
- __x._M_impl._M_end_of_storage);
+#if __cplusplus >= 201103L
+ __glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
+ || _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
+#endif
+ this->_M_impl._M_swap_data(__x._M_impl);
_Bit_alloc_traits::_S_on_swap(_M_get_Bit_allocator(),
__x._M_get_Bit_allocator());
}
@@ -1006,8 +1043,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_InputIterator __first, _InputIterator __last)
{
difference_type __offset = __position - cbegin();
- _M_insert_dispatch(__position._M_const_cast(),
- __first, __last, __false_type());
+ _M_insert_range(__position._M_const_cast(),
+ __first, __last,
+ std::__iterator_category(__first));
return begin() + __offset;
}
#else
@@ -1016,6 +1054,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
insert(iterator __position,
_InputIterator __first, _InputIterator __last)
{
+ // Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_insert_dispatch(__position, __first, __last, _Integral());
}
@@ -1127,15 +1166,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
_Bit_pointer __q = this->_M_allocate(__n);
this->_M_impl._M_end_of_storage = __q + _S_nword(__n);
- this->_M_impl._M_start = iterator(std::__addressof(*__q), 0);
+ iterator __start = iterator(std::__addressof(*__q), 0);
+ this->_M_impl._M_start = __start;
+ this->_M_impl._M_finish = __start + difference_type(__n);
}
- else
- {
- this->_M_impl._M_end_of_storage = _Bit_pointer();
- this->_M_impl._M_start = iterator(0, 0);
- }
- this->_M_impl._M_finish = this->_M_impl._M_start + difference_type(__n);
-
}
void
@@ -1155,8 +1189,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_M_shrink_to_fit();
#endif
- // Check whether it's an integral type. If so, it's not an iterator.
-
+#if __cplusplus < 201103L
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 438. Ambiguity in the "do the right thing" clause
template<typename _Integer>
@@ -1173,6 +1206,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__false_type)
{ _M_initialize_range(__first, __last,
std::__iterator_category(__first)); }
+#endif
template<typename _InputIterator>
void
@@ -1190,7 +1224,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
const size_type __n = std::distance(__first, __last);
_M_initialize(__n);
- std::copy(__first, __last, this->_M_impl._M_start);
+ std::copy(__first, __last, begin());
}
#if __cplusplus < 201103L
@@ -1254,8 +1288,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
}
}
- // Check whether it's an integral type. If so, it's not an iterator.
-
+#if __cplusplus < 201103L
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 438. Ambiguity in the "do the right thing" clause
template<typename _Integer>
@@ -1271,6 +1304,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__false_type)
{ _M_insert_range(__pos, __first, __last,
std::__iterator_category(__first)); }
+#endif
void
_M_fill_insert(iterator __position, size_type __n, bool __x);
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
index de441426532..745fdc85cf6 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
@@ -28,19 +28,17 @@ namespace __gnu_test
// It is undefined behaviour to swap() containers with unequal allocators
// if the allocator doesn't propagate, so ensure the allocators compare
// equal, while still being able to test propagation via get_personality().
+ template<typename Type>
bool
- operator==(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
- {
- return true;
- }
+ operator==(const propagating_allocator<Type, false>&,
+ const propagating_allocator<Type, false>&)
+ { return true; }
+ template<typename Type>
bool
- operator!=(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
- {
- return false;
- }
+ operator!=(const propagating_allocator<Type, false>&,
+ const propagating_allocator<Type, false>&)
+ { return false; }
}
using __gnu_test::propagating_allocator;
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
index abc4114e31d..90c863d8a35 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
@@ -23,4 +23,34 @@
typedef std::vector<bool> vbtype;
-static_assert(std::is_nothrow_move_constructible<vbtype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<vbtype>::value,
+ "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<vbtype,
+ vbtype&&, const typename vbtype::allocator_type&>::value,
+ "noexcept move constructor with allocator" );
+
+template<typename Type>
+ class not_noexcept_move_constructor_alloc : public std::allocator<Type>
+ {
+ public:
+ not_noexcept_move_constructor_alloc() noexcept { }
+
+ not_noexcept_move_constructor_alloc(
+ const not_noexcept_move_constructor_alloc& x) noexcept
+ : std::allocator<Type>(x)
+ { }
+
+ not_noexcept_move_constructor_alloc(
+ not_noexcept_move_constructor_alloc&& x) noexcept(false)
+ : std::allocator<Type>(std::move(x))
+ { }
+
+ template<typename _Tp1>
+ struct rebind
+ { typedef not_noexcept_move_constructor_alloc<_Tp1> other; };
+ };
+
+typedef std::vector<bool, not_noexcept_move_constructor_alloc<bool>> vbtype2;
+
+static_assert( std::is_nothrow_move_constructible<vbtype2>::value,
+ "noexcept move constructor with not noexcept alloc" );
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: std::vector<bool> code cleanup fixes optimizations
2019-05-14 5:46 std::vector<bool> code cleanup fixes optimizations François Dumont
@ 2019-06-24 19:31 ` François Dumont
2019-12-16 7:18 ` François Dumont
0 siblings, 1 reply; 8+ messages in thread
From: François Dumont @ 2019-06-24 19:31 UTC (permalink / raw)
To: libstdc++, gcc-patches
Hi
   Any feedback regarding this patch ?
Thanks
On 5/14/19 7:46 AM, François Dumont wrote:
> Hi
>
> Â Â Â This is the patch on vector<bool> to:
>
> - Optimize sizeof in Versioned namespace mode. We could go one step
> further by removing _M_p from _M_finish and just transform it into an
> offset but it is a little bit more impacting for the code.
>
> - Implement the swap optimization already done on main std::vector
> template class.
>
> - Fix move constructor so that it is noexcept no matter allocator move
> constructor noexcept qualification
>
> - Optimize move constructor with allocator when allocator type is
> always equal.
>
> - Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods.
> Those are now defined only in pre-C++11 mode, I can't see any abi
> issue in doing so.
>
> Â Â Â * include/bits/stl_bvector.h
> Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
> Â Â Â _Bit_type*.
> Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
> Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
> Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): Default.
> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
> Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
> Â Â Â (_Bvector_impl_data::_M_swap_data): New.
> Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
> explicitely.
> Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
> _Bvector_impl&&)): New.
> Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const
> allocator_type&)):
> Â Â Â New, use latter.
> Â Â Â (vector::vector(vector&&, const allocator_type&, true_type)): New,
> use
> Â Â Â latter.
> Â Â Â (vector::vector(vector&&, const allocator_type&, false_type)): New.
> Â Â Â (vector::vector(vector&&, const allocator_type&)): Use latters.
> Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
> Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
> Â Â Â const allocator_type&)): Use _M_initialize_range.
> Â Â Â (vector::operator[](size_type)): Use iterator operator[].
> Â Â Â (vector::operator[](size_type) const): Use const_iterator operator[].
> Â Â Â (vector::swap(vector&)): Adapt.
> Â Â Â (vector::_M_initialize(size_type)): Add assertions on allocators.
> Â Â Â Use _M_swap_data.
> Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
> Â Â Â _InputIt)): Use _M_insert_range.
> Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
> Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
> Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
> Â Â Â *
> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
> Â Â Â Add check.
>
> Tested under Linux x86_64, normal and debug modes.
>
> Ok to commit ?
>
> François
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: std::vector<bool> code cleanup fixes optimizations
2019-06-24 19:31 ` François Dumont
@ 2019-12-16 7:18 ` François Dumont
2020-07-17 10:36 ` Jonathan Wakely
0 siblings, 1 reply; 8+ messages in thread
From: François Dumont @ 2019-12-16 7:18 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 5746 bytes --]
A small refresh on this patch now tested also for versioned namespace
which require printers.py to be updated.
Note that this simplification works also for normal mode so I can apply
it independently from the stl_bvector.h part.
   * include/bits/stl_bvector.h
   [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
   _Bit_type*.
   (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
   (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
   (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): Default.
(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
   (_Bvector_impl_data::_M_reset()): Likewise.
   (_Bvector_impl_data::_M_swap_data): New.
   (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement explicitely.
   (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&, _Bvector_impl&&)):
New.
   (_Bvector_base::_Bvector_base(_Bvector_base&&, const allocator_type&)):
   New, use latter.
   (vector::vector(vector&&, const allocator_type&, true_type)): New, use
   latter.
   (vector::vector(vector&&, const allocator_type&, false_type)): New.
   (vector::vector(vector&&, const allocator_type&)): Use latters.
   (vector::vector(const vector&, const allocator_type&)): Adapt.
   [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
   const allocator_type&)): Use _M_initialize_range.
   (vector::operator[](size_type)): Use iterator operator[].
   (vector::operator[](size_type) const): Use const_iterator operator[].
   (vector::swap(vector&)): Add assertions on allocators. Use
_M_swap_data.
   [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
   _InputIt)): Use _M_insert_range.
   (vector::_M_initialize(size_type)): Adapt.
   [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
   [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
   * python/libstdcxx/v6/printers.py (StdVectorPrinter._iterator): Stop
   using start _M_offset.
   (StdVectorPrinter.to_string): Likewise.
   * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
   * testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
   Add check.
François
On 6/24/19 9:31 PM, François Dumont wrote:
> Hi
>
> Â Â Â Any feedback regarding this patch ?
>
> Thanks
>
> On 5/14/19 7:46 AM, François Dumont wrote:
>> Hi
>>
>> Â Â Â This is the patch on vector<bool> to:
>>
>> - Optimize sizeof in Versioned namespace mode. We could go one step
>> further by removing _M_p from _M_finish and just transform it into an
>> offset but it is a little bit more impacting for the code.
>>
>> - Implement the swap optimization already done on main std::vector
>> template class.
>>
>> - Fix move constructor so that it is noexcept no matter allocator
>> move constructor noexcept qualification
>>
>> - Optimize move constructor with allocator when allocator type is
>> always equal.
>>
>> - Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods.
>> Those are now defined only in pre-C++11 mode, I can't see any abi
>> issue in doing so.
>>
>> Â Â Â * include/bits/stl_bvector.h
>> Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
>> Â Â Â _Bit_type*.
>> Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
>> Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>> Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): Default.
>> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>> Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
>> Â Â Â (_Bvector_impl_data::_M_swap_data): New.
>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
>> explicitely.
>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
>> _Bvector_impl&&)): New.
>> Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const
>> allocator_type&)):
>> Â Â Â New, use latter.
>> Â Â Â (vector::vector(vector&&, const allocator_type&, true_type)):
>> New, use
>> Â Â Â latter.
>> Â Â Â (vector::vector(vector&&, const allocator_type&, false_type)): New.
>> Â Â Â (vector::vector(vector&&, const allocator_type&)): Use latters.
>> Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
>> Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
>> Â Â Â const allocator_type&)): Use _M_initialize_range.
>> Â Â Â (vector::operator[](size_type)): Use iterator operator[].
>> Â Â Â (vector::operator[](size_type) const): Use const_iterator
>> operator[].
>> Â Â Â (vector::swap(vector&)): Adapt.
>> Â Â Â (vector::_M_initialize(size_type)): Add assertions on allocators.
>> Â Â Â Use _M_swap_data.
>> Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
>> Â Â Â _InputIt)): Use _M_insert_range.
>> Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
>> Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
>> Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
>> Â Â Â *
>> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>> Â Â Â Add check.
>>
>> Tested under Linux x86_64, normal and debug modes.
>>
>> Ok to commit ?
>>
>> François
>>
>
[-- Attachment #2: bvector.patch --]
[-- Type: text/x-patch, Size: 13461 bytes --]
diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index f2eea7799dc..15ecce62683 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -449,7 +449,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
struct _Bvector_impl_data
{
+#if !_GLIBCXX_INLINE_VERSION
_Bit_iterator _M_start;
+#else
+ // We don't need the offset field for the start, it's always zero.
+ struct {
+ _Bit_type* _M_p;
+ // Allow assignment from iterators (assume offset is zero):
+ void operator=(_Bit_iterator __it) { _M_p = __it._M_p; }
+ } _M_start;
+#endif
_Bit_iterator _M_finish;
_Bit_pointer _M_end_of_storage;
@@ -458,33 +467,38 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ }
#if __cplusplus >= 201103L
+ _Bvector_impl_data(const _Bvector_impl_data&) = default;
+ _Bvector_impl_data&
+ operator=(const _Bvector_impl_data&) = default;
+
_Bvector_impl_data(_Bvector_impl_data&& __x) noexcept
- : _M_start(__x._M_start), _M_finish(__x._M_finish)
- , _M_end_of_storage(__x._M_end_of_storage)
+ : _Bvector_impl_data(__x)
{ __x._M_reset(); }
void
_M_move_data(_Bvector_impl_data&& __x) noexcept
{
- this->_M_start = __x._M_start;
- this->_M_finish = __x._M_finish;
- this->_M_end_of_storage = __x._M_end_of_storage;
+ *this = __x;
__x._M_reset();
}
#endif
void
_M_reset() _GLIBCXX_NOEXCEPT
+ { *this = _Bvector_impl_data(); }
+
+ void
+ _M_swap_data(_Bvector_impl_data& __x) _GLIBCXX_NOEXCEPT
{
- _M_start = _M_finish = _Bit_iterator();
- _M_end_of_storage = _Bit_pointer();
+ // Do not use std::swap(_M_start, __x._M_start), etc as it loses
+ // information used by TBAA.
+ std::swap(*this, __x);
}
};
struct _Bvector_impl
: public _Bit_alloc_type, public _Bvector_impl_data
{
- public:
_Bvector_impl() _GLIBCXX_NOEXCEPT_IF(
is_nothrow_default_constructible<_Bit_alloc_type>::value)
: _Bit_alloc_type()
@@ -495,7 +509,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ }
#if __cplusplus >= 201103L
- _Bvector_impl(_Bvector_impl&&) = default;
+ // Not defaulted, to enforce noexcept(true) even when
+ // !is_nothrow_move_constructible<_Bit_alloc_type>.
+ _Bvector_impl(_Bvector_impl&& __x) noexcept
+ : _Bit_alloc_type(std::move(__x)), _Bvector_impl_data(std::move(__x))
+ { }
+
+ _Bvector_impl(_Bit_alloc_type&& __a, _Bvector_impl&& __x) noexcept
+ : _Bit_alloc_type(std::move(__a)), _Bvector_impl_data(std::move(__x))
+ { }
#endif
_Bit_type*
@@ -533,6 +555,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
#if __cplusplus >= 201103L
_Bvector_base(_Bvector_base&&) = default;
+
+ _Bvector_base(_Bvector_base&& __x, const allocator_type& __a)
+ : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
+ { }
#endif
~_Bvector_base()
@@ -669,14 +695,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
: _Base(_Bit_alloc_traits::_S_select_on_copy(__x._M_get_Bit_allocator()))
{
_M_initialize(__x.size());
- _M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+ _M_copy_aligned(__x.begin(), __x.end(), begin());
}
#if __cplusplus >= 201103L
vector(vector&&) = default;
- vector(vector&& __x, const allocator_type& __a)
- noexcept(_Bit_alloc_traits::_S_always_equal())
+ private:
+ vector(vector&& __x, const allocator_type& __a, true_type) noexcept
+ : _Base(std::move(__x), __a)
+ { }
+
+ vector(vector&& __x, const allocator_type& __a, false_type)
: _Base(__a)
{
if (__x.get_allocator() == __a)
@@ -689,11 +719,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
}
}
+ public:
+ vector(vector&& __x, const allocator_type& __a)
+ noexcept(_Bit_alloc_traits::_S_always_equal())
+ : vector(std::move(__x), __a,
+ typename _Bit_alloc_traits::is_always_equal{})
+ { }
+
vector(const vector& __x, const allocator_type& __a)
: _Base(__a)
{
_M_initialize(__x.size());
- _M_copy_aligned(__x.begin(), __x.end(), this->_M_impl._M_start);
+ _M_copy_aligned(__x.begin(), __x.end(), begin());
}
vector(initializer_list<bool> __l,
@@ -711,13 +748,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
vector(_InputIterator __first, _InputIterator __last,
const allocator_type& __a = allocator_type())
: _Base(__a)
- { _M_initialize_dispatch(__first, __last, __false_type()); }
+ {
+ _M_initialize_range(__first, __last,
+ std::__iterator_category(__first));
+ }
#else
template<typename _InputIterator>
vector(_InputIterator __first, _InputIterator __last,
const allocator_type& __a = allocator_type())
: _Base(__a)
{
+ // Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_initialize_dispatch(__first, __last, _Integral());
}
@@ -808,6 +849,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
assign(_InputIterator __first, _InputIterator __last)
{
+ // Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_assign_dispatch(__first, __last, _Integral());
}
@@ -896,17 +938,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
reference
operator[](size_type __n)
- {
- return *iterator(this->_M_impl._M_start._M_p
- + __n / int(_S_word_bit), __n % int(_S_word_bit));
- }
+ { return begin()[__n]; }
const_reference
operator[](size_type __n) const
- {
- return *const_iterator(this->_M_impl._M_start._M_p
- + __n / int(_S_word_bit), __n % int(_S_word_bit));
- }
+ { return begin()[__n]; }
protected:
void
@@ -973,10 +1009,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
void
swap(vector& __x) _GLIBCXX_NOEXCEPT
{
- std::swap(this->_M_impl._M_start, __x._M_impl._M_start);
- std::swap(this->_M_impl._M_finish, __x._M_impl._M_finish);
- std::swap(this->_M_impl._M_end_of_storage,
- __x._M_impl._M_end_of_storage);
+#if __cplusplus >= 201103L
+ __glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
+ || _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
+#endif
+ this->_M_impl._M_swap_data(__x._M_impl);
_Bit_alloc_traits::_S_on_swap(_M_get_Bit_allocator(),
__x._M_get_Bit_allocator());
}
@@ -1014,8 +1051,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_InputIterator __first, _InputIterator __last)
{
difference_type __offset = __position - cbegin();
- _M_insert_dispatch(__position._M_const_cast(),
- __first, __last, __false_type());
+ _M_insert_range(__position._M_const_cast(),
+ __first, __last,
+ std::__iterator_category(__first));
return begin() + __offset;
}
#else
@@ -1024,6 +1062,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
insert(iterator __position,
_InputIterator __first, _InputIterator __last)
{
+ // Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_insert_dispatch(__position, __first, __last, _Integral());
}
@@ -1135,15 +1174,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
_Bit_pointer __q = this->_M_allocate(__n);
this->_M_impl._M_end_of_storage = __q + _S_nword(__n);
- this->_M_impl._M_start = iterator(std::__addressof(*__q), 0);
+ iterator __start = iterator(std::__addressof(*__q), 0);
+ this->_M_impl._M_start = __start;
+ this->_M_impl._M_finish = __start + difference_type(__n);
}
- else
- {
- this->_M_impl._M_end_of_storage = _Bit_pointer();
- this->_M_impl._M_start = iterator(0, 0);
- }
- this->_M_impl._M_finish = this->_M_impl._M_start + difference_type(__n);
-
}
void
@@ -1163,8 +1197,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_M_shrink_to_fit();
#endif
- // Check whether it's an integral type. If so, it's not an iterator.
-
+#if __cplusplus < 201103L
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 438. Ambiguity in the "do the right thing" clause
template<typename _Integer>
@@ -1181,6 +1214,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__false_type)
{ _M_initialize_range(__first, __last,
std::__iterator_category(__first)); }
+#endif
template<typename _InputIterator>
void
@@ -1198,7 +1232,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
const size_type __n = std::distance(__first, __last);
_M_initialize(__n);
- std::copy(__first, __last, this->_M_impl._M_start);
+ std::copy(__first, __last, begin());
}
#if __cplusplus < 201103L
@@ -1262,8 +1296,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
}
}
- // Check whether it's an integral type. If so, it's not an iterator.
-
+#if __cplusplus < 201103L
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 438. Ambiguity in the "do the right thing" clause
template<typename _Integer>
@@ -1279,6 +1312,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__false_type)
{ _M_insert_range(__pos, __first, __last,
std::__iterator_category(__first)); }
+#endif
void
_M_fill_insert(iterator __position, size_type __n, bool __x);
diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 08327516b28..251e3ca098d 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -406,7 +406,7 @@ class StdVectorPrinter:
self.bitvec = bitvec
if bitvec:
self.item = start['_M_p']
- self.so = start['_M_offset']
+ self.so = 0
self.finish = finish['_M_p']
self.fo = finish['_M_offset']
itype = self.item.dereference().type
@@ -454,12 +454,11 @@ class StdVectorPrinter:
end = self.val['_M_impl']['_M_end_of_storage']
if self.is_bool:
start = self.val['_M_impl']['_M_start']['_M_p']
- so = self.val['_M_impl']['_M_start']['_M_offset']
finish = self.val['_M_impl']['_M_finish']['_M_p']
fo = self.val['_M_impl']['_M_finish']['_M_offset']
itype = start.dereference().type
bl = 8 * itype.sizeof
- length = (bl - so) + bl * ((finish - start) - 1) + fo
+ length = bl * (finish - start) + fo
capacity = bl * (end - start)
return ('%s<bool> of length %d, capacity %d'
% (self.typename, int (length), int (capacity)))
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
index de441426532..745fdc85cf6 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
@@ -28,19 +28,17 @@ namespace __gnu_test
// It is undefined behaviour to swap() containers with unequal allocators
// if the allocator doesn't propagate, so ensure the allocators compare
// equal, while still being able to test propagation via get_personality().
+ template<typename Type>
bool
- operator==(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
- {
- return true;
- }
+ operator==(const propagating_allocator<Type, false>&,
+ const propagating_allocator<Type, false>&)
+ { return true; }
+ template<typename Type>
bool
- operator!=(const propagating_allocator<T, false>&,
- const propagating_allocator<T, false>&)
- {
- return false;
- }
+ operator!=(const propagating_allocator<Type, false>&,
+ const propagating_allocator<Type, false>&)
+ { return false; }
}
using __gnu_test::propagating_allocator;
diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
index abc4114e31d..90c863d8a35 100644
--- a/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc
@@ -23,4 +23,34 @@
typedef std::vector<bool> vbtype;
-static_assert(std::is_nothrow_move_constructible<vbtype>::value, "Error");
+static_assert( std::is_nothrow_move_constructible<vbtype>::value,
+ "noexcept move constructor" );
+static_assert( std::is_nothrow_constructible<vbtype,
+ vbtype&&, const typename vbtype::allocator_type&>::value,
+ "noexcept move constructor with allocator" );
+
+template<typename Type>
+ class not_noexcept_move_constructor_alloc : public std::allocator<Type>
+ {
+ public:
+ not_noexcept_move_constructor_alloc() noexcept { }
+
+ not_noexcept_move_constructor_alloc(
+ const not_noexcept_move_constructor_alloc& x) noexcept
+ : std::allocator<Type>(x)
+ { }
+
+ not_noexcept_move_constructor_alloc(
+ not_noexcept_move_constructor_alloc&& x) noexcept(false)
+ : std::allocator<Type>(std::move(x))
+ { }
+
+ template<typename _Tp1>
+ struct rebind
+ { typedef not_noexcept_move_constructor_alloc<_Tp1> other; };
+ };
+
+typedef std::vector<bool, not_noexcept_move_constructor_alloc<bool>> vbtype2;
+
+static_assert( std::is_nothrow_move_constructible<vbtype2>::value,
+ "noexcept move constructor with not noexcept alloc" );
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: std::vector<bool> code cleanup fixes optimizations
2019-12-16 7:18 ` François Dumont
@ 2020-07-17 10:36 ` Jonathan Wakely
2020-07-31 21:03 ` François Dumont
0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2020-07-17 10:36 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 16/12/19 08:18 +0100, François Dumont wrote:
>A small refresh on this patch now tested also for versioned namespace
>which require printers.py to be updated.
>
>Note that this simplification works also for normal mode so I can
>apply it independently from the stl_bvector.h part.
>
>
>Â Â Â * include/bits/stl_bvector.h
>Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
>Â Â Â _Bit_type*.
>Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
>Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): Default.
>(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
>Â Â Â (_Bvector_impl_data::_M_swap_data): New.
>Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement explicitely.
>Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
>_Bvector_impl&&)): New.
>Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const allocator_type&)):
>Â Â Â New, use latter.
>Â Â Â (vector::vector(vector&&, const allocator_type&, true_type)): New, use
>Â Â Â latter.
>Â Â Â (vector::vector(vector&&, const allocator_type&, false_type)): New.
>Â Â Â (vector::vector(vector&&, const allocator_type&)): Use latters.
>Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
>Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
>Â Â Â const allocator_type&)): Use _M_initialize_range.
>Â Â Â (vector::operator[](size_type)): Use iterator operator[].
>Â Â Â (vector::operator[](size_type) const): Use const_iterator operator[].
>Â Â Â (vector::swap(vector&)): Add assertions on allocators. Use
>_M_swap_data.
>Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
>Â Â Â _InputIt)): Use _M_insert_range.
>Â Â Â (vector::_M_initialize(size_type)): Adapt.
>Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
>Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
>Â Â Â * python/libstdcxx/v6/printers.py (StdVectorPrinter._iterator): Stop
>Â Â Â using start _M_offset.
>Â Â Â (StdVectorPrinter.to_string): Likewise.
>Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
>Â Â Â * testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>Â Â Â Add check.
>
>François
>
>
>On 6/24/19 9:31 PM, François Dumont wrote:
>>Hi
>>
>>Â Â Â Any feedback regarding this patch ?
>>
>>Thanks
>>
>>On 5/14/19 7:46 AM, François Dumont wrote:
>>>Hi
>>>
>>>Â Â Â This is the patch on vector<bool> to:
>>>
>>>- Optimize sizeof in Versioned namespace mode. We could go one
>>>step further by removing _M_p from _M_finish and just transform it
>>>into an offset but it is a little bit more impacting for the code.
>>>
>>>- Implement the swap optimization already done on main std::vector
>>>template class.
>>>
>>>- Fix move constructor so that it is noexcept no matter allocator
>>>move constructor noexcept qualification
>>>
>>>- Optimize move constructor with allocator when allocator type is
>>>always equal.
>>>
>>>- Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods.
>>>Those are now defined only in pre-C++11 mode, I can't see any abi
>>>issue in doing so.
>>>
>>>Â Â Â * include/bits/stl_bvector.h
>>>Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as
>>>Â Â Â _Bit_type*.
>>>Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
>>>Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>>>Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): Default.
>>>(_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>>>Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
>>>Â Â Â (_Bvector_impl_data::_M_swap_data): New.
>>>Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
>>>explicitely.
>>>Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
>>>_Bvector_impl&&)): New.
>>>Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const
>>>allocator_type&)):
>>>Â Â Â New, use latter.
>>>Â Â Â (vector::vector(vector&&, const allocator_type&, true_type)):
>>>New, use
>>>Â Â Â latter.
>>>Â Â Â (vector::vector(vector&&, const allocator_type&, false_type)): New.
>>>Â Â Â (vector::vector(vector&&, const allocator_type&)): Use latters.
>>>Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
>>>Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
>>>Â Â Â const allocator_type&)): Use _M_initialize_range.
>>>Â Â Â (vector::operator[](size_type)): Use iterator operator[].
>>>Â Â Â (vector::operator[](size_type) const): Use const_iterator
>>>operator[].
>>>Â Â Â (vector::swap(vector&)): Adapt.
>>>Â Â Â (vector::_M_initialize(size_type)): Add assertions on allocators.
>>>Â Â Â Use _M_swap_data.
>>>Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
>>>Â Â Â _InputIt)): Use _M_insert_range.
>>>Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
>>>Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
>>>Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
>>>Â Â Â *
>>>testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>>>Â Â Â Add check.
>>>
>>>Tested under Linux x86_64, normal and debug modes.
>>>
>>>Ok to commit ?
Yes, OK for master, but I have one question below (and one minor
comment).
>diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
>index f2eea7799dc..15ecce62683 100644
>--- a/libstdc++-v3/include/bits/stl_bvector.h
>+++ b/libstdc++-v3/include/bits/stl_bvector.h
>@@ -449,7 +449,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>
> struct _Bvector_impl_data
> {
>+#if !_GLIBCXX_INLINE_VERSION
> _Bit_iterator _M_start;
>+#else
>+ // We don't need the offset field for the start, it's always zero.
>+ struct {
>+ _Bit_type* _M_p;
>+ // Allow assignment from iterators (assume offset is zero):
>+ void operator=(_Bit_iterator __it) { _M_p = __it._M_p; }
>+ } _M_start;
>+#endif
> _Bit_iterator _M_finish;
> _Bit_pointer _M_end_of_storage;
>
>@@ -533,6 +555,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>
> #if __cplusplus >= 201103L
> _Bvector_base(_Bvector_base&&) = default;
>+
>+ _Bvector_base(_Bvector_base&& __x, const allocator_type& __a)
I think you can add 'noexcept' here.
Constructing the _Bit_alloc_type can't throw, and the base constructor
is noexcept.
>+ : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
>+ { }
> #endif
>
> ~_Bvector_base()
>diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>index de441426532..745fdc85cf6 100644
>--- a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>@@ -28,19 +28,17 @@ namespace __gnu_test
> // It is undefined behaviour to swap() containers with unequal allocators
> // if the allocator doesn't propagate, so ensure the allocators compare
> // equal, while still being able to test propagation via get_personality().
>+ template<typename Type>
> bool
>- operator==(const propagating_allocator<T, false>&,
>- const propagating_allocator<T, false>&)
>- {
>- return true;
>- }
>+ operator==(const propagating_allocator<Type, false>&,
>+ const propagating_allocator<Type, false>&)
>+ { return true; }
>
>+ template<typename Type>
> bool
>- operator!=(const propagating_allocator<T, false>&,
>- const propagating_allocator<T, false>&)
>- {
>- return false;
>- }
>+ operator!=(const propagating_allocator<Type, false>&,
>+ const propagating_allocator<Type, false>&)
>+ { return false; }
Why does this test need to be adapted?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: std::vector<bool> code cleanup fixes optimizations
2020-07-17 10:36 ` Jonathan Wakely
@ 2020-07-31 21:03 ` François Dumont
2020-07-31 21:47 ` Jonathan Wakely
2020-08-06 19:30 ` François Dumont
0 siblings, 2 replies; 8+ messages in thread
From: François Dumont @ 2020-07-31 21:03 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
On 17/07/20 12:36 pm, Jonathan Wakely wrote:
> On 16/12/19 08:18 +0100, François Dumont wrote:
>> A small refresh on this patch now tested also for versioned namespace
>> which require printers.py to be updated.
>>
>> Note that this simplification works also for normal mode so I can
>> apply it independently from the stl_bvector.h part.
>>
>>
>> Â Â Â * include/bits/stl_bvector.h
>> Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start):
>> Define as
>> Â Â Â _Bit_type*.
>> Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
>> Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>> Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)):
>> Default.
>> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>> Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
>> Â Â Â (_Bvector_impl_data::_M_swap_data): New.
>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
>> explicitely.
>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
>> _Bvector_impl&&)): New.
>> Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const
>> allocator_type&)):
>> Â Â Â New, use latter.
>> Â Â Â (vector::vector(vector&&, const allocator_type&, true_type)):
>> New, use
>> Â Â Â latter.
>> Â Â Â (vector::vector(vector&&, const allocator_type&, false_type)):
>> New.
>> Â Â Â (vector::vector(vector&&, const allocator_type&)): Use latters.
>> Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
>> Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
>> Â Â Â const allocator_type&)): Use _M_initialize_range.
>> Â Â Â (vector::operator[](size_type)): Use iterator operator[].
>> Â Â Â (vector::operator[](size_type) const): Use const_iterator
>> operator[].
>> Â Â Â (vector::swap(vector&)): Add assertions on allocators. Use
>> _M_swap_data.
>> Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
>> Â Â Â _InputIt)): Use _M_insert_range.
>> Â Â Â (vector::_M_initialize(size_type)): Adapt.
>> Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
>> Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
>> Â Â Â * python/libstdcxx/v6/printers.py
>> (StdVectorPrinter._iterator): Stop
>> Â Â Â using start _M_offset.
>> Â Â Â (StdVectorPrinter.to_string): Likewise.
>> Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
>> Â Â Â *
>> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>> Â Â Â Add check.
>>
>> François
>>
>>
>> On 6/24/19 9:31 PM, François Dumont wrote:
>>> Hi
>>>
>>> Â Â Â Any feedback regarding this patch ?
>>>
>>> Thanks
>>>
>>> On 5/14/19 7:46 AM, François Dumont wrote:
>>>> Hi
>>>>
>>>> Â Â Â This is the patch on vector<bool> to:
>>>>
>>>> - Optimize sizeof in Versioned namespace mode. We could go one step
>>>> further by removing _M_p from _M_finish and just transform it into
>>>> an offset but it is a little bit more impacting for the code.
>>>>
>>>> - Implement the swap optimization already done on main std::vector
>>>> template class.
>>>>
>>>> - Fix move constructor so that it is noexcept no matter allocator
>>>> move constructor noexcept qualification
>>>>
>>>> - Optimize move constructor with allocator when allocator type is
>>>> always equal.
>>>>
>>>> - Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods.
>>>> Those are now defined only in pre-C++11 mode, I can't see any abi
>>>> issue in doing so.
>>>>
>>>> Â Â Â * include/bits/stl_bvector.h
>>>> Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start):
>>>> Define as
>>>> Â Â Â _Bit_type*.
>>>> Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
>>>> Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>>>> Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)):
>>>> Default.
>>>> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>>>> Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
>>>> Â Â Â (_Bvector_impl_data::_M_swap_data): New.
>>>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
>>>> explicitely.
>>>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
>>>> _Bvector_impl&&)): New.
>>>> Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const
>>>> allocator_type&)):
>>>> Â Â Â New, use latter.
>>>> Â Â Â (vector::vector(vector&&, const allocator_type&,
>>>> true_type)): New, use
>>>> Â Â Â latter.
>>>> Â Â Â (vector::vector(vector&&, const allocator_type&,
>>>> false_type)): New.
>>>> Â Â Â (vector::vector(vector&&, const allocator_type&)): Use latters.
>>>> Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
>>>> Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
>>>> Â Â Â const allocator_type&)): Use _M_initialize_range.
>>>> Â Â Â (vector::operator[](size_type)): Use iterator operator[].
>>>> Â Â Â (vector::operator[](size_type) const): Use const_iterator
>>>> operator[].
>>>> Â Â Â (vector::swap(vector&)): Adapt.
>>>> Â Â Â (vector::_M_initialize(size_type)): Add assertions on
>>>> allocators.
>>>> Â Â Â Use _M_swap_data.
>>>> Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator,
>>>> _InputIt,
>>>> Â Â Â _InputIt)): Use _M_insert_range.
>>>> Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch):
>>>> Remove.
>>>> Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
>>>> Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
>>>> Â Â Â *
>>>> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>>>> Â Â Â Add check.
>>>>
>>>> Tested under Linux x86_64, normal and debug modes.
>>>>
>>>> Ok to commit ?
>
> Yes, OK for master, but I have one question below (and one minor
> comment).
>
>> diff --git a/libstdc++-v3/include/bits/stl_bvector.h
>> b/libstdc++-v3/include/bits/stl_bvector.h
>> index f2eea7799dc..15ecce62683 100644
>> --- a/libstdc++-v3/include/bits/stl_bvector.h
>> +++ b/libstdc++-v3/include/bits/stl_bvector.h
>> @@ -449,7 +449,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>
>> struct _Bvector_impl_data
>> {
>> +#if !_GLIBCXX_INLINE_VERSION
>> _Bit_iterator _M_start;
>> +#else
>> + // We don't need the offset field for the start, it's always zero.
>> + struct {
>> + _Bit_type* _M_p;
>> + // Allow assignment from iterators (assume offset is zero):
>> + void operator=(_Bit_iterator __it) { _M_p = __it._M_p; }
>> + } _M_start;
>> +#endif
>> _Bit_iterator _M_finish;
>> _Bit_pointer _M_end_of_storage;
>>
>> @@ -533,6 +555,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>
>> #if __cplusplus >= 201103L
>> _Bvector_base(_Bvector_base&&) = default;
>> +
>> + _Bvector_base(_Bvector_base&& __x, const allocator_type& __a)
>
> I think you can add 'noexcept' here.
>
> Constructing the _Bit_alloc_type can't throw, and the base constructor
> is noexcept.
Ok, added.
>
>> + : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
>> + { }
>> #endif
>>
>> ~_Bvector_base()
>
>
>> diff --git
>> a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>> b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>> index de441426532..745fdc85cf6 100644
>> --- a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>> +++ b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>> @@ -28,19 +28,17 @@ namespace __gnu_test
>> // It is undefined behaviour to swap() containers with unequal
>> allocators
>> // if the allocator doesn't propagate, so ensure the allocators
>> compare
>> // equal, while still being able to test propagation via
>> get_personality().
>> + template<typename Type>
>> bool
>> - operator==(const propagating_allocator<T, false>&,
>> - const propagating_allocator<T, false>&)
>> - {
>> - return true;
>> - }
>> + operator==(const propagating_allocator<Type, false>&,
>> + const propagating_allocator<Type, false>&)
>> + { return true; }
>>
>> + template<typename Type>
>> bool
>> - operator!=(const propagating_allocator<T, false>&,
>> - const propagating_allocator<T, false>&)
>> - {
>> - return false;
>> - }
>> + operator!=(const propagating_allocator<Type, false>&,
>> + const propagating_allocator<Type, false>&)
>> + { return false; }
>
> Why does this test need to be adapted?
>
Because I replicate and adapt the static assertion from the main vector
implementation which is:
#if __cplusplus >= 201103L
__glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
|| _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
#endif
in the case of vector<bool> the compared allocators are not
propagating_allocator<bool, false> but propagating_allocator<unsigned
long, false>.
Building with __glibcxx_assert was showing this.
François
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: std::vector<bool> code cleanup fixes optimizations
2020-07-31 21:03 ` François Dumont
@ 2020-07-31 21:47 ` Jonathan Wakely
2020-08-06 19:30 ` François Dumont
1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2020-07-31 21:47 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 31/07/20 23:03 +0200, François Dumont via Libstdc++ wrote:
>On 17/07/20 12:36 pm, Jonathan Wakely wrote:
>>On 16/12/19 08:18 +0100, François Dumont wrote:
>>>diff --git
>>>a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>>b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>>index de441426532..745fdc85cf6 100644
>>>--- a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>>+++ b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>>@@ -28,19 +28,17 @@ namespace __gnu_test
>>>Â // It is undefined behaviour to swap() containers with unequal
>>>allocators
>>>Â // if the allocator doesn't propagate, so ensure the allocators
>>>compare
>>>Â // equal, while still being able to test propagation via
>>>get_personality().
>>>+Â template<typename Type>
>>>Â Â Â bool
>>>-Â operator==(const propagating_allocator<T, false>&,
>>>-Â Â Â Â Â Â Â Â const propagating_allocator<T, false>&)
>>>-Â {
>>>-Â Â Â return true;
>>>-Â }
>>>+Â Â Â operator==(const propagating_allocator<Type, false>&,
>>>+Â Â Â Â Â Â Â Â Â Â const propagating_allocator<Type, false>&)
>>>+Â Â Â { return true; }
>>>
>>>+Â template<typename Type>
>>>Â Â Â bool
>>>-Â operator!=(const propagating_allocator<T, false>&,
>>>-Â Â Â Â Â Â Â Â const propagating_allocator<T, false>&)
>>>-Â {
>>>-Â Â Â return false;
>>>-Â }
>>>+Â Â Â operator!=(const propagating_allocator<Type, false>&,
>>>+Â Â Â Â Â Â Â Â Â Â const propagating_allocator<Type, false>&)
>>>+Â Â Â { return false; }
>>
>>Why does this test need to be adapted?
>>
>Because I replicate and adapt the static assertion from the main
>vector implementation which is:
>
>#if __cplusplus >= 201103L
>__glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
>Â Â Â Â Â Â Â Â Â Â || _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
>#endif
>
>in the case of vector<bool> the compared allocators are not
>propagating_allocator<bool, false> but propagating_allocator<unsigned
>long, false>.
Ah yes, of course. Thanks.
>Building with __glibcxx_assert was showing this.
>
>François
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: std::vector<bool> code cleanup fixes optimizations
2020-07-31 21:03 ` François Dumont
2020-07-31 21:47 ` Jonathan Wakely
@ 2020-08-06 19:30 ` François Dumont
2020-08-06 19:39 ` Jonathan Wakely
1 sibling, 1 reply; 8+ messages in thread
From: François Dumont @ 2020-08-06 19:30 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
I wonder if following the application of this patch we shouldn't bump
versioned namespace through _GLIBCXX_BEGIN_NAMESPACE_VERSION ?
Unless it is still considered as experimental.
François
On 31/07/20 11:03 pm, François Dumont wrote:
> On 17/07/20 12:36 pm, Jonathan Wakely wrote:
>> On 16/12/19 08:18 +0100, François Dumont wrote:
>>> A small refresh on this patch now tested also for versioned
>>> namespace which require printers.py to be updated.
>>>
>>> Note that this simplification works also for normal mode so I can
>>> apply it independently from the stl_bvector.h part.
>>>
>>>
>>> Â Â Â * include/bits/stl_bvector.h
>>> Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start):
>>> Define as
>>> Â Â Â _Bit_type*.
>>> Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
>>> Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter.
>>> Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)):
>>> Default.
>>> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>>> Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
>>> Â Â Â (_Bvector_impl_data::_M_swap_data): New.
>>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
>>> explicitely.
>>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
>>> _Bvector_impl&&)): New.
>>> Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const
>>> allocator_type&)):
>>> Â Â Â New, use latter.
>>> Â Â Â (vector::vector(vector&&, const allocator_type&, true_type)):
>>> New, use
>>> Â Â Â latter.
>>> Â Â Â (vector::vector(vector&&, const allocator_type&,
>>> false_type)): New.
>>> Â Â Â (vector::vector(vector&&, const allocator_type&)): Use latters.
>>> Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
>>> Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
>>> Â Â Â const allocator_type&)): Use _M_initialize_range.
>>> Â Â Â (vector::operator[](size_type)): Use iterator operator[].
>>> Â Â Â (vector::operator[](size_type) const): Use const_iterator
>>> operator[].
>>> Â Â Â (vector::swap(vector&)): Add assertions on allocators. Use
>>> _M_swap_data.
>>> Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt,
>>> Â Â Â _InputIt)): Use _M_insert_range.
>>> Â Â Â (vector::_M_initialize(size_type)): Adapt.
>>> Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove.
>>> Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
>>> Â Â Â * python/libstdcxx/v6/printers.py
>>> (StdVectorPrinter._iterator): Stop
>>> Â Â Â using start _M_offset.
>>> Â Â Â (StdVectorPrinter.to_string): Likewise.
>>> Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt.
>>> Â Â Â *
>>> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>>> Â Â Â Add check.
>>>
>>> François
>>>
>>>
>>> On 6/24/19 9:31 PM, François Dumont wrote:
>>>> Hi
>>>>
>>>> Â Â Â Any feedback regarding this patch ?
>>>>
>>>> Thanks
>>>>
>>>> On 5/14/19 7:46 AM, François Dumont wrote:
>>>>> Hi
>>>>>
>>>>> Â Â Â This is the patch on vector<bool> to:
>>>>>
>>>>> - Optimize sizeof in Versioned namespace mode. We could go one
>>>>> step further by removing _M_p from _M_finish and just transform it
>>>>> into an offset but it is a little bit more impacting for the code.
>>>>>
>>>>> - Implement the swap optimization already done on main std::vector
>>>>> template class.
>>>>>
>>>>> - Fix move constructor so that it is noexcept no matter allocator
>>>>> move constructor noexcept qualification
>>>>>
>>>>> - Optimize move constructor with allocator when allocator type is
>>>>> always equal.
>>>>>
>>>>> - Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods.
>>>>> Those are now defined only in pre-C++11 mode, I can't see any abi
>>>>> issue in doing so.
>>>>>
>>>>> Â Â Â * include/bits/stl_bvector.h
>>>>> Â Â Â [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start):
>>>>> Define as
>>>>> Â Â Â _Bit_type*.
>>>>> Â Â Â (_Bvector_impl_data(const _Bvector_impl_data&)): Default.
>>>>> Â Â Â (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to
>>>>> latter.
>>>>> Â Â Â (_Bvector_impl_data::operator=(const _Bvector_impl_data&)):
>>>>> Default.
>>>>> (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter.
>>>>> Â Â Â (_Bvector_impl_data::_M_reset()): Likewise.
>>>>> Â Â Â (_Bvector_impl_data::_M_swap_data): New.
>>>>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement
>>>>> explicitely.
>>>>> Â Â Â (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&,
>>>>> _Bvector_impl&&)): New.
>>>>> Â Â Â (_Bvector_base::_Bvector_base(_Bvector_base&&, const
>>>>> allocator_type&)):
>>>>> Â Â Â New, use latter.
>>>>> Â Â Â (vector::vector(vector&&, const allocator_type&,
>>>>> true_type)): New, use
>>>>> Â Â Â latter.
>>>>> Â Â Â (vector::vector(vector&&, const allocator_type&,
>>>>> false_type)): New.
>>>>> Â Â Â (vector::vector(vector&&, const allocator_type&)): Use
>>>>> latters.
>>>>> Â Â Â (vector::vector(const vector&, const allocator_type&)): Adapt.
>>>>> Â Â Â [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt,
>>>>> Â Â Â const allocator_type&)): Use _M_initialize_range.
>>>>> Â Â Â (vector::operator[](size_type)): Use iterator operator[].
>>>>> Â Â Â (vector::operator[](size_type) const): Use const_iterator
>>>>> operator[].
>>>>> Â Â Â (vector::swap(vector&)): Adapt.
>>>>> Â Â Â (vector::_M_initialize(size_type)): Add assertions on
>>>>> allocators.
>>>>> Â Â Â Use _M_swap_data.
>>>>> Â Â Â [__cplusplus >= 201103](vector::insert(const_iterator,
>>>>> _InputIt,
>>>>> Â Â Â _InputIt)): Use _M_insert_range.
>>>>> Â Â Â [__cplusplus >= 201103](vector::_M_initialize_dispatch):
>>>>> Remove.
>>>>> Â Â Â [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove.
>>>>> Â Â Â * testsuite/23_containers/vector/bool/allocator/swap.cc:
>>>>> Adapt.
>>>>> Â Â Â *
>>>>> testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc:
>>>>> Â Â Â Add check.
>>>>>
>>>>> Tested under Linux x86_64, normal and debug modes.
>>>>>
>>>>> Ok to commit ?
>>
>> Yes, OK for master, but I have one question below (and one minor
>> comment).
>>
>>> diff --git a/libstdc++-v3/include/bits/stl_bvector.h
>>> b/libstdc++-v3/include/bits/stl_bvector.h
>>> index f2eea7799dc..15ecce62683 100644
>>> --- a/libstdc++-v3/include/bits/stl_bvector.h
>>> +++ b/libstdc++-v3/include/bits/stl_bvector.h
>>> @@ -449,7 +449,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>>
>>> struct _Bvector_impl_data
>>> {
>>> +#if !_GLIBCXX_INLINE_VERSION
>>> _Bit_iterator _M_start;
>>> +#else
>>> + // We don't need the offset field for the start, it's always zero.
>>> + struct {
>>> + _Bit_type* _M_p;
>>> + // Allow assignment from iterators (assume offset is zero):
>>> + void operator=(_Bit_iterator __it) { _M_p = __it._M_p; }
>>> + } _M_start;
>>> +#endif
>>> _Bit_iterator _M_finish;
>>> _Bit_pointer _M_end_of_storage;
>>>
>>> @@ -533,6 +555,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>>
>>> #if __cplusplus >= 201103L
>>> _Bvector_base(_Bvector_base&&) = default;
>>> +
>>> + _Bvector_base(_Bvector_base&& __x, const allocator_type& __a)
>>
>> I think you can add 'noexcept' here.
>>
>> Constructing the _Bit_alloc_type can't throw, and the base constructor
>> is noexcept.
>
> Ok, added.
>
>
>>
>>> + : _M_impl(_Bit_alloc_type(__a), std::move(__x._M_impl))
>>> + { }
>>> #endif
>>>
>>> ~_Bvector_base()
>>
>>
>>> diff --git
>>> a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>> b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>> index de441426532..745fdc85cf6 100644
>>> ---
>>> a/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>> +++
>>> b/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/swap.cc
>>> @@ -28,19 +28,17 @@ namespace __gnu_test
>>> // It is undefined behaviour to swap() containers with unequal
>>> allocators
>>> // if the allocator doesn't propagate, so ensure the allocators
>>> compare
>>> // equal, while still being able to test propagation via
>>> get_personality().
>>> + template<typename Type>
>>> bool
>>> - operator==(const propagating_allocator<T, false>&,
>>> - const propagating_allocator<T, false>&)
>>> - {
>>> - return true;
>>> - }
>>> + operator==(const propagating_allocator<Type, false>&,
>>> + const propagating_allocator<Type, false>&)
>>> + { return true; }
>>>
>>> + template<typename Type>
>>> bool
>>> - operator!=(const propagating_allocator<T, false>&,
>>> - const propagating_allocator<T, false>&)
>>> - {
>>> - return false;
>>> - }
>>> + operator!=(const propagating_allocator<Type, false>&,
>>> + const propagating_allocator<Type, false>&)
>>> + { return false; }
>>
>> Why does this test need to be adapted?
>>
> Because I replicate and adapt the static assertion from the main
> vector implementation which is:
>
> #if __cplusplus >= 201103L
> __glibcxx_assert(_Bit_alloc_traits::propagate_on_container_swap::value
> || _M_get_Bit_allocator() == __x._M_get_Bit_allocator());
> #endif
>
> in the case of vector<bool> the compared allocators are not
> propagating_allocator<bool, false> but propagating_allocator<unsigned
> long, false>.
>
> Building with __glibcxx_assert was showing this.
>
> François
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: std::vector<bool> code cleanup fixes optimizations
2020-08-06 19:30 ` François Dumont
@ 2020-08-06 19:39 ` Jonathan Wakely
0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2020-08-06 19:39 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 06/08/20 21:30 +0200, François Dumont via Libstdc++ wrote:
>I wonder if following the application of this patch we shouldn't bump
>versioned namespace through _GLIBCXX_BEGIN_NAMESPACE_VERSION ?
Definitely not.
>Unless it is still considered as experimental.
Yes, it is. And it doesn't claim to provide any stability or backwards
compatibility.
A small change like this doesn't justify changing it.
We changed it last time because I was hoping to change the std::string
ABI used by the versioned namespace, but that never happened.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-08-06 19:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-14 5:46 std::vector<bool> code cleanup fixes optimizations François Dumont
2019-06-24 19:31 ` François Dumont
2019-12-16 7:18 ` François Dumont
2020-07-17 10:36 ` Jonathan Wakely
2020-07-31 21:03 ` François Dumont
2020-07-31 21:47 ` Jonathan Wakely
2020-08-06 19:30 ` François Dumont
2020-08-06 19:39 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).