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] |
The move constructor for the SSO string uses assign(const basic_string&) when either: (1) the source string is "local" and so the contents of the small string buffer need to be copied, or (2) the allocator does not propagate and is_always_equal is false. Case (1) is suboptimal, because the assign member is not noexcept and the compiler isn't smart enough to see it won't actually throw in this case. This causes extra code in the move assignment operator so that any exception will be turned into a call to std::terminate. This can be fixed by copying small strings inline instead of calling assign. Case (2) is a bug, because the specific instances of the allocators could be equal even if is_always_equal is false. This can result in an unnecessary deep copy (and potentially-throwing allocation) when the storage should be moved. This can be fixed by simply checking if the allocators are equal. PR libstdc++/87749 * include/bits/basic_string.h [_GLIBCXX_USE_CXX11_ABI] (basic_string::operator=(basic_string&&)): For short strings copy the buffer inline. Only fall back to using assign(const basic_string&) to do a deep copy when reallocation is needed. * testsuite/21_strings/basic_string/modifiers/assign/char/87749.cc: New test. * testsuite/21_strings/basic_string/modifiers/assign/char/ move_assign_optim.cc: New test. * testsuite/21_strings/basic_string/modifiers/assign/wchar_t/87749.cc: New test. * testsuite/21_strings/basic_string/modifiers/assign/wchar_t/ move_assign_optim.cc: New test. Tested powerpc64le-linux, committed to trunk. I've also attached before.s and after.s showing the generated code for this function on x86_64 with -std=gnu++17 -O3: void test(std::string& target, std::string&& source) { target = std::move(source); } That improvement comes from avoiding using assign(const basic_string&) for copying small strings. I plan to backport this to all branches (including gcc-6 if the RMs approve it).
Attachment:
patch.txt
Description: Text document
Attachment:
before.s
Description: Text document
Attachment:
after.s
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |