[2/5] range: Make range_op_table a true singleton class [PR116209]

  This is a small cleanup with respect to the ranger_op_table class.
There should only ever be one instance of ranger_op_table so
this adds a static member function which returns the instance.
A few variables that are defined in range-op.cc should be local
to the file so wrap them with an anonymous namespace.
Also change operator_table into a reference that is initialized to
the singelton.

This has a small extra overhead at intiialization time of the operator_table;
could be improved if we used C++20's consteval. Since this happens only once,
there it should be ok.

Bootstrapped and tested on x86_64-linux-gnu.

	PR tree-optimization/116209
gcc/ChangeLog:

	* range-op.cc (op_equal, op_not_equal, op_lt, op_le, op_gt, op_ge,
	op_ident, op_cst, op_cast, op_plus, op_abs, op_minus, op_negate,
	op_mult, op_addr, op_bitwise_not, op_bitwise_xor, op_bitwise_and,
	op_bitwise_or, op_min, op_max, default_operator): Wrap with anonymous namespace.
	(operator_table): Change to reference and initialize with range_op_table::singleton.
	(range_op_table::singleton): New function.
	* range-op.h (range_op_table): New method, singleton.
	Make most functions private (rather than protected).
	Make ctor private.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/range-op.cc | 19 ++++++++++++++-----
 gcc/range-op.h  |  5 +++--
 2 files changed, 17 insertions(+), 7 deletions(-)
  

On Tue, Aug 6, 2024 at 11:29 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> This is a small cleanup with respect to the ranger_op_table class.
> There should only ever be one instance of ranger_op_table so
> this adds a static member function which returns the instance.
> A few variables that are defined in range-op.cc should be local
> to the file so wrap them with an anonymous namespace.
> Also change operator_table into a reference that is initialized to
> the singelton.
>
> This has a small extra overhead at intiialization time of the operator_table;
> could be improved if we used C++20's consteval. Since this happens only once,
> there it should be ok.

Can you make it so with appropriate #if __cplusplus or __has_feature (consteval)
(or how that's done)?

>
> Bootstrapped and tested on x86_64-linux-gnu.
>
>         PR tree-optimization/116209
> gcc/ChangeLog:
>
>         * range-op.cc (op_equal, op_not_equal, op_lt, op_le, op_gt, op_ge,
>         op_ident, op_cst, op_cast, op_plus, op_abs, op_minus, op_negate,
>         op_mult, op_addr, op_bitwise_not, op_bitwise_xor, op_bitwise_and,
>         op_bitwise_or, op_min, op_max, default_operator): Wrap with anonymous namespace.
>         (operator_table): Change to reference and initialize with range_op_table::singleton.
>         (range_op_table::singleton): New function.
>         * range-op.h (range_op_table): New method, singleton.
>         Make most functions private (rather than protected).
>         Make ctor private.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/range-op.cc | 19 ++++++++++++++-----
>  gcc/range-op.h  |  5 +++--
>  2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> index c576f688221..56a014e99bc 100644
> --- a/gcc/range-op.cc
> +++ b/gcc/range-op.cc
> @@ -49,8 +49,9 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-ssa-ccp.h"
>  #include "range-op-mixed.h"
>
> -// Instantiate the operators which apply to multiple types here.
> +namespace {
>
> +// Instantiate the operators which apply to multiple types here.
>  operator_equal op_equal;
>  operator_not_equal op_not_equal;
>  operator_lt op_lt;
> @@ -74,7 +75,12 @@ operator_min op_min;
>  operator_max op_max;
>
>  // Instantaite a range operator table.
> -range_op_table operator_table;
> +range_op_table &operator_table = range_op_table::singleton();
> +
> +// Instantiate a default range operator for opcodes with no entry.
> +range_operator default_operator;
> +
> +}
>
>  // Invoke the initialization routines for each class of range.
>
> @@ -111,9 +117,12 @@ range_op_table::range_op_table ()
>    set (MAX_EXPR, op_max);
>  }
>
> -// Instantiate a default range operator for opcodes with no entry.
> -
> -range_operator default_operator;
> +// Returns the singleton instance of the table.
> +range_op_table &range_op_table::singleton()
> +{
> +  static range_op_table single;
> +  return single;
> +}
>
>  // Create a default range_op_handler.
>
> diff --git a/gcc/range-op.h b/gcc/range-op.h
> index 8edf967a445..e4e11f89624 100644
> --- a/gcc/range-op.h
> +++ b/gcc/range-op.h
> @@ -391,13 +391,13 @@ extern void wi_set_zero_nonzero_bits (tree type,
>  class range_op_table final
>  {
>  public:
> -  range_op_table ();
>    inline range_operator *operator[] (unsigned code)
>      {
>        gcc_checking_assert (code < RANGE_OP_TABLE_SIZE);
>        return m_range_tree[code];
>      }
> -protected:
> +  static range_op_table &singleton();
> +private:
>    inline void set (unsigned code, range_operator &op)
>      {
>        gcc_checking_assert (code < RANGE_OP_TABLE_SIZE);
> @@ -408,6 +408,7 @@ protected:
>    void initialize_integral_ops ();
>    void initialize_pointer_ops ();
>    void initialize_float_ops ();
> +  range_op_table ();
>  };
>
>  #endif // GCC_RANGE_OP_H
> --
> 2.43.0
>
  

On Wed, Aug 07, 2024 at 09:40:06AM +0200, Richard Biener wrote:
> On Tue, Aug 6, 2024 at 11:29 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
> >
> > This is a small cleanup with respect to the ranger_op_table class.
> > There should only ever be one instance of ranger_op_table so
> > this adds a static member function which returns the instance.
> > A few variables that are defined in range-op.cc should be local
> > to the file so wrap them with an anonymous namespace.
> > Also change operator_table into a reference that is initialized to
> > the singelton.
> >
> > This has a small extra overhead at intiialization time of the operator_table;
> > could be improved if we used C++20's consteval. Since this happens only once,
> > there it should be ok.
> 
> Can you make it so with appropriate #if __cplusplus or __has_feature (consteval)
> (or how that's done)?

That would be
#if __cpp_consteval >= 201811L
unless you need the P2564R3 paper behavior (then it would be
#if __cpp_consteval >= 202211L
).

	Jakub
  

On Wed, Aug 7, 2024 at 9:42 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Wed, Aug 07, 2024 at 09:40:06AM +0200, Richard Biener wrote:
> > On Tue, Aug 6, 2024 at 11:29 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
> > >
> > > This is a small cleanup with respect to the ranger_op_table class.
> > > There should only ever be one instance of ranger_op_table so
> > > this adds a static member function which returns the instance.
> > > A few variables that are defined in range-op.cc should be local
> > > to the file so wrap them with an anonymous namespace.
> > > Also change operator_table into a reference that is initialized to
> > > the singelton.
> > >
> > > This has a small extra overhead at intiialization time of the operator_table;
> > > could be improved if we used C++20's consteval. Since this happens only once,
> > > there it should be ok.
> >
> > Can you make it so with appropriate #if __cplusplus or __has_feature (consteval)
> > (or how that's done)?
>
> That would be
> #if __cpp_consteval >= 201811L
> unless you need the P2564R3 paper behavior (then it would be
> #if __cpp_consteval >= 202211L
> ).

Thanks - IMO it's worth optimizing static initialization.

Richard.

>
>         Jakub
>