diff mbox series

[v25,25/33] libstdc++: Optimize std::is_function compilation performance

Message ID 20231024020424.699427-26-kmatsui@gcc.gnu.org
State Unresolved
Headers
Series Optimize type traits compilation performance |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Ken Matsui Oct. 24, 2023, 2:01 a.m. UTC 
  This patch optimizes the compilation performance of std::is_function
by dispatching to the new __is_function built-in trait.

libstdc++-v3/ChangeLog:

	* include/std/type_traits (is_function): Use __is_function
	built-in trait.
	(is_function_v): Likewise. Optimize its implementation.
	(is_const_v): Move on top of is_function_v as is_function_v now
	depends on is_const_v.

Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
---
 libstdc++-v3/include/std/type_traits | 39 ++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 11 deletions(-)

Comments

Jonathan Wakely Oct. 24, 2023, 11:01 a.m. UTC | #1 
On Tue, 24 Oct 2023 at 03:16, Ken Matsui <kmatsui@gcc.gnu.org> wrote:

> This patch optimizes the compilation performance of std::is_function
> by dispatching to the new __is_function built-in trait.
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/type_traits (is_function): Use __is_function
>         built-in trait.
>         (is_function_v): Likewise. Optimize its implementation.
>         (is_const_v): Move on top of is_function_v as is_function_v now
>         depends on is_const_v.
>

I think I'd prefer to keep is_const_v where it is now, adjacent to
is_volatile_v, and move is_function_v after those.

i.e. like this (but with the additional changes to use the new built-in):

--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -3198,8 +3198,8 @@ template <typename _Tp>
   inline constexpr bool is_union_v = __is_union(_Tp);
 template <typename _Tp>
   inline constexpr bool is_class_v = __is_class(_Tp);
-template <typename _Tp>
-  inline constexpr bool is_function_v = is_function<_Tp>::value;
+// is_function_v is defined below, after is_const_v.
+
 template <typename _Tp>
   inline constexpr bool is_reference_v = false;
 template <typename _Tp>
@@ -3226,6 +3226,8 @@ template <typename _Tp>
   inline constexpr bool is_volatile_v = false;
 template <typename _Tp>
   inline constexpr bool is_volatile_v<volatile _Tp> = true;
+template <typename _Tp>
+  inline constexpr bool is_function_v = is_function<_Tp>::value;

 template <typename _Tp>
   inline constexpr bool is_trivial_v = __is_trivial(_Tp);

The variable templates are currently defined in the order shown in the
standard, in te [meta.type.synop] synopsis, and in the [meta.unary.cat]
table. So let's move _is_function_v later and add a comment saying why it's
not in the expected place.
Ken Matsui Dec. 7, 2023, 3:20 a.m. UTC | #2 
On Tue, Oct 24, 2023 at 4:02 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
>
>
> On Tue, 24 Oct 2023 at 03:16, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
>>
>> This patch optimizes the compilation performance of std::is_function
>> by dispatching to the new __is_function built-in trait.
>>
>> libstdc++-v3/ChangeLog:
>>
>>         * include/std/type_traits (is_function): Use __is_function
>>         built-in trait.
>>         (is_function_v): Likewise. Optimize its implementation.
>>         (is_const_v): Move on top of is_function_v as is_function_v now
>>         depends on is_const_v.
>
>
> I think I'd prefer to keep is_const_v where it is now, adjacent to is_volatile_v, and move is_function_v after those.
>
> i.e. like this (but with the additional changes to use the new built-in):
>
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -3198,8 +3198,8 @@ template <typename _Tp>
>    inline constexpr bool is_union_v = __is_union(_Tp);
>  template <typename _Tp>
>    inline constexpr bool is_class_v = __is_class(_Tp);
> -template <typename _Tp>
> -  inline constexpr bool is_function_v = is_function<_Tp>::value;
> +// is_function_v is defined below, after is_const_v.
> +
>  template <typename _Tp>
>    inline constexpr bool is_reference_v = false;
>  template <typename _Tp>
> @@ -3226,6 +3226,8 @@ template <typename _Tp>
>    inline constexpr bool is_volatile_v = false;
>  template <typename _Tp>
>    inline constexpr bool is_volatile_v<volatile _Tp> = true;
> +template <typename _Tp>
> +  inline constexpr bool is_function_v = is_function<_Tp>::value;
>
>  template <typename _Tp>
>    inline constexpr bool is_trivial_v = __is_trivial(_Tp);
>
> The variable templates are currently defined in the order shown in the standard, in te [meta.type.synop] synopsis, and in the [meta.unary.cat] table. So let's move _is_function_v later and add a comment saying why it's not in the expected place.
>
>

Sorry for missing this.  Thank you for your review!  That totally
makes sense, and I will update my patches accordingly.
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index 36ad9814047..1e561c8ea38 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -637,6 +637,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { };
 
   /// is_function
+#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_function)
+  template<typename _Tp>
+    struct is_function
+    : public __bool_constant<__is_function(_Tp)>
+    { };
+#else
   template<typename _Tp>
     struct is_function
     : public __bool_constant<!is_const<const _Tp>::value> { };
@@ -648,6 +654,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Tp>
     struct is_function<_Tp&&>
     : public false_type { };
+#endif
 
 #ifdef __cpp_lib_is_null_pointer // C++ >= 11
   /// is_null_pointer (LWG 2247).
@@ -3269,8 +3276,28 @@  template <typename _Tp>
   inline constexpr bool is_union_v = __is_union(_Tp);
 template <typename _Tp>
   inline constexpr bool is_class_v = __is_class(_Tp);
+
+#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_const)
+template <typename _Tp>
+  inline constexpr bool is_const_v = __is_const(_Tp);
+#else
+template <typename _Tp>
+  inline constexpr bool is_const_v = false;
 template <typename _Tp>
-  inline constexpr bool is_function_v = is_function<_Tp>::value;
+  inline constexpr bool is_const_v<const _Tp> = true;
+#endif
+
+#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_function)
+template <typename _Tp>
+  inline constexpr bool is_function_v = __is_function(_Tp);
+#else
+template <typename _Tp>
+  inline constexpr bool is_function_v = !is_const_v<const _Tp>;
+template <typename _Tp>
+  inline constexpr bool is_function_v<_Tp&> = false;
+template <typename _Tp>
+  inline constexpr bool is_function_v<_Tp&&> = false;
+#endif
 
 #if _GLIBCXX_USE_BUILTIN_TRAIT(__is_reference)
 template <typename _Tp>
@@ -3303,16 +3330,6 @@  template <typename _Tp>
   inline constexpr bool is_member_pointer_v = is_member_pointer<_Tp>::value;
 #endif
 
-#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_const)
-template <typename _Tp>
-  inline constexpr bool is_const_v = __is_const(_Tp);
-#else
-template <typename _Tp>
-  inline constexpr bool is_const_v = false;
-template <typename _Tp>
-  inline constexpr bool is_const_v<const _Tp> = true;
-#endif
-
 #if _GLIBCXX_USE_BUILTIN_TRAIT(__is_volatile)
 template <typename _Tp>
   inline constexpr bool is_volatile_v = __is_volatile(_Tp);