* [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert
@ 2023-06-20 13:38 Jonathan Wakely
2023-06-23 16:44 ` Jan Hubicka
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2023-06-20 13:38 UTC (permalink / raw)
To: libstdc++, gcc-patches; +Cc: Jan Hubicka
I intend to push this to trunk once testing finishes.
I generated the diff with -b so the whitespace changes aren't shown,
because there was some re-indenting that makes the diff look larger than
it really is.
Honza, I don't think this is likely to make much difference for the PR
110287 testcases, but I think it simplifies the code and so is an
improvement in terms of maintenance and readability.
-- >8 --
Replace the try-block with RAII types for deallocating storage and
destroying elements.
libstdc++-v3/ChangeLog:
* include/bits/vector.tcc (_M_realloc_insert): Replace try-block
with RAII types.
---
libstdc++-v3/include/bits/vector.tcc | 142 +++++++++++++++++----------
1 file changed, 89 insertions(+), 53 deletions(-)
diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
index acd11e2dc68..cda52fbbc4a 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -458,73 +458,109 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_M_realloc_insert(iterator __position, const _Tp& __x)
#endif
{
- const size_type __len =
- _M_check_len(size_type(1), "vector::_M_realloc_insert");
+ const size_type __len = _M_check_len(1u, "vector::_M_realloc_insert");
pointer __old_start = this->_M_impl._M_start;
pointer __old_finish = this->_M_impl._M_finish;
const size_type __elems_before = __position - begin();
pointer __new_start(this->_M_allocate(__len));
pointer __new_finish(__new_start);
- __try
+
+ // RAII guard for allocated storage.
+ struct _Guard
+ {
+ pointer _M_storage; // Storage to deallocate
+ size_type _M_len;
+ _Tp_alloc_type& _M_alloc;
+
+ _GLIBCXX20_CONSTEXPR
+ _Guard(pointer __s, size_type __l, _Tp_alloc_type& __a)
+ : _M_storage(__s), _M_len(__l), _M_alloc(__a)
+ { }
+
+ _GLIBCXX20_CONSTEXPR
+ ~_Guard()
{
- // The order of the three operations is dictated by the C++11
- // case, where the moves could alter a new element belonging
- // to the existing vector. This is an issue only for callers
- // taking the element by lvalue ref (see last bullet of C++11
- // [res.on.arguments]).
- _Alloc_traits::construct(this->_M_impl,
- __new_start + __elems_before,
+ if (_M_storage)
+ __gnu_cxx::__alloc_traits<_Tp_alloc_type>::
+ deallocate(_M_alloc, _M_storage, _M_len);
+ }
+
+ private:
+ _Guard(const _Guard&);
+ };
+ _Guard __guard(__new_start, __len, _M_impl);
+
+ // The order of the three operations is dictated by the C++11
+ // case, where the moves could alter a new element belonging
+ // to the existing vector. This is an issue only for callers
+ // taking the element by lvalue ref (see last bullet of C++11
+ // [res.on.arguments]).
+
+ // If this throws, the existing elements are unchanged.
#if __cplusplus >= 201103L
- std::forward<_Args>(__args)...);
+ _Alloc_traits::construct(this->_M_impl,
+ std::__to_address(__new_start + __elems_before),
+ std::forward<_Args>(__args)...);
#else
- __x);
+ _Alloc_traits::construct(this->_M_impl,
+ __new_start + __elems_before,
+ __x);
#endif
- __new_finish = pointer();
#if __cplusplus >= 201103L
- if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
- {
- __new_finish = _S_relocate(__old_start, __position.base(),
- __new_start, _M_get_Tp_allocator());
-
- ++__new_finish;
-
- __new_finish = _S_relocate(__position.base(), __old_finish,
- __new_finish, _M_get_Tp_allocator());
- }
- else
-#endif
- {
- __new_finish
- = std::__uninitialized_move_if_noexcept_a
- (__old_start, __position.base(),
- __new_start, _M_get_Tp_allocator());
-
- ++__new_finish;
-
- __new_finish
- = std::__uninitialized_move_if_noexcept_a
- (__position.base(), __old_finish,
- __new_finish, _M_get_Tp_allocator());
- }
- }
- __catch(...)
+ if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
{
- if (!__new_finish)
- _Alloc_traits::destroy(this->_M_impl,
- __new_start + __elems_before);
- else
- std::_Destroy(__new_start, __new_finish, _M_get_Tp_allocator());
- _M_deallocate(__new_start, __len);
- __throw_exception_again;
+ // Relocation cannot throw.
+ __new_finish = _S_relocate(__old_start, __position.base(),
+ __new_start, _M_get_Tp_allocator());
+ ++__new_finish;
+ __new_finish = _S_relocate(__position.base(), __old_finish,
+ __new_finish, _M_get_Tp_allocator());
}
-#if __cplusplus >= 201103L
- if _GLIBCXX17_CONSTEXPR (!_S_use_relocate())
+ else
#endif
- std::_Destroy(__old_start, __old_finish, _M_get_Tp_allocator());
- _GLIBCXX_ASAN_ANNOTATE_REINIT;
- _M_deallocate(__old_start,
- this->_M_impl._M_end_of_storage - __old_start);
+ {
+ // RAII type to destroy initialized elements.
+ struct _Guard_elts
+ {
+ pointer _M_first, _M_last; // Elements to destroy
+ _Tp_alloc_type& _M_alloc;
+
+ _GLIBCXX20_CONSTEXPR
+ _Guard_elts(pointer __elt, _Tp_alloc_type& __a)
+ : _M_first(__elt), _M_last(__elt + 1), _M_alloc(__a)
+ { }
+
+ _GLIBCXX20_CONSTEXPR
+ ~_Guard_elts()
+ { std::_Destroy(_M_first, _M_last, _M_alloc); }
+
+ private:
+ _Guard_elts(const _Guard_elts&);
+ };
+
+ // Guard the new element so it will be destroyed if anything throws.
+ _Guard_elts __guard_elts(__new_start + __elems_before, _M_impl);
+
+ __new_finish = std::__uninitialized_move_if_noexcept_a(
+ __old_start, __position.base(),
+ __new_start, _M_get_Tp_allocator());
+
+ ++__new_finish;
+ // Guard everything before the new element too.
+ __guard_elts._M_first = __new_start;
+
+ __new_finish = std::__uninitialized_move_if_noexcept_a(
+ __position.base(), __old_finish,
+ __new_finish, _M_get_Tp_allocator());
+
+ // New storage has been fully initialized, destroy the old elements.
+ __guard_elts._M_first = __old_start;
+ __guard_elts._M_last = __old_finish;
+ }
+ __guard._M_storage = __old_start;
+ __guard._M_len = this->_M_impl._M_end_of_storage - __old_start;
+
this->_M_impl._M_start = __new_start;
this->_M_impl._M_finish = __new_finish;
this->_M_impl._M_end_of_storage = __new_start + __len;
--
2.40.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert
2023-06-20 13:38 [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert Jonathan Wakely
@ 2023-06-23 16:44 ` Jan Hubicka
2023-06-23 18:19 ` Jonathan Wakely
0 siblings, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2023-06-23 16:44 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
> I intend to push this to trunk once testing finishes.
>
> I generated the diff with -b so the whitespace changes aren't shown,
> because there was some re-indenting that makes the diff look larger than
> it really is.
>
> Honza, I don't think this is likely to make much difference for the PR
> 110287 testcases, but I think it simplifies the code and so is an
> improvement in terms of maintenance and readability.
Thanks for cleaning it up :)
The new version seems slightly smaller than the original in inliner
metrics.
I started to look if we can break out useful parts out of
_M_realloc_insert to make it smaller and fit -O3 inline limits.
ipa-fnsplit does some "useful" job, like break out:
<bb 3> [local count: 107374184]:
if (__n_9(D) > 2305843009213693951)
goto <bb 4>; [50.00%]
else
goto <bb 5>; [50.00%]
<bb 4> [local count: 53687092]:
std::__throw_bad_array_new_length ();
<bb 5> [local count: 53687092]:
std::__throw_bad_alloc ();
from std::__new_allocator<std::pair<unsigned int, unsigned int> ::allocate
into a separate function, which saves another 4 instructions in the estimate.
It is fun to notice that both checks are dead with default predictor,
but we do not know that because _M_check_len is not inlined and we do
not have return value value range propagation, which I will add. With
your propsed change to _M_check_len we will also need to solve PR110377
and actually notice the value range implied by __bulitin_unreachable
early enough.
What however also goes wrong is that after splitting we decide to inline
it back before we consider inlining _M_realloc_insert, so the savings
does not help. The reason is that the profile is estimated as:
_4 = __builtin_expect (_3, 0);
if (_4 != 0)
goto <bb 3>; [10.00%]
else
goto <bb 6>; [90.00%]
so we expect that with 10% probability the allocation will exceed 64bit
address space. The reason is that __builtin_expect is defined to have
10% missrate which we can't change, since it is used in algorithms where
the probability of unlikely value really is non-zero.
There is __builtin_expect_with_probability that makes it to possible to
set probability to 0 or 100 that may be better in such situation,
however here it is useless. If code path leads to noreturn function,
we predict it as noreturn. This heuristics has lower precedence than
builtin_expect so it is not applied, but would do the same work.
To work out that the code path is really very unlikely and should be
offloaded to a cold section, we however need:
diff --git a/libstdc++-v3/include/bits/functexcept.h b/libstdc++-v3/include/bits/functexcept.h
index 89972baf2c9..2765f7865df 100644
--- a/libstdc++-v3/include/bits/functexcept.h
+++ b/libstdc++-v3/include/bits/functexcept.h
@@ -46,14 +46,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#if _GLIBCXX_HOSTED
// Helper for exception objects in <except>
void
- __throw_bad_exception(void) __attribute__((__noreturn__));
+ __throw_bad_exception(void) __attribute__((__noreturn__,__cold__));
// Helper for exception objects in <new>
void
- __throw_bad_alloc(void) __attribute__((__noreturn__));
+ __throw_bad_alloc(void) __attribute__((__noreturn__,__cold__));
void
- __throw_bad_array_new_length(void) __attribute__((__noreturn__));
+ __throw_bad_array_new_length(void) __attribute__((__noreturn__,__cold__));
// Helper for exception objects in <typeinfo>
void
This makes us to drop cont to profile_count::zero which indicates that
the code is very likely not executed at all during run of the program.
The reason why we can't take such a strong hint from unreachable
attribute is twofold. First most programs do call "exit (0)" so taking
this as a strong hint may make us to optimize whole program for size.
Second is that we consider a possibility that insane developers may make
EH delivery relatively common.
Would be possible to annotate throw functions in libstdc++ which are
very unlikely taken by a working program as __cold__ and possibly drop
the redundant __builtin_expect?
I will reorder predictors so __builtin_cold_noreturn and
__builtin_expect_with_probability thakes precedence over
__builtin_expect.
It is fun to see how many things can go wrong in such a simple use of
libstdc++ :)
Honza
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert
2023-06-23 16:44 ` Jan Hubicka
@ 2023-06-23 18:19 ` Jonathan Wakely
2023-06-28 7:56 ` Jan Hubicka
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2023-06-23 18:19 UTC (permalink / raw)
To: Jan Hubicka; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3762 bytes --]
On Fri, 23 Jun 2023 at 17:44, Jan Hubicka wrote:
> > I intend to push this to trunk once testing finishes.
> >
> > I generated the diff with -b so the whitespace changes aren't shown,
> > because there was some re-indenting that makes the diff look larger than
> > it really is.
> >
> > Honza, I don't think this is likely to make much difference for the PR
> > 110287 testcases, but I think it simplifies the code and so is an
> > improvement in terms of maintenance and readability.
>
> Thanks for cleaning it up :)
> The new version seems slightly smaller than the original in inliner
> metrics.
>
Oh good. It's pushed to trunk now.
[snip]
> To work out that the code path is really very unlikely and should be
> offloaded to a cold section, we however need:
>
> diff --git a/libstdc++-v3/include/bits/functexcept.h
> b/libstdc++-v3/include/bits/functexcept.h
> index 89972baf2c9..2765f7865df 100644
> --- a/libstdc++-v3/include/bits/functexcept.h
> +++ b/libstdc++-v3/include/bits/functexcept.h
> @@ -46,14 +46,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> #if _GLIBCXX_HOSTED
> // Helper for exception objects in <except>
> void
> - __throw_bad_exception(void) __attribute__((__noreturn__));
> + __throw_bad_exception(void) __attribute__((__noreturn__,__cold__));
>
> // Helper for exception objects in <new>
> void
> - __throw_bad_alloc(void) __attribute__((__noreturn__));
> + __throw_bad_alloc(void) __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_bad_array_new_length(void) __attribute__((__noreturn__));
> + __throw_bad_array_new_length(void)
> __attribute__((__noreturn__,__cold__));
>
> // Helper for exception objects in <typeinfo>
> void
>
> This makes us to drop cont to profile_count::zero which indicates that
> the code is very likely not executed at all during run of the program.
>
> The reason why we can't take such a strong hint from unreachable
> attribute is twofold. First most programs do call "exit (0)" so taking
> this as a strong hint may make us to optimize whole program for size.
> Second is that we consider a possibility that insane developers may make
> EH delivery relatively common.
>
> Would be possible to annotate throw functions in libstdc++ which are
> very unlikely taken by a working program as __cold__ and possibly drop
> the redundant __builtin_expect?
>
Yes, I incorrectly thought they were already considered cold, but your
explanation of why noreturn is not as strong a hint as noreturn+cold makes
sense.
I think the __throw_bad_alloc() and __throw_bad_array_new_length()
functions should always be rare, so marking them cold seems fine (users who
define their own allocators that want to throw bad_alloc "often" will
probably throw it directly, they shouldn't be using our __throw_bad_alloc()
function anyway). I don't think __throw_bad_exception is ever used, so that
doesn't matter (we could remove it from the header and just keep its
definition in the library, but there's no big advantage to doing that).
Others like __throw_length_error() should also be very very rare, and could
be marked cold.
Maybe we should just mark everything in <bits/functexcept.h> as cold. If
users want to avoid the cost of calls to those functions they can do so by
checking function preconditions/arguments to avoid the exceptions. There
are very few places where a throwing libstdc++ API doesn't have a way to
avoid the exception. The only one that isn't easily avoidable is
__throw_bad_alloc but OOM should be rare.
> I will reorder predictors so __builtin_cold_noreturn and
> __builtin_expect_with_probability thakes precedence over
> __builtin_expect.
>
> It is fun to see how many things can go wrong in such a simple use of
> libstdc++ :)
>
Yes, it's very interesting!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert
2023-06-23 18:19 ` Jonathan Wakely
@ 2023-06-28 7:56 ` Jan Hubicka
2023-06-28 8:21 ` Jonathan Wakely
2023-07-10 12:53 ` Jonathan Wakely
0 siblings, 2 replies; 6+ messages in thread
From: Jan Hubicka @ 2023-06-28 7:56 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
> I think the __throw_bad_alloc() and __throw_bad_array_new_length()
> functions should always be rare, so marking them cold seems fine (users who
> define their own allocators that want to throw bad_alloc "often" will
> probably throw it directly, they shouldn't be using our __throw_bad_alloc()
> function anyway). I don't think __throw_bad_exception is ever used, so that
> doesn't matter (we could remove it from the header and just keep its
> definition in the library, but there's no big advantage to doing that).
> Others like __throw_length_error() should also be very very rare, and could
> be marked cold.
>
> Maybe we should just mark everything in <bits/functexcept.h> as cold. If
> users want to avoid the cost of calls to those functions they can do so by
> checking function preconditions/arguments to avoid the exceptions. There
> are very few places where a throwing libstdc++ API doesn't have a way to
> avoid the exception. The only one that isn't easily avoidable is
> __throw_bad_alloc but OOM should be rare.
Hi,
this marks everything in functexcept.h as cold and I also noticed that
we probably want to mark as such terminate.
With fix to 110436 and -O3 we now inline _M_realloc_insert, yay :)
tested on x86_64-linux, OK?
* include/bits/c++config (std::__terminate): Mark cold.
* include/bits/functexcept.h: Mark everything as cold.
* libsupc++/exception: Mark terminate and unexpected as cold.
diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 009a017b048..dd47f274d5f 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -320,7 +320,7 @@ namespace std
extern "C++" __attribute__ ((__noreturn__, __always_inline__))
inline void __terminate() _GLIBCXX_USE_NOEXCEPT
{
- void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__noreturn__));
+ void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__noreturn__,__cold__));
terminate();
}
#pragma GCC visibility pop
diff --git a/libstdc++-v3/include/bits/functexcept.h b/libstdc++-v3/include/bits/functexcept.h
index 2765f7865df..a2e97413661 100644
--- a/libstdc++-v3/include/bits/functexcept.h
+++ b/libstdc++-v3/include/bits/functexcept.h
@@ -57,61 +57,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Helper for exception objects in <typeinfo>
void
- __throw_bad_cast(void) __attribute__((__noreturn__));
+ __throw_bad_cast(void) __attribute__((__noreturn__,__cold__));
void
- __throw_bad_typeid(void) __attribute__((__noreturn__));
+ __throw_bad_typeid(void) __attribute__((__noreturn__,__cold__));
// Helpers for exception objects in <stdexcept>
void
- __throw_logic_error(const char*) __attribute__((__noreturn__));
+ __throw_logic_error(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_domain_error(const char*) __attribute__((__noreturn__));
+ __throw_domain_error(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_invalid_argument(const char*) __attribute__((__noreturn__));
+ __throw_invalid_argument(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_length_error(const char*) __attribute__((__noreturn__));
+ __throw_length_error(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_out_of_range(const char*) __attribute__((__noreturn__));
+ __throw_out_of_range(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_out_of_range_fmt(const char*, ...) __attribute__((__noreturn__))
+ __throw_out_of_range_fmt(const char*, ...) __attribute__((__noreturn__,__cold__))
__attribute__((__format__(__gnu_printf__, 1, 2)));
void
- __throw_runtime_error(const char*) __attribute__((__noreturn__));
+ __throw_runtime_error(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_range_error(const char*) __attribute__((__noreturn__));
+ __throw_range_error(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_overflow_error(const char*) __attribute__((__noreturn__));
+ __throw_overflow_error(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_underflow_error(const char*) __attribute__((__noreturn__));
+ __throw_underflow_error(const char*) __attribute__((__noreturn__,__cold__));
// Helpers for exception objects in <ios>
void
- __throw_ios_failure(const char*) __attribute__((__noreturn__));
+ __throw_ios_failure(const char*) __attribute__((__noreturn__,__cold__));
void
- __throw_ios_failure(const char*, int) __attribute__((__noreturn__));
+ __throw_ios_failure(const char*, int) __attribute__((__noreturn__,__cold__));
// Helpers for exception objects in <system_error>
void
- __throw_system_error(int) __attribute__((__noreturn__));
+ __throw_system_error(int) __attribute__((__noreturn__,__cold__));
// Helpers for exception objects in <future>
void
- __throw_future_error(int) __attribute__((__noreturn__));
+ __throw_future_error(int) __attribute__((__noreturn__,__cold__));
// Helpers for exception objects in <functional>
void
- __throw_bad_function_call() __attribute__((__noreturn__));
+ __throw_bad_function_call() __attribute__((__noreturn__,__cold__));
#else // ! HOSTED
diff --git a/libstdc++-v3/libsupc++/exception b/libstdc++-v3/libsupc++/exception
index a34386e6026..00a6347ebe5 100644
--- a/libstdc++-v3/libsupc++/exception
+++ b/libstdc++-v3/libsupc++/exception
@@ -75,7 +75,7 @@ namespace std _GLIBCXX_VISIBILITY(default)
/** The runtime will call this function if %exception handling must be
* abandoned for any reason. It can also be called by the user. */
- void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__noreturn__));
+ void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__noreturn__,__cold__));
#if __cplusplus < 201703L || (__cplusplus <= 202002L && _GLIBCXX_USE_DEPRECATED)
/// If you write a replacement %unexpected handler, it must be of this type.
@@ -104,7 +104,7 @@ namespace std _GLIBCXX_VISIBILITY(default)
* @deprecated Removed from the C++ standard in C++17
*/
_GLIBCXX11_DEPRECATED
- void unexpected() __attribute__ ((__noreturn__));
+ void unexpected() __attribute__ ((__noreturn__,__cold__));
#endif
/** [18.6.4]/1: 'Returns true after completing evaluation of a
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert
2023-06-28 7:56 ` Jan Hubicka
@ 2023-06-28 8:21 ` Jonathan Wakely
2023-07-10 12:53 ` Jonathan Wakely
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2023-06-28 8:21 UTC (permalink / raw)
To: Jan Hubicka; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 6707 bytes --]
On Wed, 28 Jun 2023 at 08:56, Jan Hubicka wrote:
> > I think the __throw_bad_alloc() and __throw_bad_array_new_length()
> > functions should always be rare, so marking them cold seems fine (users
> who
> > define their own allocators that want to throw bad_alloc "often" will
> > probably throw it directly, they shouldn't be using our
> __throw_bad_alloc()
> > function anyway). I don't think __throw_bad_exception is ever used, so
> that
> > doesn't matter (we could remove it from the header and just keep its
> > definition in the library, but there's no big advantage to doing that).
> > Others like __throw_length_error() should also be very very rare, and
> could
> > be marked cold.
> >
> > Maybe we should just mark everything in <bits/functexcept.h> as cold. If
> > users want to avoid the cost of calls to those functions they can do so
> by
> > checking function preconditions/arguments to avoid the exceptions. There
> > are very few places where a throwing libstdc++ API doesn't have a way to
> > avoid the exception. The only one that isn't easily avoidable is
> > __throw_bad_alloc but OOM should be rare.
>
> Hi,
> this marks everything in functexcept.h as cold and I also noticed that
> we probably want to mark as such terminate.
>
> With fix to 110436 and -O3 we now inline _M_realloc_insert, yay :)
>
> tested on x86_64-linux, OK?
>
OK for trunk, thanks.
>
> * include/bits/c++config (std::__terminate): Mark cold.
> * include/bits/functexcept.h: Mark everything as cold.
> * libsupc++/exception: Mark terminate and unexpected as cold.
> diff --git a/libstdc++-v3/include/bits/c++config
> b/libstdc++-v3/include/bits/c++config
> index 009a017b048..dd47f274d5f 100644
> --- a/libstdc++-v3/include/bits/c++config
> +++ b/libstdc++-v3/include/bits/c++config
> @@ -320,7 +320,7 @@ namespace std
> extern "C++" __attribute__ ((__noreturn__, __always_inline__))
> inline void __terminate() _GLIBCXX_USE_NOEXCEPT
> {
> - void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__noreturn__));
> + void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__
> ((__noreturn__,__cold__));
> terminate();
> }
> #pragma GCC visibility pop
> diff --git a/libstdc++-v3/include/bits/functexcept.h
> b/libstdc++-v3/include/bits/functexcept.h
> index 2765f7865df..a2e97413661 100644
> --- a/libstdc++-v3/include/bits/functexcept.h
> +++ b/libstdc++-v3/include/bits/functexcept.h
> @@ -57,61 +57,61 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
> // Helper for exception objects in <typeinfo>
> void
> - __throw_bad_cast(void) __attribute__((__noreturn__));
> + __throw_bad_cast(void) __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_bad_typeid(void) __attribute__((__noreturn__));
> + __throw_bad_typeid(void) __attribute__((__noreturn__,__cold__));
>
> // Helpers for exception objects in <stdexcept>
> void
> - __throw_logic_error(const char*) __attribute__((__noreturn__));
> + __throw_logic_error(const char*) __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_domain_error(const char*) __attribute__((__noreturn__));
> + __throw_domain_error(const char*)
> __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_invalid_argument(const char*) __attribute__((__noreturn__));
> + __throw_invalid_argument(const char*)
> __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_length_error(const char*) __attribute__((__noreturn__));
> + __throw_length_error(const char*)
> __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_out_of_range(const char*) __attribute__((__noreturn__));
> + __throw_out_of_range(const char*)
> __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_out_of_range_fmt(const char*, ...) __attribute__((__noreturn__))
> + __throw_out_of_range_fmt(const char*, ...)
> __attribute__((__noreturn__,__cold__))
> __attribute__((__format__(__gnu_printf__, 1, 2)));
>
> void
> - __throw_runtime_error(const char*) __attribute__((__noreturn__));
> + __throw_runtime_error(const char*)
> __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_range_error(const char*) __attribute__((__noreturn__));
> + __throw_range_error(const char*) __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_overflow_error(const char*) __attribute__((__noreturn__));
> + __throw_overflow_error(const char*)
> __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_underflow_error(const char*) __attribute__((__noreturn__));
> + __throw_underflow_error(const char*)
> __attribute__((__noreturn__,__cold__));
>
> // Helpers for exception objects in <ios>
> void
> - __throw_ios_failure(const char*) __attribute__((__noreturn__));
> + __throw_ios_failure(const char*) __attribute__((__noreturn__,__cold__));
>
> void
> - __throw_ios_failure(const char*, int) __attribute__((__noreturn__));
> + __throw_ios_failure(const char*, int)
> __attribute__((__noreturn__,__cold__));
>
> // Helpers for exception objects in <system_error>
> void
> - __throw_system_error(int) __attribute__((__noreturn__));
> + __throw_system_error(int) __attribute__((__noreturn__,__cold__));
>
> // Helpers for exception objects in <future>
> void
> - __throw_future_error(int) __attribute__((__noreturn__));
> + __throw_future_error(int) __attribute__((__noreturn__,__cold__));
>
> // Helpers for exception objects in <functional>
> void
> - __throw_bad_function_call() __attribute__((__noreturn__));
> + __throw_bad_function_call() __attribute__((__noreturn__,__cold__));
>
> #else // ! HOSTED
>
> diff --git a/libstdc++-v3/libsupc++/exception
> b/libstdc++-v3/libsupc++/exception
> index a34386e6026..00a6347ebe5 100644
> --- a/libstdc++-v3/libsupc++/exception
> +++ b/libstdc++-v3/libsupc++/exception
> @@ -75,7 +75,7 @@ namespace std _GLIBCXX_VISIBILITY(default)
>
> /** The runtime will call this function if %exception handling must be
> * abandoned for any reason. It can also be called by the user. */
> - void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__noreturn__));
> + void terminate() _GLIBCXX_USE_NOEXCEPT __attribute__
> ((__noreturn__,__cold__));
>
> #if __cplusplus < 201703L || (__cplusplus <= 202002L &&
> _GLIBCXX_USE_DEPRECATED)
> /// If you write a replacement %unexpected handler, it must be of this
> type.
> @@ -104,7 +104,7 @@ namespace std _GLIBCXX_VISIBILITY(default)
> * @deprecated Removed from the C++ standard in C++17
> */
> _GLIBCXX11_DEPRECATED
> - void unexpected() __attribute__ ((__noreturn__));
> + void unexpected() __attribute__ ((__noreturn__,__cold__));
> #endif
>
> /** [18.6.4]/1: 'Returns true after completing evaluation of a
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert
2023-06-28 7:56 ` Jan Hubicka
2023-06-28 8:21 ` Jonathan Wakely
@ 2023-07-10 12:53 ` Jonathan Wakely
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2023-07-10 12:53 UTC (permalink / raw)
To: Jan Hubicka; +Cc: libstdc++, gcc-patches
On Wed, 28 Jun 2023 at 08:56, Jan Hubicka <hubicka@ucw.cz> wrote:
>
> > I think the __throw_bad_alloc() and __throw_bad_array_new_length()
> > functions should always be rare, so marking them cold seems fine (users who
> > define their own allocators that want to throw bad_alloc "often" will
> > probably throw it directly, they shouldn't be using our __throw_bad_alloc()
> > function anyway). I don't think __throw_bad_exception is ever used, so that
> > doesn't matter (we could remove it from the header and just keep its
> > definition in the library, but there's no big advantage to doing that).
> > Others like __throw_length_error() should also be very very rare, and could
> > be marked cold.
> >
> > Maybe we should just mark everything in <bits/functexcept.h> as cold. If
> > users want to avoid the cost of calls to those functions they can do so by
> > checking function preconditions/arguments to avoid the exceptions. There
> > are very few places where a throwing libstdc++ API doesn't have a way to
> > avoid the exception. The only one that isn't easily avoidable is
> > __throw_bad_alloc but OOM should be rare.
>
> Hi,
> this marks everything in functexcept.h as cold and I also noticed that
> we probably want to mark as such terminate.
Should we do the same for __glibcxx_assert_fail, declared in
libstdc++-v3/include/bits/c++config?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-07-10 12:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-20 13:38 [PATCH] libstdc++: Use RAII in std::vector::_M_realloc_insert Jonathan Wakely
2023-06-23 16:44 ` Jan Hubicka
2023-06-23 18:19 ` Jonathan Wakely
2023-06-28 7:56 ` Jan Hubicka
2023-06-28 8:21 ` Jonathan Wakely
2023-07-10 12:53 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).