diff mbox series

[v4,4/4] libstdc++: Optimize is_compound trait performance

Message ID 20230718074027.32270-4-kmatsui@gcc.gnu.org
State Accepted
Headers
Series [v4,1/4] c++, libstdc++: Implement __is_arithmetic built-in trait |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Ken Matsui July 18, 2023, 7:40 a.m. UTC 
  This patch optimizes the performance of the is_compound trait by
dispatching to the new __is_arithmetic built-in trait.

libstdc++-v3/ChangeLog:

	* include/std/type_traits (is_compound): Use __is_arithmetic
	built-in trait.
	(is_compound_v): Use is_fundamental_v instead.

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

Comments

Jonathan Wakely Aug. 8, 2023, 8:33 p.m. UTC | #1 
On Tue, 18 Jul 2023 at 08:44, Ken Matsui via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> This patch optimizes the performance of the is_compound trait by
> dispatching to the new __is_arithmetic built-in trait.
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/type_traits (is_compound): Use __is_arithmetic
>         built-in trait.
>         (is_compound_v): Use is_fundamental_v instead.
>
> Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
> ---
>  libstdc++-v3/include/std/type_traits | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/std/type_traits
> b/libstdc++-v3/include/std/type_traits
> index cf24de2fcac..73d9a2b16fc 100644
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -702,9 +702,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      { };
>
>    /// is_compound
> +#if __has_builtin(__is_arithmetic)
> +  template<typename _Tp>
> +    struct is_compound
> +    : public __bool_constant<!(__is_arithmetic(_Tp)
> +                             || is_void<_Tp>::value
> +                             || is_null_pointer<_Tp>::value)>
> +    { };
> +#else
>    template<typename _Tp>
>      struct is_compound
>      : public __not_<is_fundamental<_Tp>>::type { };
> +#endif
>

I think it would be simpler to just do this unconditionally (i.e. just a
single definition without using __has_builtin):

template<typename _Tp>
  struct is_compound
  : __bool_constant<!is_fundamental<_Tp>::value>
  { };

This still avoids instantiating __not_. If is_fundamental is much more
efficient now, then I think it's OK to instantiate it here. Otherwise we're
duplicating the logic for is_fundamental, and just giving ourselves more
code to maintain.

Nobody ever uses is_compound anyway!




>    /// @cond undocumented
>    template<typename _Tp>
> @@ -3234,7 +3243,7 @@ template <typename _Tp>
>  template <typename _Tp>
>    inline constexpr bool is_scalar_v = is_scalar<_Tp>::value;
>  template <typename _Tp>
> -  inline constexpr bool is_compound_v = is_compound<_Tp>::value;
> +  inline constexpr bool is_compound_v = !is_fundamental_v<_Tp>;
>  template <typename _Tp>
>    inline constexpr bool is_member_pointer_v =
> is_member_pointer<_Tp>::value;
>  template <typename _Tp>
> --
> 2.41.0
>
>
Ken Matsui Sept. 1, 2023, 10:40 a.m. UTC | #2 
On Tue, Aug 8, 2023 at 1:33 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>
>
>
> On Tue, 18 Jul 2023 at 08:44, Ken Matsui via Libstdc++ <libstdc++@gcc.gnu.org> wrote:
>>
>> This patch optimizes the performance of the is_compound trait by
>> dispatching to the new __is_arithmetic built-in trait.
>>
>> libstdc++-v3/ChangeLog:
>>
>>         * include/std/type_traits (is_compound): Use __is_arithmetic
>>         built-in trait.
>>         (is_compound_v): Use is_fundamental_v instead.
>>
>> Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
>> ---
>>  libstdc++-v3/include/std/type_traits | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
>> index cf24de2fcac..73d9a2b16fc 100644
>> --- a/libstdc++-v3/include/std/type_traits
>> +++ b/libstdc++-v3/include/std/type_traits
>> @@ -702,9 +702,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>      { };
>>
>>    /// is_compound
>> +#if __has_builtin(__is_arithmetic)
>> +  template<typename _Tp>
>> +    struct is_compound
>> +    : public __bool_constant<!(__is_arithmetic(_Tp)
>> +                             || is_void<_Tp>::value
>> +                             || is_null_pointer<_Tp>::value)>
>> +    { };
>> +#else
>>    template<typename _Tp>
>>      struct is_compound
>>      : public __not_<is_fundamental<_Tp>>::type { };
>> +#endif
>
>
> I think it would be simpler to just do this unconditionally (i.e. just a single definition without using __has_builtin):
>
> template<typename _Tp>
>   struct is_compound
>   : __bool_constant<!is_fundamental<_Tp>::value>
>   { };
>
> This still avoids instantiating __not_. If is_fundamental is much more efficient now, then I think it's OK to instantiate it here. Otherwise we're duplicating the logic for is_fundamental, and just giving ourselves more code to maintain.
>
> Nobody ever uses is_compound anyway!
>
Agreed! Will fix this patch. Thank you!

>
>
>>
>>    /// @cond undocumented
>>    template<typename _Tp>
>> @@ -3234,7 +3243,7 @@ template <typename _Tp>
>>  template <typename _Tp>
>>    inline constexpr bool is_scalar_v = is_scalar<_Tp>::value;
>>  template <typename _Tp>
>> -  inline constexpr bool is_compound_v = is_compound<_Tp>::value;
>> +  inline constexpr bool is_compound_v = !is_fundamental_v<_Tp>;
>>  template <typename _Tp>
>>    inline constexpr bool is_member_pointer_v = is_member_pointer<_Tp>::value;
>>  template <typename _Tp>
>> --
>> 2.41.0
>>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index cf24de2fcac..73d9a2b16fc 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -702,9 +702,18 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { };
 
   /// is_compound
+#if __has_builtin(__is_arithmetic)
+  template<typename _Tp>
+    struct is_compound
+    : public __bool_constant<!(__is_arithmetic(_Tp)
+                             || is_void<_Tp>::value
+                             || is_null_pointer<_Tp>::value)>
+    { };
+#else
   template<typename _Tp>
     struct is_compound
     : public __not_<is_fundamental<_Tp>>::type { };
+#endif
 
   /// @cond undocumented
   template<typename _Tp>
@@ -3234,7 +3243,7 @@  template <typename _Tp>
 template <typename _Tp>
   inline constexpr bool is_scalar_v = is_scalar<_Tp>::value;
 template <typename _Tp>
-  inline constexpr bool is_compound_v = is_compound<_Tp>::value;
+  inline constexpr bool is_compound_v = !is_fundamental_v<_Tp>;
 template <typename _Tp>
   inline constexpr bool is_member_pointer_v = is_member_pointer<_Tp>::value;
 template <typename _Tp>