Hi, this patch improves code generation on string constructors. We currently have _M_construct which takes as a parameter two iterators (begin/end pointers to other string) and produces new string. This patch adds special case of constructor where instead of begining/end pointers we readily know the string size and also special case when we know that source is 0 terminated. This happens commonly when producing stirng copies. Moreover currently ipa-prop is not able to propagate information that beg-end is known constant (copied string size) which makes it impossible for inliner to spot the common case where string size is known to be shorter than 15 bytes and fits in local buffer.
Finally I made new constructor inline. Because it is explicitely instantiated without C++20 constexpr we do not produce implicit instantiation (as required by standard) which prevents inlining, ipa-modref and any other IPA analysis to happen. I think we need to make many of the other functions inline, since optimization accross string manipulation is quite important. There is PR94960 to track this issue. Bootstrapped/regtested x86_64-linux, OK? libstdc++-v3/ChangeLog: PR tree-optimization/103827 PR tree-optimization/80331 PR tree-optimization/87502 * config/abi/pre/gnu.ver: Add version for _M_construct<bool> * include/bits/basic_string.h: (basic_string::_M_construct<bool>): Declare. (basic_string constructors): Use it. * include/bits/basic_string.tcc: (basic_string::_M_construct<bool>): New template. * src/c++11/string-inst.cc: Instantated S::_M_construct<bool>. gcc/testsuite/ChangeLog: * g++.dg/tree-ssa/pr103827.C: New test. * g++.dg/tree-ssa/pr80331.C: New test. * g++.dg/tree-ssa/pr87502.C: New test. diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr103827.C b/gcc/testsuite/g++.dg/tree-ssa/pr103827.C new file mode 100644 index 00000000000..6059fe514b1 --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/pr103827.C @@ -0,0 +1,22 @@ +// { dg-do compile } +// { dg-options "-O1 -fdump-tree-optimized" } +struct foo +{ + int a; + void bar() const; + ~foo() + { + if (a != 42) + __builtin_abort (); + } +}; +__attribute__ ((noinline)) +void test(const struct foo a) +{ + int b = a.a; + a.bar(); + if (a.a != b) + __builtin_printf ("optimize me away"); +} + +/* { dg-final { scan-tree-dump-not "optimize me away" "optimized" } } */ diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr80331.C b/gcc/testsuite/g++.dg/tree-ssa/pr80331.C new file mode 100644 index 00000000000..85034504f2f --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/pr80331.C @@ -0,0 +1,8 @@ +// { dg-do compile } +// { dg-additional-options "-O2 -fdump-tree-optimized" } +#include<string> +int sain() { + const std::string remove_me("remove_me"); + return 0; +} +// { dg-final { scan-tree-dump-not "remove_me" "optimized" } } diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr87502.C b/gcc/testsuite/g++.dg/tree-ssa/pr87502.C new file mode 100644 index 00000000000..7975432597d --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/pr87502.C @@ -0,0 +1,16 @@ +// { dg-do compile } +// { dg-additional-options "-O2 -fdump-tree-optimized" } +#include <string> + + +__attribute__ ((pure)) +extern int foo (const std::string &); + +int +bar () +{ + return foo ("abc") + foo (std::string("abc")); +} +// We used to add terminating zero explicitely instead of using fact +// that memcpy source is already 0 terminated. +// { dg-final { scan-tree-dump-not "remove_me" "= 0;" } } diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index ae79b371d80..75a6ade1373 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -2540,6 +2540,9 @@ GLIBCXX_3.4.34 { _ZNSt8__format25__locale_encoding_to_utf8ERKSt6localeSt17basic_string_viewIcSt11char_traitsIcEEPv; # __sso_string constructor and destructor _ZNSt12__sso_string[CD][12]Ev; + # void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<bool>(char const*, unsigned long) + # and wide char version + _ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE12_M_constructILb[01]EEEvPK[cw]m; } GLIBCXX_3.4.33; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h index 8369c24d3ae..effc22b8dc9 100644 --- a/libstdc++-v3/include/bits/basic_string.h +++ b/libstdc++-v3/include/bits/basic_string.h @@ -341,6 +341,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 void _M_construct(size_type __req, _CharT __c); + // Construct using block of memory of known size. + // If _Terminated is true assume that source is already 0 terminated. + template<bool _Terminated> + _GLIBCXX20_CONSTEXPR inline + void + _M_construct(const _CharT *__c, size_type __n); + _GLIBCXX20_CONSTEXPR allocator_type& _M_get_allocator() @@ -561,8 +568,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 : _M_dataplus(_M_local_data(), * include/bits/vector.tcc: _Alloc_traits::_S_select_on_copy(__str._M_get_allocator())) { - _M_construct(__str._M_data(), __str._M_data() + __str.length(), - std::forward_iterator_tag()); + _M_construct<true>(__str._M_data(), __str.length()); } // _GLIBCXX_RESOLVE_LIB_DEFECTS @@ -580,8 +586,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 { const _CharT* __start = __str._M_data() + __str._M_check(__pos, "basic_string::basic_string"); - _M_construct(__start, __start + __str._M_limit(__pos, npos), - std::forward_iterator_tag()); + _M_construct<false>(__start, __str._M_limit(__pos, npos)); } /** @@ -597,8 +602,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 { const _CharT* __start = __str._M_data() + __str._M_check(__pos, "basic_string::basic_string"); - _M_construct(__start, __start + __str._M_limit(__pos, __n), - std::forward_iterator_tag()); + _M_construct<false>(__start, __str._M_limit(__pos, __n)); } /** @@ -615,8 +619,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 { const _CharT* __start = __str._M_data() + __str._M_check(__pos, "string::string"); - _M_construct(__start, __start + __str._M_limit(__pos, __n), - std::forward_iterator_tag()); + _M_construct<false>(__start, __str._M_limit(__pos, __n)); } /** @@ -637,7 +640,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 if (__s == 0 && __n > 0) std::__throw_logic_error(__N("basic_string: " "construction from null is not valid")); - _M_construct(__s, __s + __n, std::forward_iterator_tag()); + _M_construct<false>(__s, __n); } /** @@ -658,8 +661,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 if (__s == 0) std::__throw_logic_error(__N("basic_string: " "construction from null is not valid")); - const _CharT* __end = __s + traits_type::length(__s); - _M_construct(__s, __end, forward_iterator_tag()); + _M_construct<true>(__s, traits_type::length (__s)); } /** @@ -723,7 +725,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 _GLIBCXX20_CONSTEXPR basic_string(const basic_string& __str, const _Alloc& __a) : _M_dataplus(_M_local_data(), __a) - { _M_construct(__str.begin(), __str.end(), std::forward_iterator_tag()); } + { _M_construct<true>(__str._M_data(), __str.length()); } _GLIBCXX20_CONSTEXPR basic_string(basic_string&& __str, const _Alloc& __a) @@ -748,7 +750,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 __str._M_set_length(0); } else - _M_construct(__str.begin(), __str.end(), std::forward_iterator_tag()); + _M_construct<true>(__str._M_data(), __str.length()); } #endif // C++11 diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc index caeddaf2f5b..395010aa432 100644 --- a/libstdc++-v3/include/bits/basic_string.tcc +++ b/libstdc++-v3/include/bits/basic_string.tcc @@ -275,6 +275,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _M_set_length(__n); } + // Length of string constructed is easier to propagate inter-procedurally + // than difference between iterators. + template<typename _CharT, typename _Traits, typename _Alloc> + template<bool _Terminated> + _GLIBCXX20_CONSTEXPR inline + void + basic_string<_CharT, _Traits, _Alloc>:: + _M_construct(const _CharT *__str, size_type __n) + { + if (__n > size_type(_S_local_capacity)) + { + _M_data(_M_create(__n, size_type(0))); + _M_capacity(__n); + } + else + _M_init_local_buf(); + + if (__n || _Terminated) + this->_S_copy(_M_data(), __str, __n + _Terminated); + + _M_length(__n); + if (!_Terminated) + traits_type::assign(_M_data()[__n], _CharT()); + } template<typename _CharT, typename _Traits, typename _Alloc> _GLIBCXX20_CONSTEXPR diff --git a/libstdc++-v3/src/c++11/string-inst.cc b/libstdc++-v3/src/c++11/string-inst.cc index ec5ba41ebcd..4be626806bf 100644 --- a/libstdc++-v3/src/c++11/string-inst.cc +++ b/libstdc++-v3/src/c++11/string-inst.cc @@ -91,6 +91,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION void S::_M_construct(const C*, const C*, forward_iterator_tag); + template + void + S::_M_construct<false>(const C*, size_t); + + template + void + S::_M_construct<true>(const C*, size_t); + #else // !_GLIBCXX_USE_CXX11_ABI template