| Message ID | ZrITwz3hu3tSEejU@fkdesktop.suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | gimple ssa: Put SCCOPY logic into a class | expand |
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 <fkastl@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; > } > >
On Tue 2024-08-06 15:14:32, Richard Biener wrote: > 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. > Thanks, pushed. Filip
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; }
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? 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 <fkastl@suse.cz> --- gcc/gimple-ssa-sccopy.cc | 66 ++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 29 deletions(-)