diff mbox series

[1/2] c++: implement __remove_pointer built-in trait

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

Checks

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

Commit Message

Ken Matsui June 15, 2023, 12:21 p.m. UTC 
  This patch implements built-in trait for std::remove_pointer.

gcc/cp/ChangeLog:

	* cp-trait.def: Define __remove_pointer.
	* semantics.cc (finish_trait_type): Handle CPTK_REMOVE_POINTER.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/has-builtin-1.C: Test existence of __remove_pointer.
	* g++.dg/ext/remove_pointer.C: New test.

Signed-off-by: Ken Matsui <kmatsui@cs.washington.edu>
---
 gcc/cp/cp-trait.def                       |  1 +
 gcc/cp/semantics.cc                       |  4 ++
 gcc/testsuite/g++.dg/ext/has-builtin-1.C  |  3 ++
 gcc/testsuite/g++.dg/ext/remove_pointer.C | 51 +++++++++++++++++++++++
 4 files changed, 59 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/ext/remove_pointer.C

Comments

Ken Matsui June 17, 2023, 12:35 p.m. UTC | #1 
Hi,

I conducted a benchmark for remove_pointer as well as is_object. Just
like the is_object benchmark, here is the benchmark code:

https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer_benchmark.cc

On my computer, using the gcc HEAD of this patch for a release build,
the patch with -DUSE_BUILTIN took 8.7% less time and used 4.3-4.9%
less memory on average compared to not using it. Although the
performance improvement was not as significant as with is_object, the
benchmark demonstrated that the compilation was consistently more
efficient.

Sincerely,
Ken Matsui

On Thu, Jun 15, 2023 at 5:22 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
>
> This patch implements built-in trait for std::remove_pointer.
>
> gcc/cp/ChangeLog:
>
>         * cp-trait.def: Define __remove_pointer.
>         * semantics.cc (finish_trait_type): Handle CPTK_REMOVE_POINTER.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/ext/has-builtin-1.C: Test existence of __remove_pointer.
>         * g++.dg/ext/remove_pointer.C: New test.
>
> Signed-off-by: Ken Matsui <kmatsui@cs.washington.edu>
> ---
>  gcc/cp/cp-trait.def                       |  1 +
>  gcc/cp/semantics.cc                       |  4 ++
>  gcc/testsuite/g++.dg/ext/has-builtin-1.C  |  3 ++
>  gcc/testsuite/g++.dg/ext/remove_pointer.C | 51 +++++++++++++++++++++++
>  4 files changed, 59 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/ext/remove_pointer.C
>
> diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def
> index 8b7fece0cc8..07823e55579 100644
> --- a/gcc/cp/cp-trait.def
> +++ b/gcc/cp/cp-trait.def
> @@ -90,6 +90,7 @@ DEFTRAIT_EXPR (IS_DEDUCIBLE, "__is_deducible ", 2)
>  DEFTRAIT_TYPE (REMOVE_CV, "__remove_cv", 1)
>  DEFTRAIT_TYPE (REMOVE_REFERENCE, "__remove_reference", 1)
>  DEFTRAIT_TYPE (REMOVE_CVREF, "__remove_cvref", 1)
> +DEFTRAIT_TYPE (REMOVE_POINTER, "__remove_pointer", 1)
>  DEFTRAIT_TYPE (UNDERLYING_TYPE,  "__underlying_type", 1)
>  DEFTRAIT_TYPE (TYPE_PACK_ELEMENT, "__type_pack_element", -1)
>
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 8fb47fd179e..885c7a6fb64 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -12373,6 +12373,10 @@ finish_trait_type (cp_trait_kind kind, tree type1, tree type2,
>        if (TYPE_REF_P (type1))
>         type1 = TREE_TYPE (type1);
>        return cv_unqualified (type1);
> +    case CPTK_REMOVE_POINTER:
> +      if (TYPE_PTR_P (type1))
> +    type1 = TREE_TYPE (type1);
> +      return type1;
>
>      case CPTK_TYPE_PACK_ELEMENT:
>        return finish_type_pack_element (type1, type2, complain);
> diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> index f343e153e56..e21e0a95509 100644
> --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> @@ -146,3 +146,6 @@
>  #if !__has_builtin (__remove_cvref)
>  # error "__has_builtin (__remove_cvref) failed"
>  #endif
> +#if !__has_builtin (__remove_pointer)
> +# error "__has_builtin (__remove_pointer) failed"
> +#endif
> diff --git a/gcc/testsuite/g++.dg/ext/remove_pointer.C b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> new file mode 100644
> index 00000000000..7b13db93950
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> @@ -0,0 +1,51 @@
> +// { dg-do compile { target c++11 } }
> +
> +#define SA(X) static_assert((X),#X)
> +
> +SA(__is_same(__remove_pointer(int), int));
> +SA(__is_same(__remove_pointer(int*), int));
> +SA(__is_same(__remove_pointer(int**), int*));
> +
> +SA(__is_same(__remove_pointer(const int*), const int));
> +SA(__is_same(__remove_pointer(const int**), const int*));
> +SA(__is_same(__remove_pointer(int* const), int));
> +SA(__is_same(__remove_pointer(int** const), int*));
> +SA(__is_same(__remove_pointer(int* const* const), int* const));
> +
> +SA(__is_same(__remove_pointer(volatile int*), volatile int));
> +SA(__is_same(__remove_pointer(volatile int**), volatile int*));
> +SA(__is_same(__remove_pointer(int* volatile), int));
> +SA(__is_same(__remove_pointer(int** volatile), int*));
> +SA(__is_same(__remove_pointer(int* volatile* volatile), int* volatile));
> +
> +SA(__is_same(__remove_pointer(const volatile int*), const volatile int));
> +SA(__is_same(__remove_pointer(const volatile int**), const volatile int*));
> +SA(__is_same(__remove_pointer(const int* volatile), const int));
> +SA(__is_same(__remove_pointer(volatile int* const), volatile int));
> +SA(__is_same(__remove_pointer(int* const volatile), int));
> +SA(__is_same(__remove_pointer(const int** volatile), const int*));
> +SA(__is_same(__remove_pointer(volatile int** const), volatile int*));
> +SA(__is_same(__remove_pointer(int** const volatile), int*));
> +SA(__is_same(__remove_pointer(int* const* const volatile), int* const));
> +SA(__is_same(__remove_pointer(int* volatile* const volatile), int* volatile));
> +SA(__is_same(__remove_pointer(int* const volatile* const volatile), int* const volatile));
> +
> +SA(__is_same(__remove_pointer(int&), int&));
> +SA(__is_same(__remove_pointer(const int&), const int&));
> +SA(__is_same(__remove_pointer(volatile int&), volatile int&));
> +SA(__is_same(__remove_pointer(const volatile int&), const volatile int&));
> +
> +SA(__is_same(__remove_pointer(int&&), int&&));
> +SA(__is_same(__remove_pointer(const int&&), const int&&));
> +SA(__is_same(__remove_pointer(volatile int&&), volatile int&&));
> +SA(__is_same(__remove_pointer(const volatile int&&), const volatile int&&));
> +
> +SA(__is_same(__remove_pointer(int[3]), int[3]));
> +SA(__is_same(__remove_pointer(const int[3]), const int[3]));
> +SA(__is_same(__remove_pointer(volatile int[3]), volatile int[3]));
> +SA(__is_same(__remove_pointer(const volatile int[3]), const volatile int[3]));
> +
> +SA(__is_same(__remove_pointer(int(int)), int(int)));
> +SA(__is_same(__remove_pointer(int(*const)(int)), int(int)));
> +SA(__is_same(__remove_pointer(int(*volatile)(int)), int(int)));
> +SA(__is_same(__remove_pointer(int(*const volatile)(int)), int(int)));
> --
> 2.41.0
>
Ken Matsui June 20, 2023, 1:20 p.m. UTC | #2 
Just a quick update, the benchmark code link has been updated and can
now be accessed at
https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer.cc.
I have also created a report file which can be found at
https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer.md.

On Sat, Jun 17, 2023 at 5:35 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
>
> Hi,
>
> I conducted a benchmark for remove_pointer as well as is_object. Just
> like the is_object benchmark, here is the benchmark code:
>
> https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer_benchmark.cc
>
> On my computer, using the gcc HEAD of this patch for a release build,
> the patch with -DUSE_BUILTIN took 8.7% less time and used 4.3-4.9%
> less memory on average compared to not using it. Although the
> performance improvement was not as significant as with is_object, the
> benchmark demonstrated that the compilation was consistently more
> efficient.
>
> Sincerely,
> Ken Matsui
>
> On Thu, Jun 15, 2023 at 5:22 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
> >
> > This patch implements built-in trait for std::remove_pointer.
> >
> > gcc/cp/ChangeLog:
> >
> >         * cp-trait.def: Define __remove_pointer.
> >         * semantics.cc (finish_trait_type): Handle CPTK_REMOVE_POINTER.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * g++.dg/ext/has-builtin-1.C: Test existence of __remove_pointer.
> >         * g++.dg/ext/remove_pointer.C: New test.
> >
> > Signed-off-by: Ken Matsui <kmatsui@cs.washington.edu>
> > ---
> >  gcc/cp/cp-trait.def                       |  1 +
> >  gcc/cp/semantics.cc                       |  4 ++
> >  gcc/testsuite/g++.dg/ext/has-builtin-1.C  |  3 ++
> >  gcc/testsuite/g++.dg/ext/remove_pointer.C | 51 +++++++++++++++++++++++
> >  4 files changed, 59 insertions(+)
> >  create mode 100644 gcc/testsuite/g++.dg/ext/remove_pointer.C
> >
> > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def
> > index 8b7fece0cc8..07823e55579 100644
> > --- a/gcc/cp/cp-trait.def
> > +++ b/gcc/cp/cp-trait.def
> > @@ -90,6 +90,7 @@ DEFTRAIT_EXPR (IS_DEDUCIBLE, "__is_deducible ", 2)
> >  DEFTRAIT_TYPE (REMOVE_CV, "__remove_cv", 1)
> >  DEFTRAIT_TYPE (REMOVE_REFERENCE, "__remove_reference", 1)
> >  DEFTRAIT_TYPE (REMOVE_CVREF, "__remove_cvref", 1)
> > +DEFTRAIT_TYPE (REMOVE_POINTER, "__remove_pointer", 1)
> >  DEFTRAIT_TYPE (UNDERLYING_TYPE,  "__underlying_type", 1)
> >  DEFTRAIT_TYPE (TYPE_PACK_ELEMENT, "__type_pack_element", -1)
> >
> > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> > index 8fb47fd179e..885c7a6fb64 100644
> > --- a/gcc/cp/semantics.cc
> > +++ b/gcc/cp/semantics.cc
> > @@ -12373,6 +12373,10 @@ finish_trait_type (cp_trait_kind kind, tree type1, tree type2,
> >        if (TYPE_REF_P (type1))
> >         type1 = TREE_TYPE (type1);
> >        return cv_unqualified (type1);
> > +    case CPTK_REMOVE_POINTER:
> > +      if (TYPE_PTR_P (type1))
> > +    type1 = TREE_TYPE (type1);
> > +      return type1;
> >
> >      case CPTK_TYPE_PACK_ELEMENT:
> >        return finish_type_pack_element (type1, type2, complain);
> > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > index f343e153e56..e21e0a95509 100644
> > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > @@ -146,3 +146,6 @@
> >  #if !__has_builtin (__remove_cvref)
> >  # error "__has_builtin (__remove_cvref) failed"
> >  #endif
> > +#if !__has_builtin (__remove_pointer)
> > +# error "__has_builtin (__remove_pointer) failed"
> > +#endif
> > diff --git a/gcc/testsuite/g++.dg/ext/remove_pointer.C b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> > new file mode 100644
> > index 00000000000..7b13db93950
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> > @@ -0,0 +1,51 @@
> > +// { dg-do compile { target c++11 } }
> > +
> > +#define SA(X) static_assert((X),#X)
> > +
> > +SA(__is_same(__remove_pointer(int), int));
> > +SA(__is_same(__remove_pointer(int*), int));
> > +SA(__is_same(__remove_pointer(int**), int*));
> > +
> > +SA(__is_same(__remove_pointer(const int*), const int));
> > +SA(__is_same(__remove_pointer(const int**), const int*));
> > +SA(__is_same(__remove_pointer(int* const), int));
> > +SA(__is_same(__remove_pointer(int** const), int*));
> > +SA(__is_same(__remove_pointer(int* const* const), int* const));
> > +
> > +SA(__is_same(__remove_pointer(volatile int*), volatile int));
> > +SA(__is_same(__remove_pointer(volatile int**), volatile int*));
> > +SA(__is_same(__remove_pointer(int* volatile), int));
> > +SA(__is_same(__remove_pointer(int** volatile), int*));
> > +SA(__is_same(__remove_pointer(int* volatile* volatile), int* volatile));
> > +
> > +SA(__is_same(__remove_pointer(const volatile int*), const volatile int));
> > +SA(__is_same(__remove_pointer(const volatile int**), const volatile int*));
> > +SA(__is_same(__remove_pointer(const int* volatile), const int));
> > +SA(__is_same(__remove_pointer(volatile int* const), volatile int));
> > +SA(__is_same(__remove_pointer(int* const volatile), int));
> > +SA(__is_same(__remove_pointer(const int** volatile), const int*));
> > +SA(__is_same(__remove_pointer(volatile int** const), volatile int*));
> > +SA(__is_same(__remove_pointer(int** const volatile), int*));
> > +SA(__is_same(__remove_pointer(int* const* const volatile), int* const));
> > +SA(__is_same(__remove_pointer(int* volatile* const volatile), int* volatile));
> > +SA(__is_same(__remove_pointer(int* const volatile* const volatile), int* const volatile));
> > +
> > +SA(__is_same(__remove_pointer(int&), int&));
> > +SA(__is_same(__remove_pointer(const int&), const int&));
> > +SA(__is_same(__remove_pointer(volatile int&), volatile int&));
> > +SA(__is_same(__remove_pointer(const volatile int&), const volatile int&));
> > +
> > +SA(__is_same(__remove_pointer(int&&), int&&));
> > +SA(__is_same(__remove_pointer(const int&&), const int&&));
> > +SA(__is_same(__remove_pointer(volatile int&&), volatile int&&));
> > +SA(__is_same(__remove_pointer(const volatile int&&), const volatile int&&));
> > +
> > +SA(__is_same(__remove_pointer(int[3]), int[3]));
> > +SA(__is_same(__remove_pointer(const int[3]), const int[3]));
> > +SA(__is_same(__remove_pointer(volatile int[3]), volatile int[3]));
> > +SA(__is_same(__remove_pointer(const volatile int[3]), const volatile int[3]));
> > +
> > +SA(__is_same(__remove_pointer(int(int)), int(int)));
> > +SA(__is_same(__remove_pointer(int(*const)(int)), int(int)));
> > +SA(__is_same(__remove_pointer(int(*volatile)(int)), int(int)));
> > +SA(__is_same(__remove_pointer(int(*const volatile)(int)), int(int)));
> > --
> > 2.41.0
> >
Patrick Palka June 20, 2023, 3:21 p.m. UTC | #3 
On Sat, 17 Jun 2023, Ken Matsui via Gcc-patches wrote:

> Hi,
> 
> I conducted a benchmark for remove_pointer as well as is_object. Just
> like the is_object benchmark, here is the benchmark code:
> 
> https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer_benchmark.cc
> 
> On my computer, using the gcc HEAD of this patch for a release build,
> the patch with -DUSE_BUILTIN took 8.7% less time and used 4.3-4.9%
> less memory on average compared to not using it. Although the
> performance improvement was not as significant as with is_object, the
> benchmark demonstrated that the compilation was consistently more
> efficient.

Thanks for the benchmark.  The improvement is lesser than I expected,
but that might be because the benchmark is "biased":

  template <std::size_t N, std::size_t Count = 256>
  struct Instantiator : Instantiator<N, Count - 1> {
      static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator>>);
  };

This only invokes remove_pointer_t on the non-pointer type Instantiator,
and so the benchmark doesn't factor in the performance of the trait when
invoked on pointer types, and traits typically will have different
performance characteristics depending on the kind of type it's given.

To more holistically assess the real-world performance of the trait the
benchmark should also consider pointer types and maybe also cv-qualified
types (given that the original implementation is in terms of
__remove_cv_t and thus performance of the original implementation may be
sensitive to cv-qualification).  So we should probably uniformly
benchmark these classes of types, via doing e.g.:

  static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator>>);
  static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator*>>);
  static_assert(!std::is_pointer_v<remove_pointer_t<const Instantiator>>);
  static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator* const>>);

(We could consider other kinds of types too, e.g. reference types and
integral types, but it seems clear based on the implementations being
benchmarked that performance won't be sensitive to reference-ness
or integral-ness.)

> 
> Sincerely,
> Ken Matsui
> 
> On Thu, Jun 15, 2023 at 5:22 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
> >
> > This patch implements built-in trait for std::remove_pointer.
> >
> > gcc/cp/ChangeLog:
> >
> >         * cp-trait.def: Define __remove_pointer.
> >         * semantics.cc (finish_trait_type): Handle CPTK_REMOVE_POINTER.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * g++.dg/ext/has-builtin-1.C: Test existence of __remove_pointer.
> >         * g++.dg/ext/remove_pointer.C: New test.
> >
> > Signed-off-by: Ken Matsui <kmatsui@cs.washington.edu>
> > ---
> >  gcc/cp/cp-trait.def                       |  1 +
> >  gcc/cp/semantics.cc                       |  4 ++
> >  gcc/testsuite/g++.dg/ext/has-builtin-1.C  |  3 ++
> >  gcc/testsuite/g++.dg/ext/remove_pointer.C | 51 +++++++++++++++++++++++
> >  4 files changed, 59 insertions(+)
> >  create mode 100644 gcc/testsuite/g++.dg/ext/remove_pointer.C
> >
> > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def
> > index 8b7fece0cc8..07823e55579 100644
> > --- a/gcc/cp/cp-trait.def
> > +++ b/gcc/cp/cp-trait.def
> > @@ -90,6 +90,7 @@ DEFTRAIT_EXPR (IS_DEDUCIBLE, "__is_deducible ", 2)
> >  DEFTRAIT_TYPE (REMOVE_CV, "__remove_cv", 1)
> >  DEFTRAIT_TYPE (REMOVE_REFERENCE, "__remove_reference", 1)
> >  DEFTRAIT_TYPE (REMOVE_CVREF, "__remove_cvref", 1)
> > +DEFTRAIT_TYPE (REMOVE_POINTER, "__remove_pointer", 1)
> >  DEFTRAIT_TYPE (UNDERLYING_TYPE,  "__underlying_type", 1)
> >  DEFTRAIT_TYPE (TYPE_PACK_ELEMENT, "__type_pack_element", -1)
> >
> > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> > index 8fb47fd179e..885c7a6fb64 100644
> > --- a/gcc/cp/semantics.cc
> > +++ b/gcc/cp/semantics.cc
> > @@ -12373,6 +12373,10 @@ finish_trait_type (cp_trait_kind kind, tree type1, tree type2,
> >        if (TYPE_REF_P (type1))
> >         type1 = TREE_TYPE (type1);
> >        return cv_unqualified (type1);
> > +    case CPTK_REMOVE_POINTER:
> > +      if (TYPE_PTR_P (type1))
> > +    type1 = TREE_TYPE (type1);
> > +      return type1;

Maybe add a newline before the 'case' to visually separate it from the
previous 'case'?  LGTM otherwise, thanks!

> >
> >      case CPTK_TYPE_PACK_ELEMENT:
> >        return finish_type_pack_element (type1, type2, complain);
> > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > index f343e153e56..e21e0a95509 100644
> > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > @@ -146,3 +146,6 @@
> >  #if !__has_builtin (__remove_cvref)
> >  # error "__has_builtin (__remove_cvref) failed"
> >  #endif
> > +#if !__has_builtin (__remove_pointer)
> > +# error "__has_builtin (__remove_pointer) failed"
> > +#endif
> > diff --git a/gcc/testsuite/g++.dg/ext/remove_pointer.C b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> > new file mode 100644
> > index 00000000000..7b13db93950
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> > @@ -0,0 +1,51 @@
> > +// { dg-do compile { target c++11 } }
> > +
> > +#define SA(X) static_assert((X),#X)
> > +
> > +SA(__is_same(__remove_pointer(int), int));
> > +SA(__is_same(__remove_pointer(int*), int));
> > +SA(__is_same(__remove_pointer(int**), int*));
> > +
> > +SA(__is_same(__remove_pointer(const int*), const int));
> > +SA(__is_same(__remove_pointer(const int**), const int*));
> > +SA(__is_same(__remove_pointer(int* const), int));
> > +SA(__is_same(__remove_pointer(int** const), int*));
> > +SA(__is_same(__remove_pointer(int* const* const), int* const));
> > +
> > +SA(__is_same(__remove_pointer(volatile int*), volatile int));
> > +SA(__is_same(__remove_pointer(volatile int**), volatile int*));
> > +SA(__is_same(__remove_pointer(int* volatile), int));
> > +SA(__is_same(__remove_pointer(int** volatile), int*));
> > +SA(__is_same(__remove_pointer(int* volatile* volatile), int* volatile));
> > +
> > +SA(__is_same(__remove_pointer(const volatile int*), const volatile int));
> > +SA(__is_same(__remove_pointer(const volatile int**), const volatile int*));
> > +SA(__is_same(__remove_pointer(const int* volatile), const int));
> > +SA(__is_same(__remove_pointer(volatile int* const), volatile int));
> > +SA(__is_same(__remove_pointer(int* const volatile), int));
> > +SA(__is_same(__remove_pointer(const int** volatile), const int*));
> > +SA(__is_same(__remove_pointer(volatile int** const), volatile int*));
> > +SA(__is_same(__remove_pointer(int** const volatile), int*));
> > +SA(__is_same(__remove_pointer(int* const* const volatile), int* const));
> > +SA(__is_same(__remove_pointer(int* volatile* const volatile), int* volatile));
> > +SA(__is_same(__remove_pointer(int* const volatile* const volatile), int* const volatile));
> > +
> > +SA(__is_same(__remove_pointer(int&), int&));
> > +SA(__is_same(__remove_pointer(const int&), const int&));
> > +SA(__is_same(__remove_pointer(volatile int&), volatile int&));
> > +SA(__is_same(__remove_pointer(const volatile int&), const volatile int&));
> > +
> > +SA(__is_same(__remove_pointer(int&&), int&&));
> > +SA(__is_same(__remove_pointer(const int&&), const int&&));
> > +SA(__is_same(__remove_pointer(volatile int&&), volatile int&&));
> > +SA(__is_same(__remove_pointer(const volatile int&&), const volatile int&&));
> > +
> > +SA(__is_same(__remove_pointer(int[3]), int[3]));
> > +SA(__is_same(__remove_pointer(const int[3]), const int[3]));
> > +SA(__is_same(__remove_pointer(volatile int[3]), volatile int[3]));
> > +SA(__is_same(__remove_pointer(const volatile int[3]), const volatile int[3]));
> > +
> > +SA(__is_same(__remove_pointer(int(int)), int(int)));
> > +SA(__is_same(__remove_pointer(int(*const)(int)), int(int)));
> > +SA(__is_same(__remove_pointer(int(*volatile)(int)), int(int)));
> > +SA(__is_same(__remove_pointer(int(*const volatile)(int)), int(int)));
> > --
> > 2.41.0
> >
> 
>
Ken Matsui June 24, 2023, 10:07 a.m. UTC | #4 
On Tue, Jun 20, 2023 at 8:22 AM Patrick Palka <ppalka@redhat.com> wrote:
>
> On Sat, 17 Jun 2023, Ken Matsui via Gcc-patches wrote:
>
> > Hi,
> >
> > I conducted a benchmark for remove_pointer as well as is_object. Just
> > like the is_object benchmark, here is the benchmark code:
> >
> > https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer_benchmark.cc
> >
> > On my computer, using the gcc HEAD of this patch for a release build,
> > the patch with -DUSE_BUILTIN took 8.7% less time and used 4.3-4.9%
> > less memory on average compared to not using it. Although the
> > performance improvement was not as significant as with is_object, the
> > benchmark demonstrated that the compilation was consistently more
> > efficient.
>
> Thanks for the benchmark.  The improvement is lesser than I expected,
> but that might be because the benchmark is "biased":
>
>   template <std::size_t N, std::size_t Count = 256>
>   struct Instantiator : Instantiator<N, Count - 1> {
>       static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator>>);
>   };
>
> This only invokes remove_pointer_t on the non-pointer type Instantiator,
> and so the benchmark doesn't factor in the performance of the trait when
> invoked on pointer types, and traits typically will have different
> performance characteristics depending on the kind of type it's given.
>
> To more holistically assess the real-world performance of the trait the
> benchmark should also consider pointer types and maybe also cv-qualified
> types (given that the original implementation is in terms of
> __remove_cv_t and thus performance of the original implementation may be
> sensitive to cv-qualification).  So we should probably uniformly
> benchmark these classes of types, via doing e.g.:
>
>   static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator>>);
>   static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator*>>);
>   static_assert(!std::is_pointer_v<remove_pointer_t<const Instantiator>>);
>   static_assert(!std::is_pointer_v<remove_pointer_t<Instantiator* const>>);
>
> (We could consider other kinds of types too, e.g. reference types and
> integral types, but it seems clear based on the implementations being
> benchmarked that performance won't be sensitive to reference-ness
> or integral-ness.)

Thank you for your review! I totally agree with your opinion that the
benchmark should have been exhaustive. I conducted a benchmark for
this new change:

https://github.com/ken-matsui/gcc-benches/blob/main/remove_pointer.md#tue-jun-20-030313-pm-pdt-2023

Time: -27.918%
Peak Memory Usage: -19.0755%
Total Memory Usage: -20.0199%

> >
> > Sincerely,
> > Ken Matsui
> >
> > On Thu, Jun 15, 2023 at 5:22 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
> > >
> > > This patch implements built-in trait for std::remove_pointer.
> > >
> > > gcc/cp/ChangeLog:
> > >
> > >         * cp-trait.def: Define __remove_pointer.
> > >         * semantics.cc (finish_trait_type): Handle CPTK_REMOVE_POINTER.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * g++.dg/ext/has-builtin-1.C: Test existence of __remove_pointer.
> > >         * g++.dg/ext/remove_pointer.C: New test.
> > >
> > > Signed-off-by: Ken Matsui <kmatsui@cs.washington.edu>
> > > ---
> > >  gcc/cp/cp-trait.def                       |  1 +
> > >  gcc/cp/semantics.cc                       |  4 ++
> > >  gcc/testsuite/g++.dg/ext/has-builtin-1.C  |  3 ++
> > >  gcc/testsuite/g++.dg/ext/remove_pointer.C | 51 +++++++++++++++++++++++
> > >  4 files changed, 59 insertions(+)
> > >  create mode 100644 gcc/testsuite/g++.dg/ext/remove_pointer.C
> > >
> > > diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def
> > > index 8b7fece0cc8..07823e55579 100644
> > > --- a/gcc/cp/cp-trait.def
> > > +++ b/gcc/cp/cp-trait.def
> > > @@ -90,6 +90,7 @@ DEFTRAIT_EXPR (IS_DEDUCIBLE, "__is_deducible ", 2)
> > >  DEFTRAIT_TYPE (REMOVE_CV, "__remove_cv", 1)
> > >  DEFTRAIT_TYPE (REMOVE_REFERENCE, "__remove_reference", 1)
> > >  DEFTRAIT_TYPE (REMOVE_CVREF, "__remove_cvref", 1)
> > > +DEFTRAIT_TYPE (REMOVE_POINTER, "__remove_pointer", 1)
> > >  DEFTRAIT_TYPE (UNDERLYING_TYPE,  "__underlying_type", 1)
> > >  DEFTRAIT_TYPE (TYPE_PACK_ELEMENT, "__type_pack_element", -1)
> > >
> > > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> > > index 8fb47fd179e..885c7a6fb64 100644
> > > --- a/gcc/cp/semantics.cc
> > > +++ b/gcc/cp/semantics.cc
> > > @@ -12373,6 +12373,10 @@ finish_trait_type (cp_trait_kind kind, tree type1, tree type2,
> > >        if (TYPE_REF_P (type1))
> > >         type1 = TREE_TYPE (type1);
> > >        return cv_unqualified (type1);
> > > +    case CPTK_REMOVE_POINTER:
> > > +      if (TYPE_PTR_P (type1))
> > > +    type1 = TREE_TYPE (type1);
> > > +      return type1;
>
> Maybe add a newline before the 'case' to visually separate it from the
> previous 'case'?  LGTM otherwise, thanks!

Will do! Thank you!

> > >
> > >      case CPTK_TYPE_PACK_ELEMENT:
> > >        return finish_type_pack_element (type1, type2, complain);
> > > diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > > index f343e153e56..e21e0a95509 100644
> > > --- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > > +++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
> > > @@ -146,3 +146,6 @@
> > >  #if !__has_builtin (__remove_cvref)
> > >  # error "__has_builtin (__remove_cvref) failed"
> > >  #endif
> > > +#if !__has_builtin (__remove_pointer)
> > > +# error "__has_builtin (__remove_pointer) failed"
> > > +#endif
> > > diff --git a/gcc/testsuite/g++.dg/ext/remove_pointer.C b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> > > new file mode 100644
> > > index 00000000000..7b13db93950
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/ext/remove_pointer.C
> > > @@ -0,0 +1,51 @@
> > > +// { dg-do compile { target c++11 } }
> > > +
> > > +#define SA(X) static_assert((X),#X)
> > > +
> > > +SA(__is_same(__remove_pointer(int), int));
> > > +SA(__is_same(__remove_pointer(int*), int));
> > > +SA(__is_same(__remove_pointer(int**), int*));
> > > +
> > > +SA(__is_same(__remove_pointer(const int*), const int));
> > > +SA(__is_same(__remove_pointer(const int**), const int*));
> > > +SA(__is_same(__remove_pointer(int* const), int));
> > > +SA(__is_same(__remove_pointer(int** const), int*));
> > > +SA(__is_same(__remove_pointer(int* const* const), int* const));
> > > +
> > > +SA(__is_same(__remove_pointer(volatile int*), volatile int));
> > > +SA(__is_same(__remove_pointer(volatile int**), volatile int*));
> > > +SA(__is_same(__remove_pointer(int* volatile), int));
> > > +SA(__is_same(__remove_pointer(int** volatile), int*));
> > > +SA(__is_same(__remove_pointer(int* volatile* volatile), int* volatile));
> > > +
> > > +SA(__is_same(__remove_pointer(const volatile int*), const volatile int));
> > > +SA(__is_same(__remove_pointer(const volatile int**), const volatile int*));
> > > +SA(__is_same(__remove_pointer(const int* volatile), const int));
> > > +SA(__is_same(__remove_pointer(volatile int* const), volatile int));
> > > +SA(__is_same(__remove_pointer(int* const volatile), int));
> > > +SA(__is_same(__remove_pointer(const int** volatile), const int*));
> > > +SA(__is_same(__remove_pointer(volatile int** const), volatile int*));
> > > +SA(__is_same(__remove_pointer(int** const volatile), int*));
> > > +SA(__is_same(__remove_pointer(int* const* const volatile), int* const));
> > > +SA(__is_same(__remove_pointer(int* volatile* const volatile), int* volatile));
> > > +SA(__is_same(__remove_pointer(int* const volatile* const volatile), int* const volatile));
> > > +
> > > +SA(__is_same(__remove_pointer(int&), int&));
> > > +SA(__is_same(__remove_pointer(const int&), const int&));
> > > +SA(__is_same(__remove_pointer(volatile int&), volatile int&));
> > > +SA(__is_same(__remove_pointer(const volatile int&), const volatile int&));
> > > +
> > > +SA(__is_same(__remove_pointer(int&&), int&&));
> > > +SA(__is_same(__remove_pointer(const int&&), const int&&));
> > > +SA(__is_same(__remove_pointer(volatile int&&), volatile int&&));
> > > +SA(__is_same(__remove_pointer(const volatile int&&), const volatile int&&));
> > > +
> > > +SA(__is_same(__remove_pointer(int[3]), int[3]));
> > > +SA(__is_same(__remove_pointer(const int[3]), const int[3]));
> > > +SA(__is_same(__remove_pointer(volatile int[3]), volatile int[3]));
> > > +SA(__is_same(__remove_pointer(const volatile int[3]), const volatile int[3]));
> > > +
> > > +SA(__is_same(__remove_pointer(int(int)), int(int)));
> > > +SA(__is_same(__remove_pointer(int(*const)(int)), int(int)));
> > > +SA(__is_same(__remove_pointer(int(*volatile)(int)), int(int)));
> > > +SA(__is_same(__remove_pointer(int(*const volatile)(int)), int(int)));
> > > --
> > > 2.41.0
> > >
> >
> >
diff mbox series

Patch

diff --git a/gcc/cp/cp-trait.def b/gcc/cp/cp-trait.def
index 8b7fece0cc8..07823e55579 100644
--- a/gcc/cp/cp-trait.def
+++ b/gcc/cp/cp-trait.def
@@ -90,6 +90,7 @@  DEFTRAIT_EXPR (IS_DEDUCIBLE, "__is_deducible ", 2)
 DEFTRAIT_TYPE (REMOVE_CV, "__remove_cv", 1)
 DEFTRAIT_TYPE (REMOVE_REFERENCE, "__remove_reference", 1)
 DEFTRAIT_TYPE (REMOVE_CVREF, "__remove_cvref", 1)
+DEFTRAIT_TYPE (REMOVE_POINTER, "__remove_pointer", 1)
 DEFTRAIT_TYPE (UNDERLYING_TYPE,  "__underlying_type", 1)
 DEFTRAIT_TYPE (TYPE_PACK_ELEMENT, "__type_pack_element", -1)
 
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 8fb47fd179e..885c7a6fb64 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -12373,6 +12373,10 @@  finish_trait_type (cp_trait_kind kind, tree type1, tree type2,
       if (TYPE_REF_P (type1))
 	type1 = TREE_TYPE (type1);
       return cv_unqualified (type1);
+    case CPTK_REMOVE_POINTER:
+      if (TYPE_PTR_P (type1))
+    type1 = TREE_TYPE (type1);
+      return type1;
 
     case CPTK_TYPE_PACK_ELEMENT:
       return finish_type_pack_element (type1, type2, complain);
diff --git a/gcc/testsuite/g++.dg/ext/has-builtin-1.C b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
index f343e153e56..e21e0a95509 100644
--- a/gcc/testsuite/g++.dg/ext/has-builtin-1.C
+++ b/gcc/testsuite/g++.dg/ext/has-builtin-1.C
@@ -146,3 +146,6 @@ 
 #if !__has_builtin (__remove_cvref)
 # error "__has_builtin (__remove_cvref) failed"
 #endif
+#if !__has_builtin (__remove_pointer)
+# error "__has_builtin (__remove_pointer) failed"
+#endif
diff --git a/gcc/testsuite/g++.dg/ext/remove_pointer.C b/gcc/testsuite/g++.dg/ext/remove_pointer.C
new file mode 100644
index 00000000000..7b13db93950
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/remove_pointer.C
@@ -0,0 +1,51 @@ 
+// { dg-do compile { target c++11 } }
+
+#define SA(X) static_assert((X),#X)
+
+SA(__is_same(__remove_pointer(int), int));
+SA(__is_same(__remove_pointer(int*), int));
+SA(__is_same(__remove_pointer(int**), int*));
+
+SA(__is_same(__remove_pointer(const int*), const int));
+SA(__is_same(__remove_pointer(const int**), const int*));
+SA(__is_same(__remove_pointer(int* const), int));
+SA(__is_same(__remove_pointer(int** const), int*));
+SA(__is_same(__remove_pointer(int* const* const), int* const));
+
+SA(__is_same(__remove_pointer(volatile int*), volatile int));
+SA(__is_same(__remove_pointer(volatile int**), volatile int*));
+SA(__is_same(__remove_pointer(int* volatile), int));
+SA(__is_same(__remove_pointer(int** volatile), int*));
+SA(__is_same(__remove_pointer(int* volatile* volatile), int* volatile));
+
+SA(__is_same(__remove_pointer(const volatile int*), const volatile int));
+SA(__is_same(__remove_pointer(const volatile int**), const volatile int*));
+SA(__is_same(__remove_pointer(const int* volatile), const int));
+SA(__is_same(__remove_pointer(volatile int* const), volatile int));
+SA(__is_same(__remove_pointer(int* const volatile), int));
+SA(__is_same(__remove_pointer(const int** volatile), const int*));
+SA(__is_same(__remove_pointer(volatile int** const), volatile int*));
+SA(__is_same(__remove_pointer(int** const volatile), int*));
+SA(__is_same(__remove_pointer(int* const* const volatile), int* const));
+SA(__is_same(__remove_pointer(int* volatile* const volatile), int* volatile));
+SA(__is_same(__remove_pointer(int* const volatile* const volatile), int* const volatile));
+
+SA(__is_same(__remove_pointer(int&), int&));
+SA(__is_same(__remove_pointer(const int&), const int&));
+SA(__is_same(__remove_pointer(volatile int&), volatile int&));
+SA(__is_same(__remove_pointer(const volatile int&), const volatile int&));
+
+SA(__is_same(__remove_pointer(int&&), int&&));
+SA(__is_same(__remove_pointer(const int&&), const int&&));
+SA(__is_same(__remove_pointer(volatile int&&), volatile int&&));
+SA(__is_same(__remove_pointer(const volatile int&&), const volatile int&&));
+
+SA(__is_same(__remove_pointer(int[3]), int[3]));
+SA(__is_same(__remove_pointer(const int[3]), const int[3]));
+SA(__is_same(__remove_pointer(volatile int[3]), volatile int[3]));
+SA(__is_same(__remove_pointer(const volatile int[3]), const volatile int[3]));
+
+SA(__is_same(__remove_pointer(int(int)), int(int)));
+SA(__is_same(__remove_pointer(int(*const)(int)), int(int)));
+SA(__is_same(__remove_pointer(int(*volatile)(int)), int(int)));
+SA(__is_same(__remove_pointer(int(*const volatile)(int)), int(int)));