Skip to content

Commit e1f59ad

Browse files
authored
Mark some std::string functions noinline. (#72869)
The intent of these particular functions, since their introduction, was to NOT be inlinable. However, the mechanism by which this was accomplished was non-obvious, and stopped working when string is compiled for C++20. A longstanding behavior specified by the C++ standard is that instantiation of the body of a template function is suppressed by an extern template declaration -- unless the function is explicitly marked either constexpr or inline. Of course, if the body is not instantiated, then it cannot possibly be inlined, and thus all the functions listed in libcxx/include/__string/extern_template_lists.h were uninlineable. But, in C++20 mode, string functions were annotated constexpr, which means they _are_ instantiated, and do become inlineable. And, in fact, they do get inlined, which has caused noticeable binary-size growth for users. For example, in C++17, `std::string f(std::string *in) { return *in; }` does not inline the copy-constructor call, and instead generates a call to the exported function defined in the libc++ shared library. I think we probably don't want to mark all functions that are currently in the extern template list as noinline, as many of them really are reasonable inlining candidates. Thus, I've restricted this change to only the few functions that were clearly intended to be outlined. See commits like b019c5c (and some others like it) for background, in which functions were removed from the extern template list in the unstable ABI in order to allow the short-string case to be inlined, while moving the long-string case to a separate function, added to the extern template list.
1 parent 38e4358 commit e1f59ad

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

libcxx/include/__config

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,6 +1182,12 @@ __sanitizer_verify_double_ended_contiguous_container(const void*, const void*, c
11821182
# define _LIBCPP_CONSTINIT
11831183
# endif
11841184

1185+
# if __has_attribute(__noinline__)
1186+
# define _LIBCPP_NOINLINE __attribute__((__noinline__))
1187+
# else
1188+
# define _LIBCPP_NOINLINE
1189+
# endif
1190+
11851191
// We often repeat things just for handling wide characters in the library.
11861192
// When wide characters are disabled, it can be useful to have a quick way of
11871193
// disabling it without having to resort to #if-#endif, which has a larger

libcxx/include/string

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1901,7 +1901,7 @@ private:
19011901
// to call the __init() functions as those are marked as inline which may
19021902
// result in over-aggressive inlining by the compiler, where our aim is
19031903
// to only inline the fast path code directly in the ctor.
1904-
_LIBCPP_CONSTEXPR_SINCE_CXX20 void __init_copy_ctor_external(const value_type* __s, size_type __sz);
1904+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE void __init_copy_ctor_external(const value_type* __s, size_type __sz);
19051905

19061906
template <class _InputIterator, __enable_if_t<__has_exactly_input_iterator_category<_InputIterator>::value, int> = 0>
19071907
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 void __init(_InputIterator __first, _InputIterator __last);
@@ -1935,15 +1935,15 @@ private:
19351935
// have proof that the input does not alias the current instance.
19361936
// For example, operator=(basic_string) performs a 'self' check.
19371937
template <bool __is_short>
1938-
_LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& __assign_no_alias(const value_type* __s, size_type __n);
1938+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string& __assign_no_alias(const value_type* __s, size_type __n);
19391939

19401940
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __erase_to_end(size_type __pos) {
19411941
__null_terminate_at(std::__to_address(__get_pointer()), __pos);
19421942
}
19431943

19441944
// __erase_external_with_move is invoked for erase() invocations where
19451945
// `n ~= npos`, likely requiring memory moves on the string data.
1946-
_LIBCPP_CONSTEXPR_SINCE_CXX20 void __erase_external_with_move(size_type __pos, size_type __n);
1946+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE void __erase_external_with_move(size_type __pos, size_type __n);
19471947

19481948
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
19491949
void __copy_assign_alloc(const basic_string& __str)
@@ -2015,8 +2015,8 @@ private:
20152015
_NOEXCEPT
20162016
{}
20172017

2018-
_LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& __assign_external(const value_type* __s);
2019-
_LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& __assign_external(const value_type* __s, size_type __n);
2018+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string& __assign_external(const value_type* __s);
2019+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE basic_string& __assign_external(const value_type* __s, size_type __n);
20202020

20212021
// Assigns the value in __s, guaranteed to be __n < __min_cap in length.
20222022
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 basic_string& __assign_short(const value_type* __s, size_type __n) {
@@ -2169,7 +2169,7 @@ basic_string<_CharT, _Traits, _Allocator>::__init(const value_type* __s, size_ty
21692169
}
21702170

21712171
template <class _CharT, class _Traits, class _Allocator>
2172-
_LIBCPP_CONSTEXPR_SINCE_CXX20
2172+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE
21732173
void basic_string<_CharT, _Traits, _Allocator>::__init_copy_ctor_external(
21742174
const value_type* __s, size_type __sz) {
21752175
if (__libcpp_is_constant_evaluated())
@@ -2398,7 +2398,7 @@ basic_string<_CharT, _Traits, _Allocator>::__grow_by_without_replace(
23982398

23992399
template <class _CharT, class _Traits, class _Allocator>
24002400
template <bool __is_short>
2401-
_LIBCPP_CONSTEXPR_SINCE_CXX20
2401+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE
24022402
basic_string<_CharT, _Traits, _Allocator>&
24032403
basic_string<_CharT, _Traits, _Allocator>::__assign_no_alias(
24042404
const value_type* __s, size_type __n) {
@@ -2416,7 +2416,7 @@ basic_string<_CharT, _Traits, _Allocator>::__assign_no_alias(
24162416
}
24172417

24182418
template <class _CharT, class _Traits, class _Allocator>
2419-
_LIBCPP_CONSTEXPR_SINCE_CXX20
2419+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE
24202420
basic_string<_CharT, _Traits, _Allocator>&
24212421
basic_string<_CharT, _Traits, _Allocator>::__assign_external(
24222422
const value_type* __s, size_type __n) {
@@ -2627,7 +2627,7 @@ basic_string<_CharT, _Traits, _Allocator>::assign(const _Tp& __t, size_type __po
26272627
}
26282628

26292629
template <class _CharT, class _Traits, class _Allocator>
2630-
_LIBCPP_CONSTEXPR_SINCE_CXX20
2630+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE
26312631
basic_string<_CharT, _Traits, _Allocator>&
26322632
basic_string<_CharT, _Traits, _Allocator>::__assign_external(const value_type* __s) {
26332633
return __assign_external(__s, traits_type::length(__s));
@@ -3112,7 +3112,7 @@ basic_string<_CharT, _Traits, _Allocator>::replace(size_type __pos, size_type __
31123112
// 'externally instantiated' erase() implementation, called when __n != npos.
31133113
// Does not check __pos against size()
31143114
template <class _CharT, class _Traits, class _Allocator>
3115-
_LIBCPP_CONSTEXPR_SINCE_CXX20
3115+
_LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE
31163116
void
31173117
basic_string<_CharT, _Traits, _Allocator>::__erase_external_with_move(
31183118
size_type __pos, size_type __n)

0 commit comments

Comments
 (0)