diff mbox series

[v23,31/33] libstdc++: Optimize std::is_pointer compilation performance

Message ID 20231020135748.1846670-32-kmatsui@gcc.gnu.org
State Unresolved
Headers
Series Optimize type traits performance |

Checks

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

Commit Message

Ken Matsui Oct. 20, 2023, 1:53 p.m. UTC 
  This patch optimizes the compilation performance of std::is_pointer
by dispatching to the new __is_pointer built-in trait.

libstdc++-v3/ChangeLog:

	* include/bits/cpp_type_traits.h (__is_pointer): Use __is_pointer
	built-in trait.
	* include/std/type_traits (is_pointer): Likewise. Optimize its
	implementation.
	(is_pointer_v): Likewise.

Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
---
 libstdc++-v3/include/bits/cpp_type_traits.h |  8 ++++
 libstdc++-v3/include/std/type_traits        | 44 +++++++++++++++++----
 2 files changed, 44 insertions(+), 8 deletions(-)

Comments

Ken Matsui Oct. 22, 2023, 12:06 p.m. UTC | #1 
Hi Patrick,

There is an issue with the code in
libstdc++-v3/include/bits/cpp_type_traits.h. Specifically, Clang 16
does not accept the code, while Clang 17 does. Given that we aim to
support the last two versions of Clang, we need to ensure that Clang
16 accepts this code. Can you please advise on the best course of
action regarding this matter?

https://godbolt.org/z/PbxhYcb7q

Sincerely,
Ken Matsui

On Fri, Oct 20, 2023 at 7:12 AM Ken Matsui <kmatsui@gcc.gnu.org> wrote:
>
> This patch optimizes the compilation performance of std::is_pointer
> by dispatching to the new __is_pointer built-in trait.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/cpp_type_traits.h (__is_pointer): Use __is_pointer
>         built-in trait.
>         * include/std/type_traits (is_pointer): Likewise. Optimize its
>         implementation.
>         (is_pointer_v): Likewise.
>
> Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
> Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
> ---
>  libstdc++-v3/include/bits/cpp_type_traits.h |  8 ++++
>  libstdc++-v3/include/std/type_traits        | 44 +++++++++++++++++----
>  2 files changed, 44 insertions(+), 8 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/cpp_type_traits.h b/libstdc++-v3/include/bits/cpp_type_traits.h
> index 4312f32a4e0..246f2cc0b17 100644
> --- a/libstdc++-v3/include/bits/cpp_type_traits.h
> +++ b/libstdc++-v3/include/bits/cpp_type_traits.h
> @@ -363,6 +363,13 @@ __INT_N(__GLIBCXX_TYPE_INT_N_3)
>    //
>    // Pointer types
>    //
> +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> +  template<typename _Tp>
> +    struct __is_pointer : __truth_type<__is_pointer(_Tp)>
> +    {
> +      enum { __value = __is_pointer(_Tp) };
> +    };
> +#else
>    template<typename _Tp>
>      struct __is_pointer
>      {
> @@ -376,6 +383,7 @@ __INT_N(__GLIBCXX_TYPE_INT_N_3)
>        enum { __value = 1 };
>        typedef __true_type __type;
>      };
> +#endif
>
>    //
>    // An arithmetic type is an integer type or a floating point type
> diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
> index 0641ecfdf2b..75a94cb8d7e 100644
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -542,19 +542,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      : public true_type { };
>  #endif
>
> -  template<typename>
> -    struct __is_pointer_helper
> +  /// is_pointer
> +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> +  template<typename _Tp>
> +    struct is_pointer
> +    : public __bool_constant<__is_pointer(_Tp)>
> +    { };
> +#else
> +  template<typename _Tp>
> +    struct is_pointer
>      : public false_type { };
>
>    template<typename _Tp>
> -    struct __is_pointer_helper<_Tp*>
> +    struct is_pointer<_Tp*>
>      : public true_type { };
>
> -  /// is_pointer
>    template<typename _Tp>
> -    struct is_pointer
> -    : public __is_pointer_helper<__remove_cv_t<_Tp>>::type
> -    { };
> +    struct is_pointer<_Tp* const>
> +    : public true_type { };
> +
> +  template<typename _Tp>
> +    struct is_pointer<_Tp* volatile>
> +    : public true_type { };
> +
> +  template<typename _Tp>
> +    struct is_pointer<_Tp* const volatile>
> +    : public true_type { };
> +#endif
>
>    /// is_lvalue_reference
>    template<typename>
> @@ -3252,8 +3266,22 @@ template <typename _Tp, size_t _Num>
>    inline constexpr bool is_array_v<_Tp[_Num]> = true;
>  #endif
>
> +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> +template <typename _Tp>
> +  inline constexpr bool is_pointer_v = __is_pointer(_Tp);
> +#else
>  template <typename _Tp>
> -  inline constexpr bool is_pointer_v = is_pointer<_Tp>::value;
> +  inline constexpr bool is_pointer_v = false;
> +template <typename _Tp>
> +  inline constexpr bool is_pointer_v<_Tp*> = true;
> +template <typename _Tp>
> +  inline constexpr bool is_pointer_v<_Tp* const> = true;
> +template <typename _Tp>
> +  inline constexpr bool is_pointer_v<_Tp* volatile> = true;
> +template <typename _Tp>
> +  inline constexpr bool is_pointer_v<_Tp* const volatile> = true;
> +#endif
> +
>  template <typename _Tp>
>    inline constexpr bool is_lvalue_reference_v = false;
>  template <typename _Tp>
> --
> 2.42.0
>
Patrick Palka Oct. 23, 2023, 5 p.m. UTC | #2 
On Sun, 22 Oct 2023, Ken Matsui wrote:

> Hi Patrick,
> 
> There is an issue with the code in
> libstdc++-v3/include/bits/cpp_type_traits.h. Specifically, Clang 16
> does not accept the code, while Clang 17 does. Given that we aim to
> support the last two versions of Clang, we need to ensure that Clang
> 16 accepts this code. Can you please advise on the best course of
> action regarding this matter?

The following workaround seems to make Clang happy:

    #include <type_traits>

    template<typename T, bool B = __is_pointer(T)>
    struct __is_pointer : std::bool_constant<B> { };

> 
> https://godbolt.org/z/PbxhYcb7q
> 
> Sincerely,
> Ken Matsui
> 
> On Fri, Oct 20, 2023 at 7:12 AM Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> >
> > This patch optimizes the compilation performance of std::is_pointer
> > by dispatching to the new __is_pointer built-in trait.
> >
> > libstdc++-v3/ChangeLog:
> >
> >         * include/bits/cpp_type_traits.h (__is_pointer): Use __is_pointer
> >         built-in trait.
> >         * include/std/type_traits (is_pointer): Likewise. Optimize its
> >         implementation.
> >         (is_pointer_v): Likewise.
> >
> > Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
> > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
> > ---
> >  libstdc++-v3/include/bits/cpp_type_traits.h |  8 ++++
> >  libstdc++-v3/include/std/type_traits        | 44 +++++++++++++++++----
> >  2 files changed, 44 insertions(+), 8 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/cpp_type_traits.h b/libstdc++-v3/include/bits/cpp_type_traits.h
> > index 4312f32a4e0..246f2cc0b17 100644
> > --- a/libstdc++-v3/include/bits/cpp_type_traits.h
> > +++ b/libstdc++-v3/include/bits/cpp_type_traits.h
> > @@ -363,6 +363,13 @@ __INT_N(__GLIBCXX_TYPE_INT_N_3)
> >    //
> >    // Pointer types
> >    //
> > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> > +  template<typename _Tp>
> > +    struct __is_pointer : __truth_type<__is_pointer(_Tp)>
> > +    {
> > +      enum { __value = __is_pointer(_Tp) };
> > +    };
> > +#else
> >    template<typename _Tp>
> >      struct __is_pointer
> >      {
> > @@ -376,6 +383,7 @@ __INT_N(__GLIBCXX_TYPE_INT_N_3)
> >        enum { __value = 1 };
> >        typedef __true_type __type;
> >      };
> > +#endif
> >
> >    //
> >    // An arithmetic type is an integer type or a floating point type
> > diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
> > index 0641ecfdf2b..75a94cb8d7e 100644
> > --- a/libstdc++-v3/include/std/type_traits
> > +++ b/libstdc++-v3/include/std/type_traits
> > @@ -542,19 +542,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >      : public true_type { };
> >  #endif
> >
> > -  template<typename>
> > -    struct __is_pointer_helper
> > +  /// is_pointer
> > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> > +  template<typename _Tp>
> > +    struct is_pointer
> > +    : public __bool_constant<__is_pointer(_Tp)>
> > +    { };
> > +#else
> > +  template<typename _Tp>
> > +    struct is_pointer
> >      : public false_type { };
> >
> >    template<typename _Tp>
> > -    struct __is_pointer_helper<_Tp*>
> > +    struct is_pointer<_Tp*>
> >      : public true_type { };
> >
> > -  /// is_pointer
> >    template<typename _Tp>
> > -    struct is_pointer
> > -    : public __is_pointer_helper<__remove_cv_t<_Tp>>::type
> > -    { };
> > +    struct is_pointer<_Tp* const>
> > +    : public true_type { };
> > +
> > +  template<typename _Tp>
> > +    struct is_pointer<_Tp* volatile>
> > +    : public true_type { };
> > +
> > +  template<typename _Tp>
> > +    struct is_pointer<_Tp* const volatile>
> > +    : public true_type { };
> > +#endif
> >
> >    /// is_lvalue_reference
> >    template<typename>
> > @@ -3252,8 +3266,22 @@ template <typename _Tp, size_t _Num>
> >    inline constexpr bool is_array_v<_Tp[_Num]> = true;
> >  #endif
> >
> > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> > +template <typename _Tp>
> > +  inline constexpr bool is_pointer_v = __is_pointer(_Tp);
> > +#else
> >  template <typename _Tp>
> > -  inline constexpr bool is_pointer_v = is_pointer<_Tp>::value;
> > +  inline constexpr bool is_pointer_v = false;
> > +template <typename _Tp>
> > +  inline constexpr bool is_pointer_v<_Tp*> = true;
> > +template <typename _Tp>
> > +  inline constexpr bool is_pointer_v<_Tp* const> = true;
> > +template <typename _Tp>
> > +  inline constexpr bool is_pointer_v<_Tp* volatile> = true;
> > +template <typename _Tp>
> > +  inline constexpr bool is_pointer_v<_Tp* const volatile> = true;
> > +#endif
> > +
> >  template <typename _Tp>
> >    inline constexpr bool is_lvalue_reference_v = false;
> >  template <typename _Tp>
> > --
> > 2.42.0
> >
> 
>
Ken Matsui Oct. 23, 2023, 5:12 p.m. UTC | #3 
On Mon, Oct 23, 2023 at 10:00 AM Patrick Palka <ppalka@redhat.com> wrote:

> On Sun, 22 Oct 2023, Ken Matsui wrote:
>
> > Hi Patrick,
> >
> > There is an issue with the code in
> > libstdc++-v3/include/bits/cpp_type_traits.h. Specifically, Clang 16
> > does not accept the code, while Clang 17 does. Given that we aim to
> > support the last two versions of Clang, we need to ensure that Clang
> > 16 accepts this code. Can you please advise on the best course of
> > action regarding this matter?
>
> The following workaround seems to make Clang happy:
>
>     #include <type_traits>
>
>     template<typename T, bool B = __is_pointer(T)>
>     struct __is_pointer : std::bool_constant<B> { };
>

Ooh, this makes sense. Thank you!


> >
> > https://godbolt.org/z/PbxhYcb7q
> >
> > Sincerely,
> > Ken Matsui
> >
> > On Fri, Oct 20, 2023 at 7:12 AM Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > >
> > > This patch optimizes the compilation performance of std::is_pointer
> > > by dispatching to the new __is_pointer built-in trait.
> > >
> > > libstdc++-v3/ChangeLog:
> > >
> > >         * include/bits/cpp_type_traits.h (__is_pointer): Use
> __is_pointer
> > >         built-in trait.
> > >         * include/std/type_traits (is_pointer): Likewise. Optimize its
> > >         implementation.
> > >         (is_pointer_v): Likewise.
> > >
> > > Co-authored-by: Jonathan Wakely <jwakely@redhat.com>
> > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
> > > ---
> > >  libstdc++-v3/include/bits/cpp_type_traits.h |  8 ++++
> > >  libstdc++-v3/include/std/type_traits        | 44 +++++++++++++++++----
> > >  2 files changed, 44 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/libstdc++-v3/include/bits/cpp_type_traits.h
> b/libstdc++-v3/include/bits/cpp_type_traits.h
> > > index 4312f32a4e0..246f2cc0b17 100644
> > > --- a/libstdc++-v3/include/bits/cpp_type_traits.h
> > > +++ b/libstdc++-v3/include/bits/cpp_type_traits.h
> > > @@ -363,6 +363,13 @@ __INT_N(__GLIBCXX_TYPE_INT_N_3)
> > >    //
> > >    // Pointer types
> > >    //
> > > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> > > +  template<typename _Tp>
> > > +    struct __is_pointer : __truth_type<__is_pointer(_Tp)>
> > > +    {
> > > +      enum { __value = __is_pointer(_Tp) };
> > > +    };
> > > +#else
> > >    template<typename _Tp>
> > >      struct __is_pointer
> > >      {
> > > @@ -376,6 +383,7 @@ __INT_N(__GLIBCXX_TYPE_INT_N_3)
> > >        enum { __value = 1 };
> > >        typedef __true_type __type;
> > >      };
> > > +#endif
> > >
> > >    //
> > >    // An arithmetic type is an integer type or a floating point type
> > > diff --git a/libstdc++-v3/include/std/type_traits
> b/libstdc++-v3/include/std/type_traits
> > > index 0641ecfdf2b..75a94cb8d7e 100644
> > > --- a/libstdc++-v3/include/std/type_traits
> > > +++ b/libstdc++-v3/include/std/type_traits
> > > @@ -542,19 +542,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >      : public true_type { };
> > >  #endif
> > >
> > > -  template<typename>
> > > -    struct __is_pointer_helper
> > > +  /// is_pointer
> > > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> > > +  template<typename _Tp>
> > > +    struct is_pointer
> > > +    : public __bool_constant<__is_pointer(_Tp)>
> > > +    { };
> > > +#else
> > > +  template<typename _Tp>
> > > +    struct is_pointer
> > >      : public false_type { };
> > >
> > >    template<typename _Tp>
> > > -    struct __is_pointer_helper<_Tp*>
> > > +    struct is_pointer<_Tp*>
> > >      : public true_type { };
> > >
> > > -  /// is_pointer
> > >    template<typename _Tp>
> > > -    struct is_pointer
> > > -    : public __is_pointer_helper<__remove_cv_t<_Tp>>::type
> > > -    { };
> > > +    struct is_pointer<_Tp* const>
> > > +    : public true_type { };
> > > +
> > > +  template<typename _Tp>
> > > +    struct is_pointer<_Tp* volatile>
> > > +    : public true_type { };
> > > +
> > > +  template<typename _Tp>
> > > +    struct is_pointer<_Tp* const volatile>
> > > +    : public true_type { };
> > > +#endif
> > >
> > >    /// is_lvalue_reference
> > >    template<typename>
> > > @@ -3252,8 +3266,22 @@ template <typename _Tp, size_t _Num>
> > >    inline constexpr bool is_array_v<_Tp[_Num]> = true;
> > >  #endif
> > >
> > > +#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
> > > +template <typename _Tp>
> > > +  inline constexpr bool is_pointer_v = __is_pointer(_Tp);
> > > +#else
> > >  template <typename _Tp>
> > > -  inline constexpr bool is_pointer_v = is_pointer<_Tp>::value;
> > > +  inline constexpr bool is_pointer_v = false;
> > > +template <typename _Tp>
> > > +  inline constexpr bool is_pointer_v<_Tp*> = true;
> > > +template <typename _Tp>
> > > +  inline constexpr bool is_pointer_v<_Tp* const> = true;
> > > +template <typename _Tp>
> > > +  inline constexpr bool is_pointer_v<_Tp* volatile> = true;
> > > +template <typename _Tp>
> > > +  inline constexpr bool is_pointer_v<_Tp* const volatile> = true;
> > > +#endif
> > > +
> > >  template <typename _Tp>
> > >    inline constexpr bool is_lvalue_reference_v = false;
> > >  template <typename _Tp>
> > > --
> > > 2.42.0
> > >
> >
> >
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/cpp_type_traits.h b/libstdc++-v3/include/bits/cpp_type_traits.h
index 4312f32a4e0..246f2cc0b17 100644
--- a/libstdc++-v3/include/bits/cpp_type_traits.h
+++ b/libstdc++-v3/include/bits/cpp_type_traits.h
@@ -363,6 +363,13 @@  __INT_N(__GLIBCXX_TYPE_INT_N_3)
   //
   // Pointer types
   //
+#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
+  template<typename _Tp>
+    struct __is_pointer : __truth_type<__is_pointer(_Tp)>
+    {
+      enum { __value = __is_pointer(_Tp) };
+    };
+#else
   template<typename _Tp>
     struct __is_pointer
     {
@@ -376,6 +383,7 @@  __INT_N(__GLIBCXX_TYPE_INT_N_3)
       enum { __value = 1 };
       typedef __true_type __type;
     };
+#endif
 
   //
   // An arithmetic type is an integer type or a floating point type
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index 0641ecfdf2b..75a94cb8d7e 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -542,19 +542,33 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     : public true_type { };
 #endif
 
-  template<typename>
-    struct __is_pointer_helper
+  /// is_pointer
+#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
+  template<typename _Tp>
+    struct is_pointer
+    : public __bool_constant<__is_pointer(_Tp)>
+    { };
+#else
+  template<typename _Tp>
+    struct is_pointer
     : public false_type { };
 
   template<typename _Tp>
-    struct __is_pointer_helper<_Tp*>
+    struct is_pointer<_Tp*>
     : public true_type { };
 
-  /// is_pointer
   template<typename _Tp>
-    struct is_pointer
-    : public __is_pointer_helper<__remove_cv_t<_Tp>>::type
-    { };
+    struct is_pointer<_Tp* const>
+    : public true_type { };
+
+  template<typename _Tp>
+    struct is_pointer<_Tp* volatile>
+    : public true_type { };
+
+  template<typename _Tp>
+    struct is_pointer<_Tp* const volatile>
+    : public true_type { };
+#endif
 
   /// is_lvalue_reference
   template<typename>
@@ -3252,8 +3266,22 @@  template <typename _Tp, size_t _Num>
   inline constexpr bool is_array_v<_Tp[_Num]> = true;
 #endif
 
+#if _GLIBCXX_USE_BUILTIN_TRAIT(__is_pointer)
+template <typename _Tp>
+  inline constexpr bool is_pointer_v = __is_pointer(_Tp);
+#else
 template <typename _Tp>
-  inline constexpr bool is_pointer_v = is_pointer<_Tp>::value;
+  inline constexpr bool is_pointer_v = false;
+template <typename _Tp>
+  inline constexpr bool is_pointer_v<_Tp*> = true;
+template <typename _Tp>
+  inline constexpr bool is_pointer_v<_Tp* const> = true;
+template <typename _Tp>
+  inline constexpr bool is_pointer_v<_Tp* volatile> = true;
+template <typename _Tp>
+  inline constexpr bool is_pointer_v<_Tp* const volatile> = true;
+#endif
+
 template <typename _Tp>
   inline constexpr bool is_lvalue_reference_v = false;
 template <typename _Tp>