On Tue, 6 Aug 2024, Filip Kastl wrote:

> Hello everybody,
> 
> In pr113054[1] Andrew said that he doesn't like the 'dead_stmts' static
> variable I used when implementing the sccopy pass.  We agreed that wrapping
> the relevant code from the pass in a class would be most likely the best
> solution.  Here is a patch that does exactly that.  I waited until stage 1 to
> submit it.
> 
> Bootstrapped and regtested on x86_64.  Is the patch ok to be pushed to trunk?

OK.

Richard.

> Cheers,
> Filip Kastl
> 
> 
> [1]
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113054
> 
> 
> -- 8< --
> 
> 
> Currently the main logic of the sccopy pass is implemented as static
> functions.  This patch instead puts the code into a class.  This also
> gets rid of a global variable (dead_stmts).
> 
> gcc/ChangeLog:
> 
>       * gimple-ssa-sccopy.cc (class scc_copy_prop): New class.
>       (replace_scc_by_value): Put into...
>       (scc_copy_prop::replace_scc_by_value): ...scc_copy_prop.
>       (sccopy_visit_op): Put into...
>       (scc_copy_prop::visit_op): ...scc_copy_prop.
>       (sccopy_propagate): Put into...
>       (scc_copy_prop::propagate): ...scc_copy_prop.
>       (init_sccopy): Replace by...
>       (scc_copy_prop::scc_copy_prop): ...the construtor.
>       (finalize_sccopy): Replace by...
>       (scc_copy_prop::~scc_copy_prop): ...the destructor.
>       (pass_sccopy::execute): Use scc_copy_prop.
> 
> Signed-off-by: Filip Kastl <fka...@suse.cz>
> ---
>  gcc/gimple-ssa-sccopy.cc | 66 ++++++++++++++++++++++------------------
>  1 file changed, 37 insertions(+), 29 deletions(-)
> 
> diff --git a/gcc/gimple-ssa-sccopy.cc b/gcc/gimple-ssa-sccopy.cc
> index 191a4c0b451..d9eaeab4abb 100644
> --- a/gcc/gimple-ssa-sccopy.cc
> +++ b/gcc/gimple-ssa-sccopy.cc
> @@ -94,11 +94,6 @@ along with GCC; see the file COPYING3.  If not see
>  
>  namespace {
>  
> -/* Bitmap tracking statements which were propagated to be removed at the end 
> of
> -   the pass.  */
> -
> -static bitmap dead_stmts;
> -
>  /* State of vertex during SCC discovery.
>  
>     unvisited  Vertex hasn't yet been popped from worklist.
> @@ -459,11 +454,33 @@ get_all_stmt_may_generate_copy (void)
>    return result;
>  }
>  
> +/* SCC copy propagation
> +
> +   'scc_copy_prop::propagate ()' is the main function of this pass.  */
> +
> +class scc_copy_prop
> +{
> +public:
> +  scc_copy_prop ();
> +  ~scc_copy_prop ();
> +  void propagate ();
> +
> +private:
> +  /* Bitmap tracking statements which were propagated so that they can be
> +     removed at the end of the pass.  */
> +  bitmap dead_stmts;
> +
> +  void visit_op (tree op, hash_set<tree> &outer_ops,
> +                             hash_set<gimple *> &scc_set, bool &is_inner,
> +                             tree &last_outer_op);
> +  void replace_scc_by_value (vec<gimple *> scc, tree val);
> +};
> +
>  /* For each statement from given SCC, replace its usages by value
>     VAL.  */
>  
> -static void
> -replace_scc_by_value (vec<gimple *> scc, tree val)
> +void
> +scc_copy_prop::replace_scc_by_value (vec<gimple *> scc, tree val)
>  {
>    for (gimple *stmt : scc)
>      {
> @@ -476,12 +493,12 @@ replace_scc_by_value (vec<gimple *> scc, tree val)
>      fprintf (dump_file, "Replacing SCC of size %d\n", scc.length ());
>  }
>  
> -/* Part of 'sccopy_propagate ()'.  */
> +/* Part of 'scc_copy_prop::propagate ()'.  */
>  
> -static void
> -sccopy_visit_op (tree op, hash_set<tree> &outer_ops,
> -              hash_set<gimple *> &scc_set, bool &is_inner,
> -              tree &last_outer_op)
> +void
> +scc_copy_prop::visit_op (tree op, hash_set<tree> &outer_ops,
> +                      hash_set<gimple *> &scc_set, bool &is_inner,
> +                      tree &last_outer_op)
>  {
>    bool op_in_scc = false;
>  
> @@ -539,8 +556,8 @@ sccopy_visit_op (tree op, hash_set<tree> &outer_ops,
>       Braun, Buchwald, Hack, Leissa, Mallon, Zwinkau, 2013, LNCS vol. 7791,
>       Section 3.2.  */
>  
> -static void
> -sccopy_propagate ()
> +void
> +scc_copy_prop::propagate ()
>  {
>    auto_vec<gimple *> useful_stmts = get_all_stmt_may_generate_copy ();
>    scc_discovery discovery;
> @@ -575,14 +592,12 @@ sccopy_propagate ()
>               for (j = 0; j < gimple_phi_num_args (phi); j++)
>                 {
>                   op = gimple_phi_arg_def (phi, j);
> -                 sccopy_visit_op (op, outer_ops, scc_set, is_inner,
> -                                last_outer_op);
> +                 visit_op (op, outer_ops, scc_set, is_inner, last_outer_op);
>                 }
>               break;
>             case GIMPLE_ASSIGN:
>               op = gimple_assign_rhs1 (stmt);
> -             sccopy_visit_op (op, outer_ops, scc_set, is_inner,
> -                            last_outer_op);
> +             visit_op (op, outer_ops, scc_set, is_inner, last_outer_op);
>               break;
>             default:
>               gcc_unreachable ();
> @@ -613,19 +628,13 @@ sccopy_propagate ()
>      }
>  }
>  
> -/* Called when pass execution starts.  */
> -
> -static void
> -init_sccopy (void)
> +scc_copy_prop::scc_copy_prop ()
>  {
>    /* For propagated statements.  */
>    dead_stmts = BITMAP_ALLOC (NULL);
>  }
>  
> -/* Called before pass execution ends.  */
> -
> -static void
> -finalize_sccopy (void)
> +scc_copy_prop::~scc_copy_prop ()
>  {
>    /* Remove all propagated statements.  */
>    simple_dce_from_worklist (dead_stmts);
> @@ -668,9 +677,8 @@ public:
>  unsigned
>  pass_sccopy::execute (function *)
>  {
> -  init_sccopy ();
> -  sccopy_propagate ();
> -  finalize_sccopy ();
> +  scc_copy_prop sccopy;
> +  sccopy.propagate ();
>    return 0;
>  }
>  
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to