diff options
author | Jonathan Wakely <jwakely@redhat.com> | 2023-06-01 10:26:10 +0100 |
---|---|---|
committer | Jonathan Wakely <jwakely@redhat.com> | 2023-06-01 16:06:15 +0100 |
commit | b7b255e77a271974479c34d1db3daafc04b920bc (patch) | |
tree | 75bdbfb1ca9c13ee9f4f3e19bb0ff8af4357b271 | |
parent | 8cbaf679a3c1875c5475bd1cb0fb86fb9d03b2d4 (diff) | |
download | gcc-b7b255e77a271974479c34d1db3daafc04b920bc.zip gcc-b7b255e77a271974479c34d1db3daafc04b920bc.tar.gz gcc-b7b255e77a271974479c34d1db3daafc04b920bc.tar.bz2 |
libstdc++: Fix code size regressions in std::vector [PR110060]
My r14-1452-gfb409a15d9babc change to add optimization hints to
std::vector causes regressions because it makes std::vector::size() and
std::vector::capacity() too big to inline. That's the opposite of what
I wanted, so revert the changes to those functions.
To achieve the original aim of optimizing vec.assign(vec.size(), x) we
can add a local optimization hint to _M_fill_assign, so that it doesn't
affect all other uses of size() and capacity().
Additionally, add the same hint to the _M_assign_aux overload for
forward iterators and add that to the testcase.
It would be nice to similarly optimize:
if (vec1.size() == vec2.size()) vec1 = vec2;
but adding hints to operator=(const vector&) doesn't help. Presumably
the relationships between the two sizes and two capacities are too
complex to track effectively.
libstdc++-v3/ChangeLog:
PR libstdc++/110060
* include/bits/stl_vector.h (_Vector_base::_M_invariant):
Remove.
(vector::size, vector::capacity): Remove calls to _M_invariant.
* include/bits/vector.tcc (vector::_M_fill_assign): Add
optimization hint to reallocating path.
(vector::_M_assign_aux(FwdIter, FwdIter, forward_iterator_tag)):
Likewise.
* testsuite/23_containers/vector/capacity/invariant.cc: Moved
to...
* testsuite/23_containers/vector/modifiers/assign/no_realloc.cc:
...here. Check assign(FwdIter, FwdIter) too.
* testsuite/23_containers/vector/types/1.cc: Revert addition
of -Wno-stringop-overread option.
-rw-r--r-- | libstdc++-v3/include/bits/stl_vector.h | 23 | ||||
-rw-r--r-- | libstdc++-v3/include/bits/vector.tcc | 17 | ||||
-rw-r--r-- | libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/no_realloc.cc (renamed from libstdc++-v3/testsuite/23_containers/vector/capacity/invariant.cc) | 6 | ||||
-rw-r--r-- | libstdc++-v3/testsuite/23_containers/vector/types/1.cc | 2 |
4 files changed, 20 insertions, 28 deletions
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h index e593be4..70ced3d 100644 --- a/libstdc++-v3/include/bits/stl_vector.h +++ b/libstdc++-v3/include/bits/stl_vector.h @@ -389,23 +389,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER protected: - __attribute__((__always_inline__)) - _GLIBCXX20_CONSTEXPR void - _M_invariant() const - { -#if __OPTIMIZE__ - if (this->_M_impl._M_finish < this->_M_impl._M_start) - __builtin_unreachable(); - if (this->_M_impl._M_finish > this->_M_impl._M_end_of_storage) - __builtin_unreachable(); - - size_t __sz = this->_M_impl._M_finish - this->_M_impl._M_start; - size_t __cap = this->_M_impl._M_end_of_storage - this->_M_impl._M_start; - if (__sz > __cap) - __builtin_unreachable(); -#endif - } - _GLIBCXX20_CONSTEXPR void _M_create_storage(size_t __n) @@ -1005,10 +988,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR size_type size() const _GLIBCXX_NOEXCEPT - { - _Base::_M_invariant(); - return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); - } + { return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); } /** Returns the size() of the largest possible %vector. */ _GLIBCXX_NODISCARD _GLIBCXX20_CONSTEXPR @@ -1095,7 +1075,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER size_type capacity() const _GLIBCXX_NOEXCEPT { - _Base::_M_invariant(); return size_type(this->_M_impl._M_end_of_storage - this->_M_impl._M_start); } diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc index d6fdea2..acd11e2 100644 --- a/libstdc++-v3/include/bits/vector.tcc +++ b/libstdc++-v3/include/bits/vector.tcc @@ -270,15 +270,18 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER vector<_Tp, _Alloc>:: _M_fill_assign(size_t __n, const value_type& __val) { + const size_type __sz = size(); if (__n > capacity()) { + if (__n <= __sz) + __builtin_unreachable(); vector __tmp(__n, __val, _M_get_Tp_allocator()); __tmp._M_impl._M_swap_data(this->_M_impl); } - else if (__n > size()) + else if (__n > __sz) { std::fill(begin(), end(), __val); - const size_type __add = __n - size(); + const size_type __add = __n - __sz; _GLIBCXX_ASAN_ANNOTATE_GROW(__add); this->_M_impl._M_finish = std::__uninitialized_fill_n_a(this->_M_impl._M_finish, @@ -316,10 +319,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER _M_assign_aux(_ForwardIterator __first, _ForwardIterator __last, std::forward_iterator_tag) { + const size_type __sz = size(); const size_type __len = std::distance(__first, __last); if (__len > capacity()) { + if (__len <= __sz) + __builtin_unreachable(); + _S_check_init_len(__len, _M_get_Tp_allocator()); pointer __tmp(_M_allocate_and_copy(__len, __first, __last)); std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish, @@ -332,14 +339,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER this->_M_impl._M_finish = this->_M_impl._M_start + __len; this->_M_impl._M_end_of_storage = this->_M_impl._M_finish; } - else if (size() >= __len) + else if (__sz >= __len) _M_erase_at_end(std::copy(__first, __last, this->_M_impl._M_start)); else { _ForwardIterator __mid = __first; - std::advance(__mid, size()); + std::advance(__mid, __sz); std::copy(__first, __mid, this->_M_impl._M_start); - const size_type __attribute__((__unused__)) __n = __len - size(); + const size_type __attribute__((__unused__)) __n = __len - __sz; _GLIBCXX_ASAN_ANNOTATE_GROW(__n); this->_M_impl._M_finish = std::__uninitialized_copy_a(__mid, __last, diff --git a/libstdc++-v3/testsuite/23_containers/vector/capacity/invariant.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/no_realloc.cc index d68db69..659f18d 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/capacity/invariant.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/assign/no_realloc.cc @@ -1,5 +1,6 @@ // { dg-do compile } // { dg-options "-O3 -g0" } +// { dg-require-normal-mode "" } // { dg-final { scan-assembler-not "_Znw" } } // GCC should be able to optimize away the paths involving reallocation. @@ -14,3 +15,8 @@ void fill_val(std::vector<int>& vec, int i) { vec.assign(vec.size(), i); } + +void fill_range(std::vector<int>& vec, const int* first) +{ + vec.assign(first, first + vec.size()); +} diff --git a/libstdc++-v3/testsuite/23_containers/vector/types/1.cc b/libstdc++-v3/testsuite/23_containers/vector/types/1.cc index 9be07d9..079e5af 100644 --- a/libstdc++-v3/testsuite/23_containers/vector/types/1.cc +++ b/libstdc++-v3/testsuite/23_containers/vector/types/1.cc @@ -18,7 +18,7 @@ // <http://www.gnu.org/licenses/>. // { dg-do compile } -// { dg-options "-Wno-unused-result -Wno-stringop-overread" } +// { dg-options "-Wno-unused-result" } #include <vector> #include <testsuite_greedy_ops.h> |