This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: optimize std::vector move assignment


On Wed, 25 Jul 2018, Jonathan Wakely wrote:

_M_copy_data is not really needed, we could add a defaulted assignment operator, or remove the move constructor (and call a new _M_clear() from the 2 users), etc. However, it seemed the least intrusive, the least likely to have weird consequences.

Yes, the alternative would be:

--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -100,14 +100,17 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
      : _M_start(__x._M_start), _M_finish(__x._M_finish),
        _M_end_of_storage(__x._M_end_of_storage)
      { __x._M_start = __x._M_finish = __x._M_end_of_storage = pointer(); }
+
+       _Vector_impl_data(const _Vector_impl_data&) = default;
+       _Vector_impl_data& oeprator=(const _Vector_impl_data&) = default;
#endif

      void
      _M_swap_data(_Vector_impl_data& __x) _GLIBCXX_NOEXCEPT
      {
-         std::swap(_M_start, __x._M_start);
-         std::swap(_M_finish, __x._M_finish);
-         std::swap(_M_end_of_storage, __x._M_end_of_storage);
+         _Vector_impl_data __tmp = *this;
+         *this = __x;
+         __x = __tmp;

Or just std::swap(*this, __x).

      }
     };

Your _M_copy_data seems fine. It avoids unintentional copies, because
the copy constructor and copy assignment operator remain deleted (at
least in C++11).

I didn't add a testcase because I don't see any dg-final scan-tree-dump in the libstdc++ testsuite. The closest would be g++.dg/pr83239.C, g++.dg/vect/pr64410.cc, g++.dg/vect/pr33426-ivdep-4.cc that include <vector>, but from previous experience (already with vector), adding libstdc++ testcases to the C++ testsuite is not very popular.

Yes, C++ tests using std::vector are sometimes a bit fragile.

I don't see any reason we can't use scan-tree-dump in the libstdc++
testsuite, if you wanted to add one. We do have other dg-final tests.

The others only test for the presence of some name in assembly. But I may try it later.

--
Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]