On Thu, Jul 8, 2021 at 2:45 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This patch adds a helper function called vect_phi_initial_value
> for returning the incoming value of a given loop phi. The main
> reason for adding it is to ensure that the right preheader edge
> is used when vectorising nested loops. (PHI_ARG_DEF_FROM_EDGE
> itself doesn't assert that the given edge is for the right block,
> although I guess that would be good to add separately.)
We were sometimes (most of the time?) using an explicit
loop where you now get it from the PHI - that makes the
assert somewhat pointless to some extent - of course it
makes sense on its own that the loop is the same as that
of the PHI def. I just wonder if you think any of the existing
code might have been wrong? If so the new assert doesn't
catch all originally wrong cases.
Otherwise OK,
Richard.
> gcc/
> * tree-vectorizer.h: Include tree-ssa-operands.h.
> (vect_phi_initial_value): New function.
> * tree-vect-loop.c (neutral_op_for_slp_reduction): Use it.
> (get_initial_defs_for_reduction, info_for_reduction): Likewise.
> (vect_create_epilog_for_reduction, vectorizable_reduction): Likewise.
> (vect_transform_cycle_phi, vectorizable_induction): Likewise.
> ---
> gcc/tree-vect-loop.c | 29 +++++++++--------------------
> gcc/tree-vectorizer.h | 21 ++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 1bd9a6ea52c..a31d7621c3b 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -3288,8 +3288,7 @@ neutral_op_for_slp_reduction (slp_tree slp_node, tree
> vector_type,
> has only a single initial value, so that value is neutral for
> all statements. */
> if (reduc_chain)
> - return PHI_ARG_DEF_FROM_EDGE (stmt_vinfo->stmt,
> - loop_preheader_edge (loop));
> + return vect_phi_initial_value (stmt_vinfo);
> return NULL_TREE;
>
> default:
> @@ -4829,13 +4828,13 @@ get_initial_defs_for_reduction (vec_info *vinfo,
> /* Get the def before the loop. In reduction chain we have only
> one initial value. Else we have as many as PHIs in the group. */
> if (reduc_chain)
> - op = j != 0 ? neutral_op : PHI_ARG_DEF_FROM_EDGE (stmt_vinfo->stmt,
> pe);
> + op = j != 0 ? neutral_op : vect_phi_initial_value (stmt_vinfo);
> else if (((vec_oprnds->length () + 1) * nunits
> - number_of_places_left_in_vector >= group_size)
> && neutral_op)
> op = neutral_op;
> else
> - op = PHI_ARG_DEF_FROM_EDGE (stmt_vinfo->stmt, pe);
> + op = vect_phi_initial_value (stmt_vinfo);
>
> /* Create 'vect_ = {op0,op1,...,opn}'. */
> number_of_places_left_in_vector--;
> @@ -4906,9 +4905,7 @@ info_for_reduction (vec_info *vinfo, stmt_vec_info
> stmt_info)
> }
> else if (STMT_VINFO_DEF_TYPE (stmt_info) == vect_nested_cycle)
> {
> - edge pe = loop_preheader_edge (gimple_bb (phi)->loop_father);
> - stmt_vec_info info
> - = vinfo->lookup_def (PHI_ARG_DEF_FROM_EDGE (phi, pe));
> + stmt_vec_info info = vinfo->lookup_def (vect_phi_initial_value (phi));
> if (info && STMT_VINFO_DEF_TYPE (info) == vect_double_reduction_def)
> stmt_info = info;
> }
> @@ -5042,8 +5039,7 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> {
> /* Get at the scalar def before the loop, that defines the initial
> value
> of the reduction variable. */
> - initial_def = PHI_ARG_DEF_FROM_EDGE (reduc_def_stmt,
> - loop_preheader_edge (loop));
> + initial_def = vect_phi_initial_value (reduc_def_stmt);
> /* Optimize: for induction condition reduction, if we can't use zero
> for induc_val, use initial_def. */
> if (STMT_VINFO_REDUC_TYPE (reduc_info) == INTEGER_INDUC_COND_REDUCTION)
> @@ -5558,9 +5554,7 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> for MIN and MAX reduction, for example. */
> if (!neutral_op)
> {
> - tree scalar_value
> - = PHI_ARG_DEF_FROM_EDGE (orig_phis[i]->stmt,
> - loop_preheader_edge (loop));
> + tree scalar_value = vect_phi_initial_value (orig_phis[i]);
> scalar_value = gimple_convert (&seq, TREE_TYPE (vectype),
> scalar_value);
> vector_identity = gimple_build_vector_from_val (&seq, vectype,
> @@ -6752,10 +6746,7 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
> else if (cond_reduc_dt == vect_constant_def)
> {
> enum vect_def_type cond_initial_dt;
> - tree cond_initial_val
> - = PHI_ARG_DEF_FROM_EDGE (reduc_def_phi, loop_preheader_edge
> (loop));
> -
> - gcc_assert (cond_reduc_val != NULL_TREE);
> + tree cond_initial_val = vect_phi_initial_value (reduc_def_phi);
> vect_is_simple_use (cond_initial_val, loop_vinfo, &cond_initial_dt);
> if (cond_initial_dt == vect_constant_def
> && types_compatible_p (TREE_TYPE (cond_initial_val),
> @@ -7528,8 +7519,7 @@ vect_transform_cycle_phi (loop_vec_info loop_vinfo,
> {
> /* Get at the scalar def before the loop, that defines the initial
> value of the reduction variable. */
> - tree initial_def = PHI_ARG_DEF_FROM_EDGE (phi,
> - loop_preheader_edge (loop));
> + tree initial_def = vect_phi_initial_value (phi);
> /* Optimize: if initial_def is for REDUC_MAX smaller than the base
> and we can't use zero for induc_val, use initial_def. Similarly
> for REDUC_MIN and initial_def larger than the base. */
> @@ -8175,8 +8165,7 @@ vectorizable_induction (loop_vec_info loop_vinfo,
> return true;
> }
>
> - init_expr = PHI_ARG_DEF_FROM_EDGE (phi,
> - loop_preheader_edge (iv_loop));
> + init_expr = vect_phi_initial_value (phi);
>
> gimple_seq stmts = NULL;
> if (!nested_in_vect_loop)
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index fa28336d429..e2fd3609fee 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -27,7 +27,7 @@ typedef class _stmt_vec_info *stmt_vec_info;
> #include "tree-hash-traits.h"
> #include "target.h"
> #include "internal-fn.h"
> -
> +#include "tree-ssa-operands.h"
>
> /* Used for naming of new temporaries. */
> enum vect_var_kind {
> @@ -1369,6 +1369,25 @@ nested_in_vect_loop_p (class loop *loop, stmt_vec_info
> stmt_info)
> && (loop->inner == (gimple_bb (stmt_info->stmt))->loop_father));
> }
>
> +/* PHI is either a scalar reduction phi or a scalar induction phi.
> + Return the initial value of the variable on entry to the containing
> + loop. */
> +
> +static inline tree
> +vect_phi_initial_value (gphi *phi)
> +{
> + basic_block bb = gimple_bb (phi);
> + edge pe = loop_preheader_edge (bb->loop_father);
> + gcc_assert (pe->dest == bb);
> + return PHI_ARG_DEF_FROM_EDGE (phi, pe);
> +}
> +
> +static inline tree
> +vect_phi_initial_value (stmt_vec_info stmt_info)
> +{
> + return vect_phi_initial_value (as_a <gphi *> (stmt_info->stmt));
> +}
> +
> /* Return true if STMT_INFO should produce a vector mask type rather than
> a normal nonmask type. */
>
