diff mbox series

[2/2] libstdc++: use new built-in trait __add_const

Message ID 20230321111056.78121-2-kmatsui@cs.washington.edu
State Superseded
Headers
Series [1/2] c++: implement __add_const built-in trait |

Commit Message

Ken Matsui March 21, 2023, 11:10 a.m. UTC 
  This patch lets libstdc++ use new built-in trait __add_const.

libstdc++-v3/ChangeLog:

	* include/std/type_traits (add_const): Use __add_const built-in trait.
---
 libstdc++-v3/include/std/type_traits | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Marc Glisse March 21, 2023, 11:20 a.m. UTC | #1 
On Tue, 21 Mar 2023, Ken Matsui via Libstdc++ wrote:

>   /// add_const
> +#if __has_builtin(__add_const)
> +  template<typename _Tp>
> +    struct add_const
> +    { using type = __add_const(_Tp); };
> +#else
>   template<typename _Tp>
>     struct add_const
>     { using type = _Tp const; };
> +#endif

Is that really better? You asked elsewhere if you should measure for each 
patch, and I think that at least for such a trivial case, you need to 
demonstrate that there is a point. The drawbacks are obvious: more code in 
libstdc++, non-standard, and more builtins in the compiler.

Using builtins makes more sense for complicated traits where you can save 
several instantiations. Now that you have done a couple simple cases to 
see how it works, I think you should concentrate on the more complicated 
cases.
Jonathan Wakely March 21, 2023, 11:24 a.m. UTC | #2 
On Tue, 21 Mar 2023 at 11:21, Marc Glisse via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> On Tue, 21 Mar 2023, Ken Matsui via Libstdc++ wrote:
>
> >   /// add_const
> > +#if __has_builtin(__add_const)
> > +  template<typename _Tp>
> > +    struct add_const
> > +    { using type = __add_const(_Tp); };
> > +#else
> >   template<typename _Tp>
> >     struct add_const
> >     { using type = _Tp const; };
> > +#endif
>
> Is that really better? You asked elsewhere if you should measure for each
> patch, and I think that at least for such a trivial case, you need to
> demonstrate that there is a point. The drawbacks are obvious: more code in
> libstdc++, non-standard, and more builtins in the compiler.
>

Right, this one isn't even getting rid of any partial specializations, but
it is giving the preprocessor more work to do.

Adding the extra built-ins to the compiler makes the compiler (very
slightly) bigger and slower, so a real benchmark would require comparing an
unpatched gcc (without the new built-in) to a patched gcc and patched
libstdc++ sources.



>
> Using builtins makes more sense for complicated traits where you can save
> several instantiations. Now that you have done a couple simple cases to
> see how it works, I think you should concentrate on the more complicated
> cases.
>
> --
> Marc Glisse
>
>
Ken Matsui March 21, 2023, 11:37 a.m. UTC | #3 
Thank you for your information. Although it matches my intuition, I sent
this patch because I was unsure my intuition was correct. As Jonathan
pointed out, there appear to be several implementation errors. The
benchmark result for this trait is kind of trivial, so I will implement the
other traits I want to implement and then come back here.

Thank you all for your help.

On Tue, Mar 21, 2023 at 4:25 AM Jonathan Wakely <jwakely@redhat.com> wrote:

>
>
> On Tue, 21 Mar 2023 at 11:21, Marc Glisse via Libstdc++ <
> libstdc++@gcc.gnu.org> wrote:
>
>> On Tue, 21 Mar 2023, Ken Matsui via Libstdc++ wrote:
>>
>> >   /// add_const
>> > +#if __has_builtin(__add_const)
>> > +  template<typename _Tp>
>> > +    struct add_const
>> > +    { using type = __add_const(_Tp); };
>> > +#else
>> >   template<typename _Tp>
>> >     struct add_const
>> >     { using type = _Tp const; };
>> > +#endif
>>
>> Is that really better? You asked elsewhere if you should measure for each
>> patch, and I think that at least for such a trivial case, you need to
>> demonstrate that there is a point. The drawbacks are obvious: more code
>> in
>> libstdc++, non-standard, and more builtins in the compiler.
>>
>
> Right, this one isn't even getting rid of any partial specializations, but
> it is giving the preprocessor more work to do.
>
> Adding the extra built-ins to the compiler makes the compiler (very
> slightly) bigger and slower, so a real benchmark would require comparing an
> unpatched gcc (without the new built-in) to a patched gcc and patched
> libstdc++ sources.
>
>
>
>>
>> Using builtins makes more sense for complicated traits where you can save
>> several instantiations. Now that you have done a couple simple cases to
>> see how it works, I think you should concentrate on the more complicated
>> cases.
>>
>> --
>> Marc Glisse
>>
>>
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index 2bd607a8b8f..1ac75a928c3 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -1560,9 +1560,15 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 
   /// add_const
+#if __has_builtin(__add_const)
+  template<typename _Tp>
+    struct add_const
+    { using type = __add_const(_Tp); };
+#else
   template<typename _Tp>
     struct add_const
     { using type = _Tp const; };
+#endif
 
   /// add_volatile
   template<typename _Tp>