| Message ID | orh7odfqhi.fsf_-_@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | make FOR_EACH_IMM_USE_STMT safe for early exits (was: Re: move sincos after pre) | expand |
On Tue, Dec 22, 2020 at 11:03 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Oct 28, 2020, Richard Biener <richard.guenther@gmail.com> wrote: > > >> BTW, any reason why we are not (yet?) using something like: > >> > >> #define FOR_EACH_IMM_USE_STMT(STMT, ITER, SSAVAR) \ > >> for (auto_end_imm_use_stmt_traverse auto_end \ > >> ((((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR))), \ > >> &(ITER))); \ > >> !end_imm_use_stmt_p (&(ITER)); \ > >> (void) ((STMT) = next_imm_use_stmt (&(ITER)))) > > > Just laziness. Or rather last time I remembered this I tried to do it > > more fancy via range-for but didn't get very far due to the nested > > iteration ... > > Use a dtor to automatically remove ITER from IMM_USE list in > FOR_EACH_IMM_USE_STMT. > > Regstrapped on x86_64-linux-gnu. Ok to install? Hmm - while the change looks good, doesn't it end up calling end_imm_use_stmt_tranverse twice for those uses still calling BREAK_FROM_IMM_USE_STMT? Thus, please remove uses of BREAK_FROM_IMM_USE_STMT together with this patch. Thanks, Richard. > > for gcc/ChangeLog > > * ssa-iterators.h (end_imm_use_stmt_traverse): Forward > declare. > (auto_end_imm_use_stmt_traverse): New struct. > (FOR_EACH_IMM_USE_STMT): Use it. > --- > gcc/ssa-iterators.h | 29 ++++++++++++++++++++++++----- > 1 file changed, 24 insertions(+), 5 deletions(-) > > diff --git a/gcc/ssa-iterators.h b/gcc/ssa-iterators.h > index 98724da14522d..817997aa952d4 100644 > --- a/gcc/ssa-iterators.h > +++ b/gcc/ssa-iterators.h > @@ -77,16 +77,35 @@ struct imm_use_iterator > !end_readonly_imm_use_p (&(ITER)); \ > (void) ((DEST) = next_readonly_imm_use (&(ITER)))) > > -/* Use this iterator to visit each stmt which has a use of SSAVAR. */ > +/* Forward declare for use in the class below. */ > +static inline void end_imm_use_stmt_traverse (imm_use_iterator *); > + > +/* arrange to automatically call, upon descruction, end_imm_use_stmt_traverse > + with a given pointer to imm_use_iterator. */ > +struct auto_end_imm_use_stmt_traverse > +{ > + imm_use_iterator *imm; > + auto_end_imm_use_stmt_traverse (imm_use_iterator *imm) > + : imm (imm) {} > + ~auto_end_imm_use_stmt_traverse () > + { end_imm_use_stmt_traverse (imm); } > +}; > + > +/* Use this iterator to visit each stmt which has a use of SSAVAR. The > + destructor of the auto_end_imm_use_stmt_traverse object deals with removing > + ITER from SSAVAR's IMM_USE list even when leaving the scope early. */ > > #define FOR_EACH_IMM_USE_STMT(STMT, ITER, SSAVAR) \ > - for ((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR)); \ > + for (struct auto_end_imm_use_stmt_traverse \ > + auto_end_imm_use_stmt_traverse \ > + ((((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR))), \ > + &(ITER))); \ > !end_imm_use_stmt_p (&(ITER)); \ > (void) ((STMT) = next_imm_use_stmt (&(ITER)))) > > -/* Use this to terminate the FOR_EACH_IMM_USE_STMT loop early. Failure to > - do so will result in leaving a iterator marker node in the immediate > - use list, and nothing good will come from that. */ > +/* These used to be needed to exit FOR_EACH_IMM_USE_STMT early, without leaving > + ITER behind in the stmt lists. This is now taken care of automatically, but > + it doesn't hurt to use the macros for documentation purposes. */ > #define BREAK_FROM_IMM_USE_STMT(ITER) \ > { \ > end_imm_use_stmt_traverse (&(ITER)); \ > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
On Jan 4, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > Hmm - while the change looks good, doesn't it end up > calling end_imm_use_stmt_tranverse twice for those > uses still calling BREAK_FROM_IMM_USE_STMT? It does. I'd considered introducing a separate method to call end_imm_use_stmt_traverse if imm is not NULL, and then set it to NULL, but calling the function multiple times is not a problem: delink_imm_use just returns immediately the second time. > Thus, please remove uses of BREAK_FROM_IMM_USE_STMT > together with this patch. And RETURN_FROM_IMM_USE_STMT, I suppose? I wasn't sure whether to remove them and their users.
On Wed, Jan 6, 2021 at 12:34 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Jan 4, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > > > Hmm - while the change looks good, doesn't it end up > > calling end_imm_use_stmt_tranverse twice for those > > uses still calling BREAK_FROM_IMM_USE_STMT? > > It does. I'd considered introducing a separate method to call > end_imm_use_stmt_traverse if imm is not NULL, and then set it to NULL, > but calling the function multiple times is not a problem: delink_imm_use > just returns immediately the second time. > > > Thus, please remove uses of BREAK_FROM_IMM_USE_STMT > > together with this patch. > > And RETURN_FROM_IMM_USE_STMT, I suppose? Sure. > I wasn't sure whether to remove them and their users. > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
On Jan 7, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > On Wed, Jan 6, 2021 at 12:34 PM Alexandre Oliva <oliva@adacore.com> wrote: >> >> On Jan 4, 2021, Richard Biener <richard.guenther@gmail.com> wrote: >> >> > Thus, please remove uses of BREAK_FROM_IMM_USE_STMT >> > together with this patch. >> >> And RETURN_FROM_IMM_USE_STMT, I suppose? > Sure. Done. make FOR_EACH_IMM_USE_STMT safe for early exits Use a dtor to automatically remove ITER from IMM_USE list in FOR_EACH_IMM_USE_STMT. Regstrapped on x86_64-linux-gnu. Ok to install? for gcc/ChangeLog * ssa-iterators.h (end_imm_use_stmt_traverse): Forward declare. (auto_end_imm_use_stmt_traverse): New struct. (FOR_EACH_IMM_USE_STMT): Use it. (BREAK_FROM_IMM_USE_STMT, RETURN_FROM_IMM_USE_STMT): Remove, along with uses... * gimple-ssa-strength-reduction.c: ... here, ... * graphite-scop-detection.c: ... here, ... * ipa-modref.c, ipa-pure-const.c, ipa-sra.c: ... here, ... * tree-predcom.c, tree-ssa-ccp.c: ... here, ... * tree-ssa-dce.c, tree-ssa-dse.c: ... here, ... * tree-ssa-loop-ivopts.c, tree-ssa-math-opts.c: ... here, ... * tree-ssa-phiprop.c, tree-ssa.c: ... here, ... * tree-vect-slp.c: ... and here, ... * doc/tree-ssa.texi: ... and the example here. --- gcc/doc/tree-ssa.texi | 16 ++------------ gcc/gimple-ssa-strength-reduction.c | 2 +- gcc/graphite-scop-detection.c | 4 +--- gcc/ipa-modref.c | 4 +--- gcc/ipa-pure-const.c | 8 ++++--- gcc/ipa-sra.c | 22 ++++++++++---------- gcc/ssa-iterators.h | 39 +++++++++++++++++++---------------- gcc/tree-predcom.c | 2 +- gcc/tree-ssa-ccp.c | 2 +- gcc/tree-ssa-dce.c | 2 +- gcc/tree-ssa-dse.c | 8 ++++--- gcc/tree-ssa-loop-ivopts.c | 2 +- gcc/tree-ssa-math-opts.c | 2 +- gcc/tree-ssa-phiprop.c | 2 +- gcc/tree-ssa.c | 2 +- gcc/tree-vect-slp.c | 4 ++-- 16 files changed, 54 insertions(+), 67 deletions(-) diff --git a/gcc/doc/tree-ssa.texi b/gcc/doc/tree-ssa.texi index f927f89cb7f64..057135f80d116 100644 --- a/gcc/doc/tree-ssa.texi +++ b/gcc/doc/tree-ssa.texi @@ -385,20 +385,8 @@ optimization can manipulate the stmt when all the uses have been processed. This is a little slower than the FAST version since it adds a placeholder element and must sort through the list a bit for each statement. This placeholder element must be also be removed if the loop is -terminated early. The macro @code{BREAK_FROM_IMM_USE_STMT} is provided -to do this : - -@smallexample - FOR_EACH_IMM_USE_STMT (stmt, iterator, ssa_var) - @{ - if (stmt == last_stmt) - BREAK_FROM_IMM_USE_STMT (iterator); - - FOR_EACH_IMM_USE_ON_STMT (imm_use_p, iterator) - SET_USE (imm_use_p, ssa_var_2); - fold_stmt (stmt); - @} -@end smallexample +terminated early; a destructor takes care of that when leaving the +@code{FOR_EACH_IMM_USE_STMT} scope. There are checks in @code{verify_ssa} which verify that the immediate use list is up to date, as well as checking that an optimization didn't break from the diff --git a/gcc/gimple-ssa-strength-reduction.c b/gcc/gimple-ssa-strength-reduction.c index b6cbb21dfbc73..a92cf03c1f322 100644 --- a/gcc/gimple-ssa-strength-reduction.c +++ b/gcc/gimple-ssa-strength-reduction.c @@ -524,7 +524,7 @@ uses_consumed_by_stmt (tree name, gimple *stmt, unsigned recurse = 0) recurse + 1)) { retval = false; - BREAK_FROM_IMM_USE_STMT (iter); + break; } } diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c index 4f7bbe5d03524..3e729b159b095 100644 --- a/gcc/graphite-scop-detection.c +++ b/gcc/graphite-scop-detection.c @@ -1262,9 +1262,7 @@ build_cross_bb_scalars_def (scop_p scop, tree def, basic_block def_bb, && (def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))) { add_write (writes, def); - /* This is required by the FOR_EACH_IMM_USE_STMT when we want to break - before all the uses have been visited. */ - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } } diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index 04613201f1f1e..74ad876cf581e 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -1597,9 +1597,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, FOR_EACH_IMM_USE_STMT (use_stmt, ui, name) { if (lattice[index].flags == 0) - { - BREAK_FROM_IMM_USE_STMT (ui); - } + break; if (is_gimple_debug (use_stmt)) continue; if (dump_file) diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c index 66f2177110788..957217ae4ae34 100644 --- a/gcc/ipa-pure-const.c +++ b/gcc/ipa-pure-const.c @@ -842,20 +842,20 @@ check_retval_uses (tree retval, gimple *stmt) { tree op2 = gimple_cond_rhs (cond); if (!integer_zerop (op2)) - RETURN_FROM_IMM_USE_STMT (use_iter, false); + return false; } else if (gassign *ga = dyn_cast<gassign *> (use_stmt)) { enum tree_code code = gimple_assign_rhs_code (ga); if (TREE_CODE_CLASS (code) != tcc_comparison) - RETURN_FROM_IMM_USE_STMT (use_iter, false); + return false; if (!integer_zerop (gimple_assign_rhs2 (ga))) - RETURN_FROM_IMM_USE_STMT (use_iter, false); + return false; } else if (is_gimple_debug (use_stmt)) ; else if (use_stmt != stmt) - RETURN_FROM_IMM_USE_STMT (use_iter, false); + return false; return true; } diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c index d0579231d9cfc..5d2c0dfce533e 100644 --- a/gcc/ipa-sra.c +++ b/gcc/ipa-sra.c @@ -850,7 +850,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, || (arg_count = gimple_call_num_args (call)) == 0) { res = -1; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } cgraph_edge *cs = node->get_edge (stmt); @@ -874,7 +874,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, || all_uses != simple_uses) { res = -1; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } res += all_uses; } @@ -891,7 +891,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, if (TREE_CODE (lhs) != SSA_NAME) { res = -1; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } gcc_assert (!gimple_vdef (stmt)); if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs))) @@ -901,7 +901,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, if (tmp < 0) { res = tmp; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } res += tmp; } @@ -909,7 +909,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, else { res = -1; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } } return res; @@ -1016,7 +1016,7 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, || (arg_count = gimple_call_num_args (call)) == 0) { ret = true; - BREAK_FROM_IMM_USE_STMT (ui); + break; } cgraph_edge *cs = node->get_edge (stmt); @@ -1062,7 +1062,7 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, if (uses_ok != all_uses) { ret = true; - BREAK_FROM_IMM_USE_STMT (ui); + break; } } @@ -1975,7 +1975,7 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) if (t != name) { res = false; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } } else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt)) @@ -1991,20 +1991,20 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) if (TREE_CODE (lhs) != SSA_NAME) { res = false; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } gcc_assert (!gimple_vdef (stmt)); if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs)) && !ssa_name_only_returned_p (lhs, analyzed)) { res = false; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } } else { res = false; - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } } return res; diff --git a/gcc/ssa-iterators.h b/gcc/ssa-iterators.h index c0b45b63db442..f70b0a42368d7 100644 --- a/gcc/ssa-iterators.h +++ b/gcc/ssa-iterators.h @@ -77,29 +77,32 @@ struct imm_use_iterator !end_readonly_imm_use_p (&(ITER)); \ (void) ((DEST) = next_readonly_imm_use (&(ITER)))) -/* Use this iterator to visit each stmt which has a use of SSAVAR. */ +/* Forward declare for use in the class below. */ +static inline void end_imm_use_stmt_traverse (imm_use_iterator *); + +/* arrange to automatically call, upon descruction, end_imm_use_stmt_traverse + with a given pointer to imm_use_iterator. */ +struct auto_end_imm_use_stmt_traverse +{ + imm_use_iterator *imm; + auto_end_imm_use_stmt_traverse (imm_use_iterator *imm) + : imm (imm) {} + ~auto_end_imm_use_stmt_traverse () + { end_imm_use_stmt_traverse (imm); } +}; + +/* Use this iterator to visit each stmt which has a use of SSAVAR. The + destructor of the auto_end_imm_use_stmt_traverse object deals with removing + ITER from SSAVAR's IMM_USE list even when leaving the scope early. */ #define FOR_EACH_IMM_USE_STMT(STMT, ITER, SSAVAR) \ - for ((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR)); \ + for (struct auto_end_imm_use_stmt_traverse \ + auto_end_imm_use_stmt_traverse \ + ((((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR))), \ + &(ITER))); \ !end_imm_use_stmt_p (&(ITER)); \ (void) ((STMT) = next_imm_use_stmt (&(ITER)))) -/* Use this to terminate the FOR_EACH_IMM_USE_STMT loop early. Failure to - do so will result in leaving a iterator marker node in the immediate - use list, and nothing good will come from that. */ -#define BREAK_FROM_IMM_USE_STMT(ITER) \ - { \ - end_imm_use_stmt_traverse (&(ITER)); \ - break; \ - } - -/* Similarly for return. */ -#define RETURN_FROM_IMM_USE_STMT(ITER, VAL) \ - { \ - end_imm_use_stmt_traverse (&(ITER)); \ - return (VAL); \ - } - /* Use this iterator in combination with FOR_EACH_IMM_USE_STMT to get access to each occurrence of ssavar on the stmt returned by that iterator.. for instance: diff --git a/gcc/tree-predcom.c b/gcc/tree-predcom.c index 265c3bbe445b0..5482f50198a6e 100644 --- a/gcc/tree-predcom.c +++ b/gcc/tree-predcom.c @@ -2367,7 +2367,7 @@ base_names_in_chain_on (class loop *loop, tree name, tree var) && flow_bb_inside_loop_p (loop, gimple_bb (stmt))) { phi = stmt; - BREAK_FROM_IMM_USE_STMT (iter); + break; } } if (!phi) diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c index c5c2e2a61075c..965f092ccccb4 100644 --- a/gcc/tree-ssa-ccp.c +++ b/gcc/tree-ssa-ccp.c @@ -3020,7 +3020,7 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip, } use_bool = false; - BREAK_FROM_IMM_USE_STMT (iter); + break; } tree new_lhs = make_ssa_name (TREE_TYPE (lhs)); diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c index d155bdfb2aefd..51d4fcbb1c84a 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -1516,7 +1516,7 @@ eliminate_unnecessary_stmts (void) || gimple_plf (stmt, STMT_NECESSARY)) { found = true; - BREAK_FROM_IMM_USE_STMT (iter); + break; } } if (found) diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c index cc252fcc20e5b..4967a5a99272b 100644 --- a/gcc/tree-ssa-dse.c +++ b/gcc/tree-ssa-dse.c @@ -693,7 +693,7 @@ dse_optimize_redundant_stores (gimple *stmt) { /* Limit stmt walking. */ if (++cnt > param_dse_max_alias_queries_per_store) - BREAK_FROM_IMM_USE_STMT (ui); + break; /* If USE_STMT stores 0 into one or more of the same locations as STMT and STMT would kill USE_STMT, then we can just remove @@ -712,7 +712,7 @@ dse_optimize_redundant_stores (gimple *stmt) ao_ref write; if (!initialize_ao_ref_for_dse (use_stmt, &write)) - BREAK_FROM_IMM_USE_STMT (ui) + break; if (valid_ao_ref_for_dse (&write) && stmt_kills_ref_p (stmt, &write)) @@ -805,7 +805,7 @@ dse_classify_store (ao_ref *ref, gimple *stmt, if (++cnt > param_dse_max_alias_queries_per_store) { fail = true; - BREAK_FROM_IMM_USE_STMT (ui); + break; } /* We have visited ourselves already so ignore STMT for the @@ -852,7 +852,7 @@ dse_classify_store (ao_ref *ref, gimple *stmt, } fail = true; - BREAK_FROM_IMM_USE_STMT (ui); + break; } /* If this is a store, remember it as we possibly need to walk the defs uses. */ diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c index 8e66da1e92321..4012ae3f19ddb 100644 --- a/gcc/tree-ssa-loop-ivopts.c +++ b/gcc/tree-ssa-loop-ivopts.c @@ -7627,7 +7627,7 @@ remove_unused_ivs (struct ivopts_data *data, bitmap toremove) count++; if (count > 1) - BREAK_FROM_IMM_USE_STMT (imm_iter); + break; } if (!count) diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 52849728ab2c2..651fc586d2395 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -1284,7 +1284,7 @@ execute_cse_sincos_1 (tree name) and, in subsequent rounds, that the built_in type is the same type, or a compatible type. */ if (type != t && !types_compatible_p (type, t)) - RETURN_FROM_IMM_USE_STMT (use_iter, false); + return false; } if (seen_cos + seen_sin + seen_cexpi <= 1) return false; diff --git a/gcc/tree-ssa-phiprop.c b/gcc/tree-ssa-phiprop.c index d5cbf33bdbcc5..64d6eda5f6c26 100644 --- a/gcc/tree-ssa-phiprop.c +++ b/gcc/tree-ssa-phiprop.c @@ -119,7 +119,7 @@ phivn_valid_p (struct phiprop_d *phivn, tree name, basic_block bb) && !dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), bb)) { ok = false; - BREAK_FROM_IMM_USE_STMT (ui2); + break; } } diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c index 2a6d0866e9f8d..cf54c8914268d 100644 --- a/gcc/tree-ssa.c +++ b/gcc/tree-ssa.c @@ -607,7 +607,7 @@ release_defs_bitset (bitmap toremove) } if (!remove_now) - BREAK_FROM_IMM_USE_STMT (uit); + break; } if (remove_now) diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c index e7191ed326737..877d44b2257bb 100644 --- a/gcc/tree-vect-slp.c +++ b/gcc/tree-vect-slp.c @@ -3857,7 +3857,7 @@ vect_bb_slp_mark_live_stmts (bb_vec_info bb_vinfo, slp_tree node, mark_visited = false; else STMT_VINFO_LIVE_P (stmt_info) = false; - BREAK_FROM_IMM_USE_STMT (use_iter); + break; } } /* We have to verify whether we can insert the lane extract @@ -4124,7 +4124,7 @@ vect_bb_slp_scalar_cost (vec_info *vinfo, (vect_stmt_to_vectorize (use_stmt_info))) { (*life)[i] = true; - BREAK_FROM_IMM_USE_STMT (use_iter); + break; } } }
On Sat, Jan 9, 2021 at 9:33 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Jan 7, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > > > On Wed, Jan 6, 2021 at 12:34 PM Alexandre Oliva <oliva@adacore.com> wrote: > >> > >> On Jan 4, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > >> > >> > Thus, please remove uses of BREAK_FROM_IMM_USE_STMT > >> > together with this patch. > >> > >> And RETURN_FROM_IMM_USE_STMT, I suppose? > > > Sure. > > Done. > > > make FOR_EACH_IMM_USE_STMT safe for early exits > > Use a dtor to automatically remove ITER from IMM_USE list in > FOR_EACH_IMM_USE_STMT. > > Regstrapped on x86_64-linux-gnu. Ok to install? OK. Thanks, Richard. > > > for gcc/ChangeLog > > * ssa-iterators.h (end_imm_use_stmt_traverse): Forward > declare. > (auto_end_imm_use_stmt_traverse): New struct. > (FOR_EACH_IMM_USE_STMT): Use it. > (BREAK_FROM_IMM_USE_STMT, RETURN_FROM_IMM_USE_STMT): Remove, > along with uses... > * gimple-ssa-strength-reduction.c: ... here, ... > * graphite-scop-detection.c: ... here, ... > * ipa-modref.c, ipa-pure-const.c, ipa-sra.c: ... here, ... > * tree-predcom.c, tree-ssa-ccp.c: ... here, ... > * tree-ssa-dce.c, tree-ssa-dse.c: ... here, ... > * tree-ssa-loop-ivopts.c, tree-ssa-math-opts.c: ... here, ... > * tree-ssa-phiprop.c, tree-ssa.c: ... here, ... > * tree-vect-slp.c: ... and here, ... > * doc/tree-ssa.texi: ... and the example here. > --- > gcc/doc/tree-ssa.texi | 16 ++------------ > gcc/gimple-ssa-strength-reduction.c | 2 +- > gcc/graphite-scop-detection.c | 4 +--- > gcc/ipa-modref.c | 4 +--- > gcc/ipa-pure-const.c | 8 ++++--- > gcc/ipa-sra.c | 22 ++++++++++---------- > gcc/ssa-iterators.h | 39 +++++++++++++++++++---------------- > gcc/tree-predcom.c | 2 +- > gcc/tree-ssa-ccp.c | 2 +- > gcc/tree-ssa-dce.c | 2 +- > gcc/tree-ssa-dse.c | 8 ++++--- > gcc/tree-ssa-loop-ivopts.c | 2 +- > gcc/tree-ssa-math-opts.c | 2 +- > gcc/tree-ssa-phiprop.c | 2 +- > gcc/tree-ssa.c | 2 +- > gcc/tree-vect-slp.c | 4 ++-- > 16 files changed, 54 insertions(+), 67 deletions(-) > > diff --git a/gcc/doc/tree-ssa.texi b/gcc/doc/tree-ssa.texi > index f927f89cb7f64..057135f80d116 100644 > --- a/gcc/doc/tree-ssa.texi > +++ b/gcc/doc/tree-ssa.texi > @@ -385,20 +385,8 @@ optimization can manipulate the stmt when all the uses have been > processed. This is a little slower than the FAST version since it adds a > placeholder element and must sort through the list a bit for each statement. > This placeholder element must be also be removed if the loop is > -terminated early. The macro @code{BREAK_FROM_IMM_USE_STMT} is provided > -to do this : > - > -@smallexample > - FOR_EACH_IMM_USE_STMT (stmt, iterator, ssa_var) > - @{ > - if (stmt == last_stmt) > - BREAK_FROM_IMM_USE_STMT (iterator); > - > - FOR_EACH_IMM_USE_ON_STMT (imm_use_p, iterator) > - SET_USE (imm_use_p, ssa_var_2); > - fold_stmt (stmt); > - @} > -@end smallexample > +terminated early; a destructor takes care of that when leaving the > +@code{FOR_EACH_IMM_USE_STMT} scope. > > There are checks in @code{verify_ssa} which verify that the immediate use list > is up to date, as well as checking that an optimization didn't break from the > diff --git a/gcc/gimple-ssa-strength-reduction.c b/gcc/gimple-ssa-strength-reduction.c > index b6cbb21dfbc73..a92cf03c1f322 100644 > --- a/gcc/gimple-ssa-strength-reduction.c > +++ b/gcc/gimple-ssa-strength-reduction.c > @@ -524,7 +524,7 @@ uses_consumed_by_stmt (tree name, gimple *stmt, unsigned recurse = 0) > recurse + 1)) > { > retval = false; > - BREAK_FROM_IMM_USE_STMT (iter); > + break; > } > } > > diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c > index 4f7bbe5d03524..3e729b159b095 100644 > --- a/gcc/graphite-scop-detection.c > +++ b/gcc/graphite-scop-detection.c > @@ -1262,9 +1262,7 @@ build_cross_bb_scalars_def (scop_p scop, tree def, basic_block def_bb, > && (def_bb != gimple_bb (use_stmt) && !is_gimple_debug (use_stmt))) > { > add_write (writes, def); > - /* This is required by the FOR_EACH_IMM_USE_STMT when we want to break > - before all the uses have been visited. */ > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > } > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index 04613201f1f1e..74ad876cf581e 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1597,9 +1597,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice> &lattice, int depth, > FOR_EACH_IMM_USE_STMT (use_stmt, ui, name) > { > if (lattice[index].flags == 0) > - { > - BREAK_FROM_IMM_USE_STMT (ui); > - } > + break; > if (is_gimple_debug (use_stmt)) > continue; > if (dump_file) > diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c > index 66f2177110788..957217ae4ae34 100644 > --- a/gcc/ipa-pure-const.c > +++ b/gcc/ipa-pure-const.c > @@ -842,20 +842,20 @@ check_retval_uses (tree retval, gimple *stmt) > { > tree op2 = gimple_cond_rhs (cond); > if (!integer_zerop (op2)) > - RETURN_FROM_IMM_USE_STMT (use_iter, false); > + return false; > } > else if (gassign *ga = dyn_cast<gassign *> (use_stmt)) > { > enum tree_code code = gimple_assign_rhs_code (ga); > if (TREE_CODE_CLASS (code) != tcc_comparison) > - RETURN_FROM_IMM_USE_STMT (use_iter, false); > + return false; > if (!integer_zerop (gimple_assign_rhs2 (ga))) > - RETURN_FROM_IMM_USE_STMT (use_iter, false); > + return false; > } > else if (is_gimple_debug (use_stmt)) > ; > else if (use_stmt != stmt) > - RETURN_FROM_IMM_USE_STMT (use_iter, false); > + return false; > > return true; > } > diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c > index d0579231d9cfc..5d2c0dfce533e 100644 > --- a/gcc/ipa-sra.c > +++ b/gcc/ipa-sra.c > @@ -850,7 +850,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, > || (arg_count = gimple_call_num_args (call)) == 0) > { > res = -1; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > > cgraph_edge *cs = node->get_edge (stmt); > @@ -874,7 +874,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, > || all_uses != simple_uses) > { > res = -1; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > res += all_uses; > } > @@ -891,7 +891,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, > if (TREE_CODE (lhs) != SSA_NAME) > { > res = -1; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > gcc_assert (!gimple_vdef (stmt)); > if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs))) > @@ -901,7 +901,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, > if (tmp < 0) > { > res = tmp; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > res += tmp; > } > @@ -909,7 +909,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, > else > { > res = -1; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > } > return res; > @@ -1016,7 +1016,7 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, > || (arg_count = gimple_call_num_args (call)) == 0) > { > ret = true; > - BREAK_FROM_IMM_USE_STMT (ui); > + break; > } > > cgraph_edge *cs = node->get_edge (stmt); > @@ -1062,7 +1062,7 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, > if (uses_ok != all_uses) > { > ret = true; > - BREAK_FROM_IMM_USE_STMT (ui); > + break; > } > } > > @@ -1975,7 +1975,7 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) > if (t != name) > { > res = false; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > } > else if ((is_gimple_assign (stmt) && !gimple_has_volatile_ops (stmt)) > @@ -1991,20 +1991,20 @@ ssa_name_only_returned_p (tree name, bitmap analyzed) > if (TREE_CODE (lhs) != SSA_NAME) > { > res = false; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > gcc_assert (!gimple_vdef (stmt)); > if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs)) > && !ssa_name_only_returned_p (lhs, analyzed)) > { > res = false; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > } > else > { > res = false; > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > } > return res; > diff --git a/gcc/ssa-iterators.h b/gcc/ssa-iterators.h > index c0b45b63db442..f70b0a42368d7 100644 > --- a/gcc/ssa-iterators.h > +++ b/gcc/ssa-iterators.h > @@ -77,29 +77,32 @@ struct imm_use_iterator > !end_readonly_imm_use_p (&(ITER)); \ > (void) ((DEST) = next_readonly_imm_use (&(ITER)))) > > -/* Use this iterator to visit each stmt which has a use of SSAVAR. */ > +/* Forward declare for use in the class below. */ > +static inline void end_imm_use_stmt_traverse (imm_use_iterator *); > + > +/* arrange to automatically call, upon descruction, end_imm_use_stmt_traverse > + with a given pointer to imm_use_iterator. */ > +struct auto_end_imm_use_stmt_traverse > +{ > + imm_use_iterator *imm; > + auto_end_imm_use_stmt_traverse (imm_use_iterator *imm) > + : imm (imm) {} > + ~auto_end_imm_use_stmt_traverse () > + { end_imm_use_stmt_traverse (imm); } > +}; > + > +/* Use this iterator to visit each stmt which has a use of SSAVAR. The > + destructor of the auto_end_imm_use_stmt_traverse object deals with removing > + ITER from SSAVAR's IMM_USE list even when leaving the scope early. */ > > #define FOR_EACH_IMM_USE_STMT(STMT, ITER, SSAVAR) \ > - for ((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR)); \ > + for (struct auto_end_imm_use_stmt_traverse \ > + auto_end_imm_use_stmt_traverse \ > + ((((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR))), \ > + &(ITER))); \ > !end_imm_use_stmt_p (&(ITER)); \ > (void) ((STMT) = next_imm_use_stmt (&(ITER)))) > > -/* Use this to terminate the FOR_EACH_IMM_USE_STMT loop early. Failure to > - do so will result in leaving a iterator marker node in the immediate > - use list, and nothing good will come from that. */ > -#define BREAK_FROM_IMM_USE_STMT(ITER) \ > - { \ > - end_imm_use_stmt_traverse (&(ITER)); \ > - break; \ > - } > - > -/* Similarly for return. */ > -#define RETURN_FROM_IMM_USE_STMT(ITER, VAL) \ > - { \ > - end_imm_use_stmt_traverse (&(ITER)); \ > - return (VAL); \ > - } > - > /* Use this iterator in combination with FOR_EACH_IMM_USE_STMT to > get access to each occurrence of ssavar on the stmt returned by > that iterator.. for instance: > diff --git a/gcc/tree-predcom.c b/gcc/tree-predcom.c > index 265c3bbe445b0..5482f50198a6e 100644 > --- a/gcc/tree-predcom.c > +++ b/gcc/tree-predcom.c > @@ -2367,7 +2367,7 @@ base_names_in_chain_on (class loop *loop, tree name, tree var) > && flow_bb_inside_loop_p (loop, gimple_bb (stmt))) > { > phi = stmt; > - BREAK_FROM_IMM_USE_STMT (iter); > + break; > } > } > if (!phi) > diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c > index c5c2e2a61075c..965f092ccccb4 100644 > --- a/gcc/tree-ssa-ccp.c > +++ b/gcc/tree-ssa-ccp.c > @@ -3020,7 +3020,7 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip, > } > > use_bool = false; > - BREAK_FROM_IMM_USE_STMT (iter); > + break; > } > > tree new_lhs = make_ssa_name (TREE_TYPE (lhs)); > diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c > index d155bdfb2aefd..51d4fcbb1c84a 100644 > --- a/gcc/tree-ssa-dce.c > +++ b/gcc/tree-ssa-dce.c > @@ -1516,7 +1516,7 @@ eliminate_unnecessary_stmts (void) > || gimple_plf (stmt, STMT_NECESSARY)) > { > found = true; > - BREAK_FROM_IMM_USE_STMT (iter); > + break; > } > } > if (found) > diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c > index cc252fcc20e5b..4967a5a99272b 100644 > --- a/gcc/tree-ssa-dse.c > +++ b/gcc/tree-ssa-dse.c > @@ -693,7 +693,7 @@ dse_optimize_redundant_stores (gimple *stmt) > { > /* Limit stmt walking. */ > if (++cnt > param_dse_max_alias_queries_per_store) > - BREAK_FROM_IMM_USE_STMT (ui); > + break; > > /* If USE_STMT stores 0 into one or more of the same locations > as STMT and STMT would kill USE_STMT, then we can just remove > @@ -712,7 +712,7 @@ dse_optimize_redundant_stores (gimple *stmt) > ao_ref write; > > if (!initialize_ao_ref_for_dse (use_stmt, &write)) > - BREAK_FROM_IMM_USE_STMT (ui) > + break; > > if (valid_ao_ref_for_dse (&write) > && stmt_kills_ref_p (stmt, &write)) > @@ -805,7 +805,7 @@ dse_classify_store (ao_ref *ref, gimple *stmt, > if (++cnt > param_dse_max_alias_queries_per_store) > { > fail = true; > - BREAK_FROM_IMM_USE_STMT (ui); > + break; > } > > /* We have visited ourselves already so ignore STMT for the > @@ -852,7 +852,7 @@ dse_classify_store (ao_ref *ref, gimple *stmt, > } > > fail = true; > - BREAK_FROM_IMM_USE_STMT (ui); > + break; > } > /* If this is a store, remember it as we possibly need to walk the > defs uses. */ > diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c > index 8e66da1e92321..4012ae3f19ddb 100644 > --- a/gcc/tree-ssa-loop-ivopts.c > +++ b/gcc/tree-ssa-loop-ivopts.c > @@ -7627,7 +7627,7 @@ remove_unused_ivs (struct ivopts_data *data, bitmap toremove) > count++; > > if (count > 1) > - BREAK_FROM_IMM_USE_STMT (imm_iter); > + break; > } > > if (!count) > diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c > index 52849728ab2c2..651fc586d2395 100644 > --- a/gcc/tree-ssa-math-opts.c > +++ b/gcc/tree-ssa-math-opts.c > @@ -1284,7 +1284,7 @@ execute_cse_sincos_1 (tree name) > and, in subsequent rounds, that the built_in type is the same > type, or a compatible type. */ > if (type != t && !types_compatible_p (type, t)) > - RETURN_FROM_IMM_USE_STMT (use_iter, false); > + return false; > } > if (seen_cos + seen_sin + seen_cexpi <= 1) > return false; > diff --git a/gcc/tree-ssa-phiprop.c b/gcc/tree-ssa-phiprop.c > index d5cbf33bdbcc5..64d6eda5f6c26 100644 > --- a/gcc/tree-ssa-phiprop.c > +++ b/gcc/tree-ssa-phiprop.c > @@ -119,7 +119,7 @@ phivn_valid_p (struct phiprop_d *phivn, tree name, basic_block bb) > && !dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), bb)) > { > ok = false; > - BREAK_FROM_IMM_USE_STMT (ui2); > + break; > } > } > > diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c > index 2a6d0866e9f8d..cf54c8914268d 100644 > --- a/gcc/tree-ssa.c > +++ b/gcc/tree-ssa.c > @@ -607,7 +607,7 @@ release_defs_bitset (bitmap toremove) > } > > if (!remove_now) > - BREAK_FROM_IMM_USE_STMT (uit); > + break; > } > > if (remove_now) > diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c > index e7191ed326737..877d44b2257bb 100644 > --- a/gcc/tree-vect-slp.c > +++ b/gcc/tree-vect-slp.c > @@ -3857,7 +3857,7 @@ vect_bb_slp_mark_live_stmts (bb_vec_info bb_vinfo, slp_tree node, > mark_visited = false; > else > STMT_VINFO_LIVE_P (stmt_info) = false; > - BREAK_FROM_IMM_USE_STMT (use_iter); > + break; > } > } > /* We have to verify whether we can insert the lane extract > @@ -4124,7 +4124,7 @@ vect_bb_slp_scalar_cost (vec_info *vinfo, > (vect_stmt_to_vectorize (use_stmt_info))) > { > (*life)[i] = true; > - BREAK_FROM_IMM_USE_STMT (use_iter); > + break; > } > } > } > > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > Vim, Vi, Voltei pro Emacs -- GNUlius Caesar
On 1/9/21 3:33 PM, Alexandre Oliva wrote: > On Jan 7, 2021, Richard Biener <richard.guenther@gmail.com> wrote: > >> On Wed, Jan 6, 2021 at 12:34 PM Alexandre Oliva <oliva@adacore.com> wrote: >>> On Jan 4, 2021, Richard Biener <richard.guenther@gmail.com> wrote: >>> >>>> Thus, please remove uses of BREAK_FROM_IMM_USE_STMT >>>> together with this patch. >>> And RETURN_FROM_IMM_USE_STMT, I suppose? >> Sure. > Done. > > > make FOR_EACH_IMM_USE_STMT safe for early exits > > Use a dtor to automatically remove ITER from IMM_USE list in > FOR_EACH_IMM_USE_STMT. > > Regstrapped on x86_64-linux-gnu. Ok to install? > > > Nice! I like it. One less wart for my epitaph :-). Thanks Andrew
diff --git a/gcc/ssa-iterators.h b/gcc/ssa-iterators.h index 98724da14522d..817997aa952d4 100644 --- a/gcc/ssa-iterators.h +++ b/gcc/ssa-iterators.h @@ -77,16 +77,35 @@ struct imm_use_iterator !end_readonly_imm_use_p (&(ITER)); \ (void) ((DEST) = next_readonly_imm_use (&(ITER)))) -/* Use this iterator to visit each stmt which has a use of SSAVAR. */ +/* Forward declare for use in the class below. */ +static inline void end_imm_use_stmt_traverse (imm_use_iterator *); + +/* arrange to automatically call, upon descruction, end_imm_use_stmt_traverse + with a given pointer to imm_use_iterator. */ +struct auto_end_imm_use_stmt_traverse +{ + imm_use_iterator *imm; + auto_end_imm_use_stmt_traverse (imm_use_iterator *imm) + : imm (imm) {} + ~auto_end_imm_use_stmt_traverse () + { end_imm_use_stmt_traverse (imm); } +}; + +/* Use this iterator to visit each stmt which has a use of SSAVAR. The + destructor of the auto_end_imm_use_stmt_traverse object deals with removing + ITER from SSAVAR's IMM_USE list even when leaving the scope early. */ #define FOR_EACH_IMM_USE_STMT(STMT, ITER, SSAVAR) \ - for ((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR)); \ + for (struct auto_end_imm_use_stmt_traverse \ + auto_end_imm_use_stmt_traverse \ + ((((STMT) = first_imm_use_stmt (&(ITER), (SSAVAR))), \ + &(ITER))); \ !end_imm_use_stmt_p (&(ITER)); \ (void) ((STMT) = next_imm_use_stmt (&(ITER)))) -/* Use this to terminate the FOR_EACH_IMM_USE_STMT loop early. Failure to - do so will result in leaving a iterator marker node in the immediate - use list, and nothing good will come from that. */ +/* These used to be needed to exit FOR_EACH_IMM_USE_STMT early, without leaving + ITER behind in the stmt lists. This is now taken care of automatically, but + it doesn't hurt to use the macros for documentation purposes. */ #define BREAK_FROM_IMM_USE_STMT(ITER) \ { \ end_imm_use_stmt_traverse (&(ITER)); \