diff mbox series

libstdc++: use new built-in trait __is_reference

Message ID CAML+3pVYKw0zzseC1H+yKwO5L77S001qcvwgwh80AizD80djOw@mail.gmail.com
State Not Applicable
Headers
Series libstdc++: use new built-in trait __is_reference |

Checks

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

Commit Message

Ken Matsui March 19, 2023, 4:21 a.m. UTC 
  libstdc++-v3/ChangeLog:

* include/std/type_traits (is_reference): Use __is_reference built-in trait.
---

Comments

Xi Ruoyao March 20, 2023, 7:25 a.m. UTC | #1 
You need to CC libstdc++@gcc.gnu.org for any patches touching libstdc++.

On Sat, 2023-03-18 at 21:21 -0700, Ken Matsui via Gcc-patches wrote:
> libstdc++-v3/ChangeLog:
> 
> * include/std/type_traits (is_reference): Use __is_reference built-in
> trait.

Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
nothing) to indent the ChangeLog content.

Is there any benefit to use a builtin, instead of the existing
implementation?  I can see no but maybe I'm stupid.

> ---
> diff --git a/libstdc++-v3/include/std/type_traits
> b/libstdc++-v3/include/std/type_traits

The patch fails to apply.  It seems because your mail client inserted an
additional newline before "b/".  Try to use git-send-email or configure
the mail client properly.

> index 2bd607a8b8f..18408d8ceb6 100644
> --- a/libstdc++-v3/include/std/type_traits
> +++ b/libstdc++-v3/include/std/type_traits
> @@ -639,6 +639,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    // Composite type categories.
> 
>    /// is_reference
> +#if __has_builtin(__is_reference)
> +  template<typename _Tp>
> +    struct is_reference
> +    : public integral_constant<bool, __is_reference(_Tp)>

If a patch depends on another patch not applied yet, sent them in a
series.  Or people are puzzled because when this patch is applied alone,
the code fails to build.

> +    { };
> +#else
>    template<typename _Tp>
>      struct is_reference
>      : public false_type
> @@ -653,6 +659,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      struct is_reference<_Tp&&>
>      : public true_type
>      { };
> +#endif
> 
>    /// is_arithmetic
>    template<typename _Tp>
Ken Matsui March 20, 2023, 7:30 a.m. UTC | #2 
I see. Thank you!

On Mon, Mar 20, 2023 at 12:26 AM Xi Ruoyao <xry111@xry111.site> wrote:
>
> You need to CC libstdc++@gcc.gnu.org for any patches touching libstdc++.
>
> On Sat, 2023-03-18 at 21:21 -0700, Ken Matsui via Gcc-patches wrote:
> > libstdc++-v3/ChangeLog:
> >
> > * include/std/type_traits (is_reference): Use __is_reference built-in
> > trait.
>
> Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> nothing) to indent the ChangeLog content.
>
> Is there any benefit to use a builtin, instead of the existing
> implementation?  I can see no but maybe I'm stupid.
>
> > ---
> > diff --git a/libstdc++-v3/include/std/type_traits
> > b/libstdc++-v3/include/std/type_traits
>
> The patch fails to apply.  It seems because your mail client inserted an
> additional newline before "b/".  Try to use git-send-email or configure
> the mail client properly.
>
> > index 2bd607a8b8f..18408d8ceb6 100644
> > --- a/libstdc++-v3/include/std/type_traits
> > +++ b/libstdc++-v3/include/std/type_traits
> > @@ -639,6 +639,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >    // Composite type categories.
> >
> >    /// is_reference
> > +#if __has_builtin(__is_reference)
> > +  template<typename _Tp>
> > +    struct is_reference
> > +    : public integral_constant<bool, __is_reference(_Tp)>
>
> If a patch depends on another patch not applied yet, sent them in a
> series.  Or people are puzzled because when this patch is applied alone,
> the code fails to build.
>
> > +    { };
> > +#else
> >    template<typename _Tp>
> >      struct is_reference
> >      : public false_type
> > @@ -653,6 +659,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >      struct is_reference<_Tp&&>
> >      : public true_type
> >      { };
> > +#endif
> >
> >    /// is_arithmetic
> >    template<typename _Tp>
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
Xi Ruoyao March 20, 2023, 7:51 a.m. UTC | #3 
On Mon, 2023-03-20 at 00:30 -0700, Ken Matsui wrote:
> I see. Thank you!

Please continue to read.  I guess you missed some inline comments from
me...

> 
> On Mon, Mar 20, 2023 at 12:26 AM Xi Ruoyao <xry111@xry111.site> wrote:
> > 
> > You need to CC libstdc++@gcc.gnu.org for any patches touching
> > libstdc++.
> > 
> > On Sat, 2023-03-18 at 21:21 -0700, Ken Matsui via Gcc-patches wrote:
> > > libstdc++-v3/ChangeLog:
> > > 
> > > * include/std/type_traits (is_reference): Use __is_reference
> > > built-in
> > > trait.
> > 
> > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > nothing) to indent the ChangeLog content.
> > 
> > Is there any benefit to use a builtin, instead of the existing
> > implementation?  I can see no but maybe I'm stupid.
> > 
> > > ---
> > > diff --git a/libstdc++-v3/include/std/type_traits
> > > b/libstdc++-v3/include/std/type_traits
> > 
> > The patch fails to apply.  It seems because your mail client
> > inserted an
> > additional newline before "b/".  Try to use git-send-email or
> > configure
> > the mail client properly.
> > 
> > > index 2bd607a8b8f..18408d8ceb6 100644
> > > --- a/libstdc++-v3/include/std/type_traits
> > > +++ b/libstdc++-v3/include/std/type_traits
> > > @@ -639,6 +639,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >    // Composite type categories.
> > > 
> > >    /// is_reference
> > > +#if __has_builtin(__is_reference)
> > > +  template<typename _Tp>
> > > +    struct is_reference
> > > +    : public integral_constant<bool, __is_reference(_Tp)>
> > 
> > If a patch depends on another patch not applied yet, sent them in a
> > series.  Or people are puzzled because when this patch is applied
> > alone,
> > the code fails to build.
> > 
> > > +    { };
> > > +#else
> > >    template<typename _Tp>
> > >      struct is_reference
> > >      : public false_type
> > > @@ -653,6 +659,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > >      struct is_reference<_Tp&&>
> > >      : public true_type
> > >      { };
> > > +#endif
> > > 
> > >    /// is_arithmetic
> > >    template<typename _Tp>
> > 
> > --
> > Xi Ruoyao <xry111@xry111.site>
> > School of Aerospace Science and Technology, Xidian University
Ken Matsui March 20, 2023, 8:03 a.m. UTC | #4 
Oops, I assumed those were my email... Thank you for your heads up and
your comments!

> Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> nothing) to indent the ChangeLog content.

Do you mean like the following?

```
libstdc++-v3/ChangeLog:

[TAB]* include/std/type_traits (is_reference): Use __is_reference built-in
trait.
```

> Is there any benefit to use a builtin, instead of the existing
> implementation?  I can see no but maybe I'm stupid.

My patches are based on the GSoC project "C++: Implement compiler
built-in traits for the standard library traits". These built-in
traits basically make the compilation faster.

https://gcc.gnu.org/wiki/SummerOfCode

> The patch fails to apply.  It seems because your mail client inserted an
> additional newline before "b/".  Try to use git-send-email or configure
> the mail client properly.

Let me try to use git-send-email instead. I stupidly don't understand
how to use them, so I was making my patches manually...

> If a patch depends on another patch not applied yet, sent them in a
> series.  Or people are puzzled because when this patch is applied alone,
> the code fails to build.

Oooh, this one! [PATCH 3/7] - the third patch in a series of seven patches
I totally missed that. Thank you!

On Mon, Mar 20, 2023 at 12:51 AM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Mon, 2023-03-20 at 00:30 -0700, Ken Matsui wrote:
> > I see. Thank you!
>
> Please continue to read.  I guess you missed some inline comments from
> me...
>
> >
> > On Mon, Mar 20, 2023 at 12:26 AM Xi Ruoyao <xry111@xry111.site> wrote:
> > >
> > > You need to CC libstdc++@gcc.gnu.org for any patches touching
> > > libstdc++.
> > >
> > > On Sat, 2023-03-18 at 21:21 -0700, Ken Matsui via Gcc-patches wrote:
> > > > libstdc++-v3/ChangeLog:
> > > >
> > > > * include/std/type_traits (is_reference): Use __is_reference
> > > > built-in
> > > > trait.
> > >
> > > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > > nothing) to indent the ChangeLog content.
> > >
> > > Is there any benefit to use a builtin, instead of the existing
> > > implementation?  I can see no but maybe I'm stupid.
> > >
> > > > ---
> > > > diff --git a/libstdc++-v3/include/std/type_traits
> > > > b/libstdc++-v3/include/std/type_traits
> > >
> > > The patch fails to apply.  It seems because your mail client
> > > inserted an
> > > additional newline before "b/".  Try to use git-send-email or
> > > configure
> > > the mail client properly.
> > >
> > > > index 2bd607a8b8f..18408d8ceb6 100644
> > > > --- a/libstdc++-v3/include/std/type_traits
> > > > +++ b/libstdc++-v3/include/std/type_traits
> > > > @@ -639,6 +639,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > >    // Composite type categories.
> > > >
> > > >    /// is_reference
> > > > +#if __has_builtin(__is_reference)
> > > > +  template<typename _Tp>
> > > > +    struct is_reference
> > > > +    : public integral_constant<bool, __is_reference(_Tp)>
> > >
> > > If a patch depends on another patch not applied yet, sent them in a
> > > series.  Or people are puzzled because when this patch is applied
> > > alone,
> > > the code fails to build.
> > >
> > > > +    { };
> > > > +#else
> > > >    template<typename _Tp>
> > > >      struct is_reference
> > > >      : public false_type
> > > > @@ -653,6 +659,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > > >      struct is_reference<_Tp&&>
> > > >      : public true_type
> > > >      { };
> > > > +#endif
> > > >
> > > >    /// is_arithmetic
> > > >    template<typename _Tp>
> > >
> > > --
> > > Xi Ruoyao <xry111@xry111.site>
> > > School of Aerospace Science and Technology, Xidian University
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
Xi Ruoyao March 20, 2023, 8:07 a.m. UTC | #5 
On Mon, 2023-03-20 at 01:03 -0700, Ken Matsui wrote:
> Oops, I assumed those were my email... Thank you for your heads up and
> your comments!
> 
> > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > nothing) to indent the ChangeLog content.
> 
> Do you mean like the following?
> 
> ```
> libstdc++-v3/ChangeLog:
> 
> [TAB]* include/std/type_traits (is_reference): Use __is_reference
> built-in
> trait.
> ```

Yep.

> > Is there any benefit to use a builtin, instead of the existing
> > implementation?  I can see no but maybe I'm stupid.
> 
> My patches are based on the GSoC project "C++: Implement compiler
> built-in traits for the standard library traits". These built-in
> traits basically make the compilation faster.
> 
> https://gcc.gnu.org/wiki/SummerOfCode

Ok, to me making compilation faster is a valid reason.

> > The patch fails to apply.  It seems because your mail client inserted an
> > additional newline before "b/".  Try to use git-send-email or configure
> > the mail client properly.
> 
> Let me try to use git-send-email instead. I stupidly don't understand
> how to use them, so I was making my patches manually...

Or adjust the mail client correctly.  You can send the patch to yourself
first and see if it's not "mangled" by the mail client when you debug
such an issue...

But when you finally end up sending 10 patches in a series you'll find
git send-email much easier :).
Ken Matsui March 20, 2023, 8:32 a.m. UTC | #6 
Thank you!

On Mon, Mar 20, 2023 at 1:07 AM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On Mon, 2023-03-20 at 01:03 -0700, Ken Matsui wrote:
> > Oops, I assumed those were my email... Thank you for your heads up and
> > your comments!
> >
> > > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > > nothing) to indent the ChangeLog content.
> >
> > Do you mean like the following?
> >
> > ```
> > libstdc++-v3/ChangeLog:
> >
> > [TAB]* include/std/type_traits (is_reference): Use __is_reference
> > built-in
> > trait.
> > ```
>
> Yep.
>
> > > Is there any benefit to use a builtin, instead of the existing
> > > implementation?  I can see no but maybe I'm stupid.
> >
> > My patches are based on the GSoC project "C++: Implement compiler
> > built-in traits for the standard library traits". These built-in
> > traits basically make the compilation faster.
> >
> > https://gcc.gnu.org/wiki/SummerOfCode
>
> Ok, to me making compilation faster is a valid reason.
>
> > > The patch fails to apply.  It seems because your mail client inserted an
> > > additional newline before "b/".  Try to use git-send-email or configure
> > > the mail client properly.
> >
> > Let me try to use git-send-email instead. I stupidly don't understand
> > how to use them, so I was making my patches manually...
>
> Or adjust the mail client correctly.  You can send the patch to yourself
> first and see if it's not "mangled" by the mail client when you debug
> such an issue...
>
> But when you finally end up sending 10 patches in a series you'll find
> git send-email much easier :).
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
Jonathan Wakely March 20, 2023, 9:14 a.m. UTC | #7 
On Mon, 20 Mar 2023 at 08:08, Xi Ruoyao via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On Mon, 2023-03-20 at 01:03 -0700, Ken Matsui wrote:
> > Oops, I assumed those were my email... Thank you for your heads up and
> > your comments!
> >
> > > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > > nothing) to indent the ChangeLog content.
> >
> > Do you mean like the following?
> >
> > ```
> > libstdc++-v3/ChangeLog:
> >
> > [TAB]* include/std/type_traits (is_reference): Use __is_reference
> > built-in
> > trait.
> > ```
>
> Yep.
>
> > > Is there any benefit to use a builtin, instead of the existing
> > > implementation?  I can see no but maybe I'm stupid.
> >
> > My patches are based on the GSoC project "C++: Implement compiler
> > built-in traits for the standard library traits". These built-in
> > traits basically make the compilation faster.
> >
> > https://gcc.gnu.org/wiki/SummerOfCode
>
> Ok, to me making compilation faster is a valid reason.

Does it actually make compilation faster though?

Has it been measured?

> > > The patch fails to apply.  It seems because your mail client inserted an
> > > additional newline before "b/".  Try to use git-send-email or configure
> > > the mail client properly.
> >
> > Let me try to use git-send-email instead. I stupidly don't understand
> > how to use them, so I was making my patches manually...
>
> Or adjust the mail client correctly.  You can send the patch to yourself
> first and see if it's not "mangled" by the mail client when you debug
> such an issue...
>
> But when you finally end up sending 10 patches in a series you'll find
> git send-email much easier :).

Figuring out how to generate proper patches is an important part of
contributing to GCC, so part of any GSoC project.
Ken Matsui March 20, 2023, 9:56 a.m. UTC | #8 
> Does it actually make compilation faster though?
>
> Has it been measured?

In my understanding, what I have implemented so far is so simple that
it does not affect the speed. These traits are what Partick kindly
recommended to get started. As explained on the GSoC page, some traits
might involve expensive instantiation of multiple class templates. So
IMHO, implementing built-in traits for those traits can make
compilation cheaper.

I have not measured it, but Patrick might have done?

On Mon, Mar 20, 2023 at 2:14 AM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
> On Mon, 20 Mar 2023 at 08:08, Xi Ruoyao via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > On Mon, 2023-03-20 at 01:03 -0700, Ken Matsui wrote:
> > > Oops, I assumed those were my email... Thank you for your heads up and
> > > your comments!
> > >
> > > > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > > > nothing) to indent the ChangeLog content.
> > >
> > > Do you mean like the following?
> > >
> > > ```
> > > libstdc++-v3/ChangeLog:
> > >
> > > [TAB]* include/std/type_traits (is_reference): Use __is_reference
> > > built-in
> > > trait.
> > > ```
> >
> > Yep.
> >
> > > > Is there any benefit to use a builtin, instead of the existing
> > > > implementation?  I can see no but maybe I'm stupid.
> > >
> > > My patches are based on the GSoC project "C++: Implement compiler
> > > built-in traits for the standard library traits". These built-in
> > > traits basically make the compilation faster.
> > >
> > > https://gcc.gnu.org/wiki/SummerOfCode
> >
> > Ok, to me making compilation faster is a valid reason.
>
> Does it actually make compilation faster though?
>
> Has it been measured?
>
> > > > The patch fails to apply.  It seems because your mail client inserted an
> > > > additional newline before "b/".  Try to use git-send-email or configure
> > > > the mail client properly.
> > >
> > > Let me try to use git-send-email instead. I stupidly don't understand
> > > how to use them, so I was making my patches manually...
> >
> > Or adjust the mail client correctly.  You can send the patch to yourself
> > first and see if it's not "mangled" by the mail client when you debug
> > such an issue...
> >
> > But when you finally end up sending 10 patches in a series you'll find
> > git send-email much easier :).
>
> Figuring out how to generate proper patches is an important part of
> contributing to GCC, so part of any GSoC project.
Ken Matsui March 20, 2023, 1:22 p.m. UTC | #9 
It looks like I was able to use git send-email. The new patches became
a series, so I couldn't figure out how to associate them with this
email and another email. Please disregard this email.

On Mon, Mar 20, 2023 at 2:56 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
>
> > Does it actually make compilation faster though?
> >
> > Has it been measured?
>
> In my understanding, what I have implemented so far is so simple that
> it does not affect the speed. These traits are what Partick kindly
> recommended to get started. As explained on the GSoC page, some traits
> might involve expensive instantiation of multiple class templates. So
> IMHO, implementing built-in traits for those traits can make
> compilation cheaper.
>
> I have not measured it, but Patrick might have done?
>
> On Mon, Mar 20, 2023 at 2:14 AM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >
> > On Mon, 20 Mar 2023 at 08:08, Xi Ruoyao via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> > >
> > > On Mon, 2023-03-20 at 01:03 -0700, Ken Matsui wrote:
> > > > Oops, I assumed those were my email... Thank you for your heads up and
> > > > your comments!
> > > >
> > > > > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > > > > nothing) to indent the ChangeLog content.
> > > >
> > > > Do you mean like the following?
> > > >
> > > > ```
> > > > libstdc++-v3/ChangeLog:
> > > >
> > > > [TAB]* include/std/type_traits (is_reference): Use __is_reference
> > > > built-in
> > > > trait.
> > > > ```
> > >
> > > Yep.
> > >
> > > > > Is there any benefit to use a builtin, instead of the existing
> > > > > implementation?  I can see no but maybe I'm stupid.
> > > >
> > > > My patches are based on the GSoC project "C++: Implement compiler
> > > > built-in traits for the standard library traits". These built-in
> > > > traits basically make the compilation faster.
> > > >
> > > > https://gcc.gnu.org/wiki/SummerOfCode
> > >
> > > Ok, to me making compilation faster is a valid reason.
> >
> > Does it actually make compilation faster though?
> >
> > Has it been measured?
> >
> > > > > The patch fails to apply.  It seems because your mail client inserted an
> > > > > additional newline before "b/".  Try to use git-send-email or configure
> > > > > the mail client properly.
> > > >
> > > > Let me try to use git-send-email instead. I stupidly don't understand
> > > > how to use them, so I was making my patches manually...
> > >
> > > Or adjust the mail client correctly.  You can send the patch to yourself
> > > first and see if it's not "mangled" by the mail client when you debug
> > > such an issue...
> > >
> > > But when you finally end up sending 10 patches in a series you'll find
> > > git send-email much easier :).
> >
> > Figuring out how to generate proper patches is an important part of
> > contributing to GCC, so part of any GSoC project.
Patrick Palka March 20, 2023, 3:23 p.m. UTC | #10 
On Mon, Mar 20, 2023 at 5:56 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
>
> > Does it actually make compilation faster though?
> >
> > Has it been measured?
>
> In my understanding, what I have implemented so far is so simple that
> it does not affect the speed. These traits are what Partick kindly
> recommended to get started. As explained on the GSoC page, some traits
> might involve expensive instantiation of multiple class templates. So
> IMHO, implementing built-in traits for those traits can make
> compilation cheaper.
>
> I have not measured it, but Patrick might have done?

Although the native implementation of using two partial
specializations is very efficient, I'd suspect using a built-in would
be even better because it'd avoid the work of having to figure out
which of the two partial specializations to instantiate.

I contrived the attached benchmark to measure this, which instantiates
~160k specializations of is_reference_v (it was simpler to write a
benchmark for the variable template version). On my machine gcc trunk
(release build) as well as Clang 15 use between 10-15% less
time/memory with -DUSE_BUILTIN than without for this benchmark, so
this suggests the built-in improves compile time/memory usage for this
trait by at least 10-15%.  For more complicated traits the improvement
should be even better.

>
> On Mon, Mar 20, 2023 at 2:14 AM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >
> > On Mon, 20 Mar 2023 at 08:08, Xi Ruoyao via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> > >
> > > On Mon, 2023-03-20 at 01:03 -0700, Ken Matsui wrote:
> > > > Oops, I assumed those were my email... Thank you for your heads up and
> > > > your comments!
> > > >
> > > > > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces, nor
> > > > > nothing) to indent the ChangeLog content.
> > > >
> > > > Do you mean like the following?
> > > >
> > > > ```
> > > > libstdc++-v3/ChangeLog:
> > > >
> > > > [TAB]* include/std/type_traits (is_reference): Use __is_reference
> > > > built-in
> > > > trait.
> > > > ```
> > >
> > > Yep.
> > >
> > > > > Is there any benefit to use a builtin, instead of the existing
> > > > > implementation?  I can see no but maybe I'm stupid.
> > > >
> > > > My patches are based on the GSoC project "C++: Implement compiler
> > > > built-in traits for the standard library traits". These built-in
> > > > traits basically make the compilation faster.
> > > >
> > > > https://gcc.gnu.org/wiki/SummerOfCode
> > >
> > > Ok, to me making compilation faster is a valid reason.
> >
> > Does it actually make compilation faster though?
> >
> > Has it been measured?
> >
> > > > > The patch fails to apply.  It seems because your mail client inserted an
> > > > > additional newline before "b/".  Try to use git-send-email or configure
> > > > > the mail client properly.
> > > >
> > > > Let me try to use git-send-email instead. I stupidly don't understand
> > > > how to use them, so I was making my patches manually...
> > >
> > > Or adjust the mail client correctly.  You can send the patch to yourself
> > > first and see if it's not "mangled" by the mail client when you debug
> > > such an issue...
> > >
> > > But when you finally end up sending 10 patches in a series you'll find
> > > git send-email much easier :).
> >
> > Figuring out how to generate proper patches is an important part of
> > contributing to GCC, so part of any GSoC project.
>
Ken Matsui March 20, 2023, 8:24 p.m. UTC | #11 
Thank you so much for taking the benchmark! This is a great improvement
than I thought.

In GCC contributions, do I need to benchmark (and report) every built-in
trait I implement?

On Mon, Mar 20, 2023 at 8:24 AM Patrick Palka <ppalka@redhat.com> wrote:

> On Mon, Mar 20, 2023 at 5:56 AM Ken Matsui <kmatsui@cs.washington.edu>
> wrote:
> >
> > > Does it actually make compilation faster though?
> > >
> > > Has it been measured?
> >
> > In my understanding, what I have implemented so far is so simple that
> > it does not affect the speed. These traits are what Partick kindly
> > recommended to get started. As explained on the GSoC page, some traits
> > might involve expensive instantiation of multiple class templates. So
> > IMHO, implementing built-in traits for those traits can make
> > compilation cheaper.
> >
> > I have not measured it, but Patrick might have done?
>
> Although the native implementation of using two partial
> specializations is very efficient, I'd suspect using a built-in would
> be even better because it'd avoid the work of having to figure out
> which of the two partial specializations to instantiate.
>
> I contrived the attached benchmark to measure this, which instantiates
> ~160k specializations of is_reference_v (it was simpler to write a
> benchmark for the variable template version). On my machine gcc trunk
> (release build) as well as Clang 15 use between 10-15% less
> time/memory with -DUSE_BUILTIN than without for this benchmark, so
> this suggests the built-in improves compile time/memory usage for this
> trait by at least 10-15%.  For more complicated traits the improvement
> should be even better.
>
> >
> > On Mon, Mar 20, 2023 at 2:14 AM Jonathan Wakely <jwakely.gcc@gmail.com>
> wrote:
> > >
> > > On Mon, 20 Mar 2023 at 08:08, Xi Ruoyao via Libstdc++
> > > <libstdc++@gcc.gnu.org> wrote:
> > > >
> > > > On Mon, 2023-03-20 at 01:03 -0700, Ken Matsui wrote:
> > > > > Oops, I assumed those were my email... Thank you for your heads up
> and
> > > > > your comments!
> > > > >
> > > > > > Bad ChangeLog format.  You should have a tab (not 4 or 8 spaces,
> nor
> > > > > > nothing) to indent the ChangeLog content.
> > > > >
> > > > > Do you mean like the following?
> > > > >
> > > > > ```
> > > > > libstdc++-v3/ChangeLog:
> > > > >
> > > > > [TAB]* include/std/type_traits (is_reference): Use __is_reference
> > > > > built-in
> > > > > trait.
> > > > > ```
> > > >
> > > > Yep.
> > > >
> > > > > > Is there any benefit to use a builtin, instead of the existing
> > > > > > implementation?  I can see no but maybe I'm stupid.
> > > > >
> > > > > My patches are based on the GSoC project "C++: Implement compiler
> > > > > built-in traits for the standard library traits". These built-in
> > > > > traits basically make the compilation faster.
> > > > >
> > > > > https://gcc.gnu.org/wiki/SummerOfCode
> > > >
> > > > Ok, to me making compilation faster is a valid reason.
> > >
> > > Does it actually make compilation faster though?
> > >
> > > Has it been measured?
> > >
> > > > > > The patch fails to apply.  It seems because your mail client
> inserted an
> > > > > > additional newline before "b/".  Try to use git-send-email or
> configure
> > > > > > the mail client properly.
> > > > >
> > > > > Let me try to use git-send-email instead. I stupidly don't
> understand
> > > > > how to use them, so I was making my patches manually...
> > > >
> > > > Or adjust the mail client correctly.  You can send the patch to
> yourself
> > > > first and see if it's not "mangled" by the mail client when you debug
> > > > such an issue...
> > > >
> > > > But when you finally end up sending 10 patches in a series you'll
> find
> > > > git send-email much easier :).
> > >
> > > Figuring out how to generate proper patches is an important part of
> > > contributing to GCC, so part of any GSoC project.
> >
>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/type_traits
b/libstdc++-v3/include/std/type_traits
index 2bd607a8b8f..18408d8ceb6 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -639,6 +639,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // Composite type categories.

   /// is_reference
+#if __has_builtin(__is_reference)
+  template<typename _Tp>
+    struct is_reference
+    : public integral_constant<bool, __is_reference(_Tp)>
+    { };
+#else
   template<typename _Tp>
     struct is_reference
     : public false_type
@@ -653,6 +659,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct is_reference<_Tp&&>
     : public true_type
     { };
+#endif

   /// is_arithmetic
   template<typename _Tp>