On Wed, Jul 25, 2018 at 1:09 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Richard Biener <richard.guent...@gmail.com> writes:
> > On Tue, Jul 24, 2018 at 12:07 PM Richard Sandiford
> > <richard.sandif...@arm.com> wrote:
> >>
> >> This patch adds a pattern_stmt_p field to stmt_vec_info, so that it's
> >> possible to tell whether the statement is a pattern statement without
> >> referring to other statements. The new field goes in what was
> >> previously a hole in the structure, so the size is the same as before.
> >
> > Not sure what the advantage is? is_pattern_stmt_p () looks nicer
> > than ->is_pattern_p
>
> I can keep the function wrapper if you prefer that. But having a
> statement "know" whether it's a pattern stmt makes things like
> freeing stmt_vec_infos simpler (see later patches in the series).
Ah, ok.
> It should also be cheaper to test, but that's much more minor.
So please keep the wrapper.
I guess at some point we should decide what to do with all
the STMT_VINFO_ macros (and the others, {LOOP,BB}_ stuff
is already used inconsistently).
Richard.
> Thanks,
> Richard
>
> >
> >>
> >> 2018-07-24 Richard Sandiford <richard.sandif...@arm.com>
> >>
> >> gcc/
> >> * tree-vectorizer.h (_stmt_vec_info::pattern_stmt_p): New field.
> >> (is_pattern_stmt_p): Delete.
> >> * tree-vect-patterns.c (vect_init_pattern_stmt): Set pattern_stmt_p
> >> on pattern statements.
> >> (vect_split_statement, vect_mark_pattern_stmts): Use the new
> >> pattern_stmt_p field instead of is_pattern_stmt_p.
> >> * tree-vect-data-refs.c (vect_preserves_scalar_order_p): Likewise.
> >> * tree-vect-loop.c (vectorizable_live_operation): Likewise.
> >> * tree-vect-slp.c (vect_build_slp_tree_2): Likewise.
> >> (vect_find_last_scalar_stmt_in_slp, vect_remove_slp_scalar_calls)
> >> (vect_schedule_slp): Likewise.
> >> * tree-vect-stmts.c (vect_mark_stmts_to_be_vectorized): Likewise.
> >> (vectorizable_call, vectorizable_simd_clone_call,
> >> vectorizable_shift)
> >> (vectorizable_store, vect_remove_stores): Likewise.
> >>
> >> Index: gcc/tree-vectorizer.h
> >> ===================================================================
> >> --- gcc/tree-vectorizer.h 2018-07-24 10:23:56.440544995 +0100
> >> +++ gcc/tree-vectorizer.h 2018-07-24 10:24:02.364492386 +0100
> >> @@ -791,6 +791,12 @@ struct _stmt_vec_info {
> >> /* Stmt is part of some pattern (computation idiom) */
> >> bool in_pattern_p;
> >>
> >> + /* True if the statement was created during pattern recognition as
> >> + part of the replacement for RELATED_STMT. This implies that the
> >> + statement isn't part of any basic block, although for convenience
> >> + its gimple_bb is the same as for RELATED_STMT. */
> >> + bool pattern_stmt_p;
> >> +
> >> /* Is this statement vectorizable or should it be skipped in (partial)
> >> vectorization. */
> >> bool vectorizable;
> >> @@ -1151,16 +1157,6 @@ get_later_stmt (stmt_vec_info stmt1_info
> >> return stmt2_info;
> >> }
> >>
> >> -/* Return TRUE if a statement represented by STMT_INFO is a part of a
> >> - pattern. */
> >> -
> >> -static inline bool
> >> -is_pattern_stmt_p (stmt_vec_info stmt_info)
> >> -{
> >> - stmt_vec_info related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);
> >> - return related_stmt_info && STMT_VINFO_IN_PATTERN_P (related_stmt_info);
> >> -}
> >> -
> >> /* Return true if BB is a loop header. */
> >>
> >> static inline bool
> >> Index: gcc/tree-vect-patterns.c
> >> ===================================================================
> >> --- gcc/tree-vect-patterns.c 2018-07-24 10:23:59.408518638 +0100
> >> +++ gcc/tree-vect-patterns.c 2018-07-24 10:24:02.360492422 +0100
> >> @@ -108,6 +108,7 @@ vect_init_pattern_stmt (gimple *pattern_
> >> pattern_stmt_info = orig_stmt_info->vinfo->add_stmt (pattern_stmt);
> >> gimple_set_bb (pattern_stmt, gimple_bb (orig_stmt_info->stmt));
> >>
> >> + pattern_stmt_info->pattern_stmt_p = true;
> >> STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info;
> >> STMT_VINFO_DEF_TYPE (pattern_stmt_info)
> >> = STMT_VINFO_DEF_TYPE (orig_stmt_info);
> >> @@ -630,7 +631,7 @@ vect_recog_temp_ssa_var (tree type, gimp
> >> vect_split_statement (stmt_vec_info stmt2_info, tree new_rhs,
> >> gimple *stmt1, tree vectype)
> >> {
> >> - if (is_pattern_stmt_p (stmt2_info))
> >> + if (stmt2_info->pattern_stmt_p)
> >> {
> >> /* STMT2_INFO is part of a pattern. Get the statement to which
> >> the pattern is attached. */
> >> @@ -4726,7 +4727,7 @@ vect_mark_pattern_stmts (stmt_vec_info o
> >> gimple *def_seq = STMT_VINFO_PATTERN_DEF_SEQ (orig_stmt_info);
> >>
> >> gimple *orig_pattern_stmt = NULL;
> >> - if (is_pattern_stmt_p (orig_stmt_info))
> >> + if (orig_stmt_info->pattern_stmt_p)
> >> {
> >> /* We're replacing a statement in an existing pattern definition
> >> sequence. */
> >> Index: gcc/tree-vect-data-refs.c
> >> ===================================================================
> >> --- gcc/tree-vect-data-refs.c 2018-07-24 10:23:53.204573732 +0100
> >> +++ gcc/tree-vect-data-refs.c 2018-07-24 10:24:02.356492457 +0100
> >> @@ -212,9 +212,9 @@ vect_preserves_scalar_order_p (stmt_vec_
> >> (but could happen later) while reads will happen no later than their
> >> current position (but could happen earlier). Reordering is therefore
> >> only possible if the first access is a write. */
> >> - if (is_pattern_stmt_p (stmtinfo_a))
> >> + if (stmtinfo_a->pattern_stmt_p)
> >> stmtinfo_a = STMT_VINFO_RELATED_STMT (stmtinfo_a);
> >> - if (is_pattern_stmt_p (stmtinfo_b))
> >> + if (stmtinfo_b->pattern_stmt_p)
> >> stmtinfo_b = STMT_VINFO_RELATED_STMT (stmtinfo_b);
> >> stmt_vec_info earlier_stmt_info = get_earlier_stmt (stmtinfo_a,
> > stmtinfo_b);
> >> return !DR_IS_WRITE (STMT_VINFO_DATA_REF (earlier_stmt_info));
> >> Index: gcc/tree-vect-loop.c
> >> ===================================================================
> >> --- gcc/tree-vect-loop.c 2018-07-24 10:23:56.436545030 +0100
> >> +++ gcc/tree-vect-loop.c 2018-07-24 10:24:02.360492422 +0100
> >> @@ -7907,7 +7907,7 @@ vectorizable_live_operation (stmt_vec_in
> >> }
> >>
> >> /* If stmt has a related stmt, then use that for getting the lhs. */
> >> - gimple *stmt = (is_pattern_stmt_p (stmt_info)
> >> + gimple *stmt = (stmt_info->pattern_stmt_p
> >> ? STMT_VINFO_RELATED_STMT (stmt_info)->stmt
> >> : stmt_info->stmt);
> >>
> >> Index: gcc/tree-vect-slp.c
> >> ===================================================================
> >> --- gcc/tree-vect-slp.c 2018-07-24 10:23:53.204573732 +0100
> >> +++ gcc/tree-vect-slp.c 2018-07-24 10:24:02.360492422 +0100
> >> @@ -376,7 +376,7 @@ vect_get_and_check_slp_defs (vec_info *v
> >> /* Check if DEF_STMT_INFO is a part of a pattern in LOOP and get
> >> the def stmt from the pattern. Check that all the stmts of the
> >> node are in the pattern. */
> >> - if (def_stmt_info && is_pattern_stmt_p (def_stmt_info))
> >> + if (def_stmt_info && def_stmt_info->pattern_stmt_p)
> >> {
> >> pattern = true;
> >> if (!first && !oprnd_info->first_pattern
> >> @@ -1315,7 +1315,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,
> >> /* ??? Rejecting patterns this way doesn't work. We'd have
> >> to
> >> do extra work to cancel the pattern so the uses see the
> >> scalar version. */
> >> - && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))
> >> + && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)
> >> {
> >> slp_tree grandchild;
> >>
> >> @@ -1359,7 +1359,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,
> >> /* ??? Rejecting patterns this way doesn't work. We'd have to
> >> do extra work to cancel the pattern so the uses see the
> >> scalar version. */
> >> - && !is_pattern_stmt_p (stmt_info))
> >> + && !stmt_info->pattern_stmt_p)
> >> {
> >> dump_printf_loc (MSG_NOTE, vect_location,
> >> "Building vector operands from scalars\n");
> >> @@ -1486,7 +1486,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,
> >> /* ??? Rejecting patterns this way doesn't work. We'd
> >> have
> >> to do extra work to cancel the pattern so the uses see the
> >> scalar version. */
> >> - && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))
> >> + && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)
> >> {
> >> unsigned int j;
> >> slp_tree grandchild;
> >> @@ -1848,7 +1848,7 @@ vect_find_last_scalar_stmt_in_slp (slp_t
> >>
> >> for (int i = 0; SLP_TREE_SCALAR_STMTS (node).iterate (i, &stmt_vinfo);
> >> i++)
> >> {
> >> - if (is_pattern_stmt_p (stmt_vinfo))
> >> + if (stmt_vinfo->pattern_stmt_p)
> >> stmt_vinfo = STMT_VINFO_RELATED_STMT (stmt_vinfo);
> >> last = last ? get_later_stmt (stmt_vinfo, last) : stmt_vinfo;
> >> }
> >> @@ -4044,8 +4044,7 @@ vect_remove_slp_scalar_calls (slp_tree n
> >> gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt);
> >> if (!stmt || gimple_bb (stmt) == NULL)
> >> continue;
> >> - if (is_pattern_stmt_p (stmt_info)
> >> - || !PURE_SLP_STMT (stmt_info))
> >> + if (stmt_info->pattern_stmt_p || !PURE_SLP_STMT (stmt_info))
> >> continue;
> >> lhs = gimple_call_lhs (stmt);
> >> new_stmt = gimple_build_assign (lhs, build_zero_cst (TREE_TYPE
> >> (lhs)));
> >> @@ -4106,7 +4105,7 @@ vect_schedule_slp (vec_info *vinfo)
> >> if (!STMT_VINFO_DATA_REF (store_info))
> >> break;
> >>
> >> - if (is_pattern_stmt_p (store_info))
> >> + if (store_info->pattern_stmt_p)
> >> store_info = STMT_VINFO_RELATED_STMT (store_info);
> >> /* Free the attached stmt_vec_info and remove the stmt. */
> >> gsi = gsi_for_stmt (store_info);
> >> Index: gcc/tree-vect-stmts.c
> >> ===================================================================
> >> --- gcc/tree-vect-stmts.c 2018-07-24 10:23:56.440544995 +0100
> >> +++ gcc/tree-vect-stmts.c 2018-07-24 10:24:02.364492386 +0100
> >> @@ -731,7 +731,7 @@ vect_mark_stmts_to_be_vectorized (loop_v
> >> break;
> >> }
> >>
> >> - if (is_pattern_stmt_p (stmt_vinfo))
> >> + if (stmt_vinfo->pattern_stmt_p)
> >> {
> >> /* Pattern statements are not inserted into the code, so
> >> FOR_EACH_PHI_OR_STMT_USE optimizes their operands out, and we
> >> @@ -3623,7 +3623,7 @@ vectorizable_call (stmt_vec_info stmt_in
> >> if (slp_node)
> >> return true;
> >>
> >> - if (is_pattern_stmt_p (stmt_info))
> >> + if (stmt_info->pattern_stmt_p)
> >> stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);
> >> lhs = gimple_get_lhs (stmt_info->stmt);
> >>
> >> @@ -4362,7 +4362,7 @@ vectorizable_simd_clone_call (stmt_vec_i
> >> if (scalar_dest)
> >> {
> >> type = TREE_TYPE (scalar_dest);
> >> - if (is_pattern_stmt_p (stmt_info))
> >> + if (stmt_info->pattern_stmt_p)
> >> lhs = gimple_call_lhs (STMT_VINFO_RELATED_STMT (stmt_info)->stmt);
> >> else
> >> lhs = gimple_call_lhs (stmt);
> >> @@ -5552,7 +5552,7 @@ vectorizable_shift (stmt_vec_info stmt_i
> >> /* If the shift amount is computed by a pattern stmt we cannot
> >> use the scalar amount directly thus give up and use a vector
> >> shift. */
> >> - if (op1_def_stmt_info && is_pattern_stmt_p (op1_def_stmt_info))
> >> + if (op1_def_stmt_info && op1_def_stmt_info->pattern_stmt_p)
> >> scalar_shift_arg = false;
> >> }
> >> else
> >> @@ -6286,7 +6286,7 @@ vectorizable_store (stmt_vec_info stmt_i
> >> {
> >> tree scalar_dest = gimple_assign_lhs (assign);
> >> if (TREE_CODE (scalar_dest) == VIEW_CONVERT_EXPR
> >> - && is_pattern_stmt_p (stmt_info))
> >> + && stmt_info->pattern_stmt_p)
> >> scalar_dest = TREE_OPERAND (scalar_dest, 0);
> >> if (TREE_CODE (scalar_dest) != ARRAY_REF
> >> && TREE_CODE (scalar_dest) != BIT_FIELD_REF
> >> @@ -9839,7 +9839,7 @@ vect_remove_stores (stmt_vec_info first_
> >> while (next_stmt_info)
> >> {
> >> stmt_vec_info tmp = DR_GROUP_NEXT_ELEMENT (next_stmt_info);
> >> - if (is_pattern_stmt_p (next_stmt_info))
> >> + if (next_stmt_info->pattern_stmt_p)
> >> next_stmt_info = STMT_VINFO_RELATED_STMT (next_stmt_info);
> >> /* Free the attached stmt_vec_info and remove the stmt. */
> >> next_si = gsi_for_stmt (next_stmt_info->stmt);
