libstdc++: Simplify std::_Destroy using 'if constexpr'

Message ID 20241128184859.934977-1-jwakely@redhat.com
State Committed
Commit 1467409beb27a693e96994899f30e4db015b5c2c
Headers
Series libstdc++: Simplify std::_Destroy using 'if constexpr' |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Jonathan Wakely Nov. 28, 2024, 6:48 p.m. UTC
  This is another place where we can use 'if constexpr' to replace
dispatching to a specialized class template, improving compile times and
avoiding a function call.

libstdc++-v3/ChangeLog:

	* include/bits/stl_construct.h (_Destroy(FwdIter, FwdIter)): Use
	'if constexpr' instead of dispatching to a member function of a
	class template.
	(_Destroy_n(FwdIter, Size)): Likewise.
	(_Destroy_aux, _Destroy_n_aux): Only define for C++98.
---

This seems worthwhile, as another small reduction in compile times,
similar to a number of recent patches.

Tested x86_64-linux.

 libstdc++-v3/include/bits/stl_construct.h | 33 ++++++++++++++++++-----
 1 file changed, 27 insertions(+), 6 deletions(-)
  

Comments

Patrick Palka Dec. 2, 2024, 4:37 p.m. UTC | #1
On Thu, 28 Nov 2024, Jonathan Wakely wrote:

> This is another place where we can use 'if constexpr' to replace
> dispatching to a specialized class template, improving compile times and
> avoiding a function call.
> 
> libstdc++-v3/ChangeLog:
> 
> 	* include/bits/stl_construct.h (_Destroy(FwdIter, FwdIter)): Use
> 	'if constexpr' instead of dispatching to a member function of a
> 	class template.
> 	(_Destroy_n(FwdIter, Size)): Likewise.
> 	(_Destroy_aux, _Destroy_n_aux): Only define for C++98.
> ---
> 
> This seems worthwhile, as another small reduction in compile times,
> similar to a number of recent patches.

LGTM

> 
> Tested x86_64-linux.
> 
>  libstdc++-v3/include/bits/stl_construct.h | 33 ++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/libstdc++-v3/include/bits/stl_construct.h b/libstdc++-v3/include/bits/stl_construct.h
> index 9d6111396e1..6889a9bfa0e 100644
> --- a/libstdc++-v3/include/bits/stl_construct.h
> +++ b/libstdc++-v3/include/bits/stl_construct.h
> @@ -166,6 +166,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  #endif
>      }
>  
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wc++17-extensions" // for if-constexpr
> +
> +#if __cplusplus < 201103L
>    template<bool>
>      struct _Destroy_aux
>      {
> @@ -185,6 +189,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>          static void
>          __destroy(_ForwardIterator, _ForwardIterator) { }
>      };
> +#endif
>  
>    /**
>     * Destroy a range of objects.  If the value_type of the object has
> @@ -201,15 +206,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        // A deleted destructor is trivial, this ensures we reject such types:
>        static_assert(is_destructible<_Value_type>::value,
>  		    "value type is destructible");
> -#endif
> +      if constexpr (!__has_trivial_destructor(_Value_type))
> +	for (; __first != __last; ++__first)
> +	  std::_Destroy(std::__addressof(*__first));
>  #if __cpp_constexpr_dynamic_alloc // >= C++20
> -      if (std::__is_constant_evaluated())
> -	return std::_Destroy_aux<false>::__destroy(__first, __last);
> +      else if (std::__is_constant_evaluated())
> +	for (; __first != __last; ++__first)
> +	  std::destroy_at(std::__addressof(*__first));
>  #endif
> +#else
>        std::_Destroy_aux<__has_trivial_destructor(_Value_type)>::
>  	__destroy(__first, __last);
> +#endif
>      }
>  
> +#if __cplusplus < 201103L
>    template<bool>
>      struct _Destroy_n_aux
>      {
> @@ -234,6 +245,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  	  return __first;
>  	}
>      };
> +#endif
>  
>    /**
>     * Destroy a range of objects.  If the value_type of the object has
> @@ -250,14 +262,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        // A deleted destructor is trivial, this ensures we reject such types:
>        static_assert(is_destructible<_Value_type>::value,
>  		    "value type is destructible");
> -#endif
> +      if constexpr (!__has_trivial_destructor(_Value_type))
> +	for (; __count > 0; (void)++__first, --__count)
> +	  std::_Destroy(std::__addressof(*__first));
>  #if __cpp_constexpr_dynamic_alloc // >= C++20
> -      if (std::__is_constant_evaluated())
> -	return std::_Destroy_n_aux<false>::__destroy_n(__first, __count);
> +      else if (std::__is_constant_evaluated())
> +	for (; __count > 0; (void)++__first, --__count)
> +	  std::destroy_at(std::__addressof(*__first));
>  #endif
> +      else
> +	std::advance(__first, __count);
> +      return __first;
> +#else
>        return std::_Destroy_n_aux<__has_trivial_destructor(_Value_type)>::
>  	__destroy_n(__first, __count);
> +#endif
>      }
> +#pragma GCC diagnostic pop
>  
>  #if __glibcxx_raw_memory_algorithms // >= C++17
>    template <typename _ForwardIterator>
> -- 
> 2.47.0
> 
>
  

Patch

diff --git a/libstdc++-v3/include/bits/stl_construct.h b/libstdc++-v3/include/bits/stl_construct.h
index 9d6111396e1..6889a9bfa0e 100644
--- a/libstdc++-v3/include/bits/stl_construct.h
+++ b/libstdc++-v3/include/bits/stl_construct.h
@@ -166,6 +166,10 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
     }
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions" // for if-constexpr
+
+#if __cplusplus < 201103L
   template<bool>
     struct _Destroy_aux
     {
@@ -185,6 +189,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
         static void
         __destroy(_ForwardIterator, _ForwardIterator) { }
     };
+#endif
 
   /**
    * Destroy a range of objects.  If the value_type of the object has
@@ -201,15 +206,21 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // A deleted destructor is trivial, this ensures we reject such types:
       static_assert(is_destructible<_Value_type>::value,
 		    "value type is destructible");
-#endif
+      if constexpr (!__has_trivial_destructor(_Value_type))
+	for (; __first != __last; ++__first)
+	  std::_Destroy(std::__addressof(*__first));
 #if __cpp_constexpr_dynamic_alloc // >= C++20
-      if (std::__is_constant_evaluated())
-	return std::_Destroy_aux<false>::__destroy(__first, __last);
+      else if (std::__is_constant_evaluated())
+	for (; __first != __last; ++__first)
+	  std::destroy_at(std::__addressof(*__first));
 #endif
+#else
       std::_Destroy_aux<__has_trivial_destructor(_Value_type)>::
 	__destroy(__first, __last);
+#endif
     }
 
+#if __cplusplus < 201103L
   template<bool>
     struct _Destroy_n_aux
     {
@@ -234,6 +245,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  return __first;
 	}
     };
+#endif
 
   /**
    * Destroy a range of objects.  If the value_type of the object has
@@ -250,14 +262,23 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // A deleted destructor is trivial, this ensures we reject such types:
       static_assert(is_destructible<_Value_type>::value,
 		    "value type is destructible");
-#endif
+      if constexpr (!__has_trivial_destructor(_Value_type))
+	for (; __count > 0; (void)++__first, --__count)
+	  std::_Destroy(std::__addressof(*__first));
 #if __cpp_constexpr_dynamic_alloc // >= C++20
-      if (std::__is_constant_evaluated())
-	return std::_Destroy_n_aux<false>::__destroy_n(__first, __count);
+      else if (std::__is_constant_evaluated())
+	for (; __count > 0; (void)++__first, --__count)
+	  std::destroy_at(std::__addressof(*__first));
 #endif
+      else
+	std::advance(__first, __count);
+      return __first;
+#else
       return std::_Destroy_n_aux<__has_trivial_destructor(_Value_type)>::
 	__destroy_n(__first, __count);
+#endif
     }
+#pragma GCC diagnostic pop
 
 #if __glibcxx_raw_memory_algorithms // >= C++17
   template <typename _ForwardIterator>