On Thu, Jul 26, 2018 at 12:55 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Richard Biener <richard.guent...@gmail.com> writes:
> > On Tue, Jul 24, 2018 at 12:08 PM Richard Sandiford
> > <richard.sandif...@arm.com> wrote:
> >>
> >> Alignment information is really a property of a stmt_vec_info
> >> (and the way we want to vectorise it) rather than the original scalar dr.
> >> I think that was true even before the recent dr sharing.
> >
> > But that is only so as long as we handle only stmts with a single DR.
> > In reality alignment info _is_ a property of the DR and not of the stmt.
> >
> > So you're doing a shortcut here, shouldn't we rename
> > dr_misalignment to stmt_dr_misalignment then?
> >
> > Otherwise I don't see how this makes sense semantically.
>
> OK, the patch below takes a different approach, suggested in the
> 38/46 thread. The idea is to make dr_aux link back to both the scalar
> data_reference and the containing stmt_vec_info, so that it becomes a
> lookup-free key for a vectorisable reference.
>
> The data_reference link is just STMT_VINFO_DATA_REF, moved from
> _stmt_vec_info. The stmt pointer is a new field and always tracks
> the current stmt_vec_info for the reference (which might be a pattern
> stmt or the original stmt).
>
> Then 38/40 can use dr_aux instead of data_reference (compared to current
> sources) and instead of stmt_vec_info (compared to the original series).
> This still avoids the repeated lookups that the series is trying to avoid.
>
> The patch also makes the dr_aux in the current (possibly pattern) stmt
> be the one that counts, rather than have the information stay with the
> original DR_STMT. A new macro (STMT_VINFO_DR_INFO) gives this
> information for a given stmt_vec_info.
>
> The changes together should make it easier to have multiple dr_auxs
> in a single statement.
I like this.
OK.
Richard.
> Thanks,
> Richard
>
>
> 2018-07-26 Richard Sandiford <richard.sandif...@arm.com>
>
> gcc/
> * tree-vectorizer.h (vec_info::move_dr): New member function.
> (dataref_aux): Rename to...
> (dr_vec_info): ...this and add "dr" and "stmt" fields.
> (_stmt_vec_info::dr_aux): Update accordingly.
> (_stmt_vec_info::data_ref_info): Delete.
> (STMT_VINFO_GROUPED_ACCESS, DR_GROUP_FIRST_ELEMENT)
> (DR_GROUP_NEXT_ELEMENT, DR_GROUP_SIZE, DR_GROUP_STORE_COUNT)
> (DR_GROUP_GAP, DR_GROUP_SAME_DR_STMT, REDUC_GROUP_FIRST_ELEMENT):
> (REDUC_GROUP_NEXT_ELEMENT, REDUC_GROUP_SIZE): Use dr_aux.dr instead
> of data_ref.
> (STMT_VINFO_DATA_REF): Likewise. Turn into an lvalue.
> (STMT_VINFO_DR_INFO): New macro.
> (DR_VECT_AUX): Use STMT_VINFO_DR_INKFO and vect_dr_stmt.
> (set_dr_misalignment): Update after rename of dataref_aux.
> (vect_dr_stmt): Move earlier in file. Return dr_aux.stmt.
> * tree-vect-stmts.c (new_stmt_vec_info): Remove redundant
> initialization of STMT_VINFO_DATA_REF.
> * tree-vectorizer.c (vec_info::move_dr): New function.
> * tree-vect-patterns.c (vect_recog_bool_pattern)
> (vect_recog_mask_conversion_pattern)
> (vect_recog_gather_scatter_pattern): Use it.
> * tree-vect-data-refs.c (vect_analyze_data_refs): Initialize
> the "dr" and "stmt" fields of dr_vec_info instead of
> STMT_VINFO_DATA_REF.
>
> Index: gcc/tree-vectorizer.h
> ===================================================================
> --- gcc/tree-vectorizer.h 2018-07-26 11:30:55.000000000 +0100
> +++ gcc/tree-vectorizer.h 2018-07-26 11:30:56.197256524 +0100
> @@ -240,6 +240,7 @@ struct vec_info {
> stmt_vec_info lookup_stmt (gimple *);
> stmt_vec_info lookup_def (tree);
> stmt_vec_info lookup_single_use (tree);
> + void move_dr (stmt_vec_info, stmt_vec_info);
>
> /* The type of vectorization. */
> vec_kind kind;
> @@ -767,7 +768,11 @@ enum vect_memory_access_type {
> VMAT_GATHER_SCATTER
> };
>
> -struct dataref_aux {
> +struct dr_vec_info {
> + /* The data reference itself. */
> + data_reference *dr;
> + /* The statement that contains the data reference. */
> + stmt_vec_info stmt;
> /* The misalignment in bytes of the reference, or -1 if not known. */
> int misalignment;
> /* The byte alignment that we'd ideally like the reference to have,
> @@ -818,11 +823,7 @@ struct _stmt_vec_info {
> data-ref (array/pointer/struct access). A GIMPLE stmt is expected to
> have
> at most one such data-ref. */
>
> - /* Information about the data-ref (access function, etc),
> - relative to the inner-most containing loop. */
> - struct data_reference *data_ref_info;
> -
> - dataref_aux dr_aux;
> + dr_vec_info dr_aux;
>
> /* Information about the data-ref relative to this loop
> nest (the loop that is being considered for vectorization). */
> @@ -996,7 +997,7 @@ #define STMT_VINFO_LIVE_P(S)
> #define STMT_VINFO_VECTYPE(S) (S)->vectype
> #define STMT_VINFO_VEC_STMT(S) (S)->vectorized_stmt
> #define STMT_VINFO_VECTORIZABLE(S) (S)->vectorizable
> -#define STMT_VINFO_DATA_REF(S) (S)->data_ref_info
> +#define STMT_VINFO_DATA_REF(S) ((S)->dr_aux.dr + 0)
> #define STMT_VINFO_GATHER_SCATTER_P(S) (S)->gather_scatter_p
> #define STMT_VINFO_STRIDED_P(S) (S)->strided_p
> #define STMT_VINFO_MEMORY_ACCESS_TYPE(S) (S)->memory_access_type
> @@ -1017,13 +1018,17 @@ #define STMT_VINFO_DR_OFFSET_ALIGNMENT(S
> #define STMT_VINFO_DR_STEP_ALIGNMENT(S) \
> (S)->dr_wrt_vec_loop.step_alignment
>
> +#define STMT_VINFO_DR_INFO(S) \
> + (gcc_checking_assert ((S)->dr_aux.stmt == (S)), &(S)->dr_aux)
> +
> #define STMT_VINFO_IN_PATTERN_P(S) (S)->in_pattern_p
> #define STMT_VINFO_RELATED_STMT(S) (S)->related_stmt
> #define STMT_VINFO_PATTERN_DEF_SEQ(S) (S)->pattern_def_seq
> #define STMT_VINFO_SAME_ALIGN_REFS(S) (S)->same_align_refs
> #define STMT_VINFO_SIMD_CLONE_INFO(S) (S)->simd_clone_info
> #define STMT_VINFO_DEF_TYPE(S) (S)->def_type
> -#define STMT_VINFO_GROUPED_ACCESS(S) ((S)->data_ref_info &&
> DR_GROUP_FIRST_ELEMENT(S))
> +#define STMT_VINFO_GROUPED_ACCESS(S) \
> + ((S)->dr_aux.dr && DR_GROUP_FIRST_ELEMENT(S))
> #define STMT_VINFO_LOOP_PHI_EVOLUTION_BASE_UNCHANGED(S)
> (S)->loop_phi_evolution_base_unchanged
> #define STMT_VINFO_LOOP_PHI_EVOLUTION_PART(S) (S)->loop_phi_evolution_part
> #define STMT_VINFO_MIN_NEG_DIST(S) (S)->min_neg_dist
> @@ -1031,16 +1036,25 @@ #define STMT_VINFO_NUM_SLP_USES(S) (S)->
> #define STMT_VINFO_REDUC_TYPE(S) (S)->reduc_type
> #define STMT_VINFO_REDUC_DEF(S) (S)->reduc_def
>
> -#define DR_GROUP_FIRST_ELEMENT(S) (gcc_checking_assert
> ((S)->data_ref_info), (S)->first_element)
> -#define DR_GROUP_NEXT_ELEMENT(S) (gcc_checking_assert
> ((S)->data_ref_info), (S)->next_element)
> -#define DR_GROUP_SIZE(S) (gcc_checking_assert
> ((S)->data_ref_info), (S)->size)
> -#define DR_GROUP_STORE_COUNT(S) (gcc_checking_assert
> ((S)->data_ref_info), (S)->store_count)
> -#define DR_GROUP_GAP(S) (gcc_checking_assert
> ((S)->data_ref_info), (S)->gap)
> -#define DR_GROUP_SAME_DR_STMT(S) (gcc_checking_assert
> ((S)->data_ref_info), (S)->same_dr_stmt)
> -
> -#define REDUC_GROUP_FIRST_ELEMENT(S) (gcc_checking_assert
> (!(S)->data_ref_info), (S)->first_element)
> -#define REDUC_GROUP_NEXT_ELEMENT(S) (gcc_checking_assert
> (!(S)->data_ref_info), (S)->next_element)
> -#define REDUC_GROUP_SIZE(S) (gcc_checking_assert
> (!(S)->data_ref_info), (S)->size)
> +#define DR_GROUP_FIRST_ELEMENT(S) \
> + (gcc_checking_assert ((S)->dr_aux.dr), (S)->first_element)
> +#define DR_GROUP_NEXT_ELEMENT(S) \
> + (gcc_checking_assert ((S)->dr_aux.dr), (S)->next_element)
> +#define DR_GROUP_SIZE(S) \
> + (gcc_checking_assert ((S)->dr_aux.dr), (S)->size)
> +#define DR_GROUP_STORE_COUNT(S) \
> + (gcc_checking_assert ((S)->dr_aux.dr), (S)->store_count)
> +#define DR_GROUP_GAP(S) \
> + (gcc_checking_assert ((S)->dr_aux.dr), (S)->gap)
> +#define DR_GROUP_SAME_DR_STMT(S) \
> + (gcc_checking_assert ((S)->dr_aux.dr), (S)->same_dr_stmt)
> +
> +#define REDUC_GROUP_FIRST_ELEMENT(S) \
> + (gcc_checking_assert (!(S)->dr_aux.dr), (S)->first_element)
> +#define REDUC_GROUP_NEXT_ELEMENT(S) \
> + (gcc_checking_assert (!(S)->dr_aux.dr), (S)->next_element)
> +#define REDUC_GROUP_SIZE(S) \
> + (gcc_checking_assert (!(S)->dr_aux.dr), (S)->size)
>
> #define STMT_VINFO_RELEVANT_P(S) ((S)->relevant !=
> vect_unused_in_scope)
>
> @@ -1048,7 +1062,7 @@ #define HYBRID_SLP_STMT(S)
> #define PURE_SLP_STMT(S) ((S)->slp_type == pure_slp)
> #define STMT_SLP_TYPE(S) (S)->slp_type
>
> -#define DR_VECT_AUX(dr) (&vinfo_for_stmt (DR_STMT (dr))->dr_aux)
> +#define DR_VECT_AUX(dr) (STMT_VINFO_DR_INFO (vect_dr_stmt (dr)))
>
> #define VECT_MAX_COST 1000
>
> @@ -1259,6 +1273,20 @@ add_stmt_costs (void *data, stmt_vector_
> cost->misalign, cost->where);
> }
>
> +/* Return the stmt DR is in. For DR_STMT that have been replaced by
> + a pattern this returns the corresponding pattern stmt. Otherwise
> + DR_STMT is returned. */
> +
> +inline stmt_vec_info
> +vect_dr_stmt (data_reference *dr)
> +{
> + gimple *stmt = DR_STMT (dr);
> + stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> + /* DR_STMT should never refer to a stmt in a pattern replacement. */
> + gcc_checking_assert (!is_pattern_stmt_p (stmt_info));
> + return stmt_info->dr_aux.stmt;
> +}
> +
> /*-----------------------------------------------------------------*/
> /* Info on data references alignment. */
> /*-----------------------------------------------------------------*/
> @@ -1268,8 +1296,7 @@ #define DR_MISALIGNMENT_UNINITIALIZED (-
> inline void
> set_dr_misalignment (struct data_reference *dr, int val)
> {
> - dataref_aux *data_aux = DR_VECT_AUX (dr);
> - data_aux->misalignment = val;
> + DR_VECT_AUX (dr)->misalignment = val;
> }
>
> inline int
> @@ -1336,22 +1363,6 @@ vect_dr_behavior (data_reference *dr)
> return &STMT_VINFO_DR_WRT_VEC_LOOP (stmt_info);
> }
>
> -/* Return the stmt DR is in. For DR_STMT that have been replaced by
> - a pattern this returns the corresponding pattern stmt. Otherwise
> - DR_STMT is returned. */
> -
> -inline stmt_vec_info
> -vect_dr_stmt (data_reference *dr)
> -{
> - gimple *stmt = DR_STMT (dr);
> - stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
> - if (STMT_VINFO_IN_PATTERN_P (stmt_info))
> - return STMT_VINFO_RELATED_STMT (stmt_info);
> - /* DR_STMT should never refer to a stmt in a pattern replacement. */
> - gcc_checking_assert (!STMT_VINFO_RELATED_STMT (stmt_info));
> - return stmt_info;
> -}
> -
> /* Return true if the vect cost model is unlimited. */
> static inline bool
> unlimited_cost_model (loop_p loop)
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c 2018-07-26 11:30:55.000000000 +0100
> +++ gcc/tree-vect-stmts.c 2018-07-26 11:30:56.197256524 +0100
> @@ -9872,7 +9872,6 @@ new_stmt_vec_info (gimple *stmt, vec_inf
> STMT_VINFO_VECTORIZABLE (res) = true;
> STMT_VINFO_IN_PATTERN_P (res) = false;
> STMT_VINFO_PATTERN_DEF_SEQ (res) = NULL;
> - STMT_VINFO_DATA_REF (res) = NULL;
> STMT_VINFO_VEC_REDUCTION_TYPE (res) = TREE_CODE_REDUCTION;
> STMT_VINFO_VEC_CONST_COND_REDUC_CODE (res) = ERROR_MARK;
>
> Index: gcc/tree-vectorizer.c
> ===================================================================
> --- gcc/tree-vectorizer.c 2018-07-26 11:30:55.000000000 +0100
> +++ gcc/tree-vectorizer.c 2018-07-26 11:30:56.197256524 +0100
> @@ -562,6 +562,21 @@ vec_info::lookup_single_use (tree lhs)
> return NULL;
> }
>
> +/* Record that NEW_STMT_INFO now implements the same data reference
> + as OLD_STMT_INFO. */
> +
> +void
> +vec_info::move_dr (stmt_vec_info new_stmt_info, stmt_vec_info old_stmt_info)
> +{
> + gcc_assert (!is_pattern_stmt_p (old_stmt_info));
> + STMT_VINFO_DR_INFO (old_stmt_info)->stmt = new_stmt_info;
> + new_stmt_info->dr_aux = old_stmt_info->dr_aux;
> + STMT_VINFO_DR_WRT_VEC_LOOP (new_stmt_info)
> + = STMT_VINFO_DR_WRT_VEC_LOOP (old_stmt_info);
> + STMT_VINFO_GATHER_SCATTER_P (new_stmt_info)
> + = STMT_VINFO_GATHER_SCATTER_P (old_stmt_info);
> +}
> +
> /* A helper function to free scev and LOOP niter information, as well as
> clear loop constraint LOOP_C_FINITE. */
>
> Index: gcc/tree-vect-patterns.c
> ===================================================================
> --- gcc/tree-vect-patterns.c 2018-07-26 11:30:55.000000000 +0100
> +++ gcc/tree-vect-patterns.c 2018-07-26 11:30:56.193256600 +0100
> @@ -3828,10 +3828,7 @@ vect_recog_bool_pattern (stmt_vec_info s
> }
> pattern_stmt = gimple_build_assign (lhs, SSA_NAME, rhs);
> pattern_stmt_info = vinfo->add_stmt (pattern_stmt);
> - STMT_VINFO_DATA_REF (pattern_stmt_info)
> - = STMT_VINFO_DATA_REF (stmt_vinfo);
> - STMT_VINFO_DR_WRT_VEC_LOOP (pattern_stmt_info)
> - = STMT_VINFO_DR_WRT_VEC_LOOP (stmt_vinfo);
> + vinfo->move_dr (pattern_stmt_info, stmt_vinfo);
> *type_out = vectype;
> vect_pattern_detected ("vect_recog_bool_pattern", last_stmt);
>
> @@ -3954,14 +3951,7 @@ vect_recog_mask_conversion_pattern (stmt
>
> pattern_stmt_info = vinfo->add_stmt (pattern_stmt);
> if (STMT_VINFO_DATA_REF (stmt_vinfo))
> - {
> - STMT_VINFO_DATA_REF (pattern_stmt_info)
> - = STMT_VINFO_DATA_REF (stmt_vinfo);
> - STMT_VINFO_DR_WRT_VEC_LOOP (pattern_stmt_info)
> - = STMT_VINFO_DR_WRT_VEC_LOOP (stmt_vinfo);
> - STMT_VINFO_GATHER_SCATTER_P (pattern_stmt_info)
> - = STMT_VINFO_GATHER_SCATTER_P (stmt_vinfo);
> - }
> + vinfo->move_dr (pattern_stmt_info, stmt_vinfo);
>
> *type_out = vectype1;
> vect_pattern_detected ("vect_recog_mask_conversion_pattern",
> last_stmt);
> @@ -4283,11 +4273,7 @@ vect_recog_gather_scatter_pattern (stmt_
> /* Copy across relevant vectorization info and associate DR with the
> new pattern statement instead of the original statement. */
> stmt_vec_info pattern_stmt_info = loop_vinfo->add_stmt (pattern_stmt);
> - STMT_VINFO_DATA_REF (pattern_stmt_info) = dr;
> - STMT_VINFO_DR_WRT_VEC_LOOP (pattern_stmt_info)
> - = STMT_VINFO_DR_WRT_VEC_LOOP (stmt_info);
> - STMT_VINFO_GATHER_SCATTER_P (pattern_stmt_info)
> - = STMT_VINFO_GATHER_SCATTER_P (stmt_info);
> + loop_vinfo->move_dr (pattern_stmt_info, stmt_info);
>
> tree vectype = STMT_VINFO_VECTYPE (stmt_info);
> *type_out = vectype;
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> --- gcc/tree-vect-data-refs.c 2018-07-26 11:30:55.000000000 +0100
> +++ gcc/tree-vect-data-refs.c 2018-07-26 11:30:56.193256600 +0100
> @@ -4120,7 +4120,10 @@ vect_analyze_data_refs (vec_info *vinfo,
> poly_uint64 vf;
>
> gcc_assert (DR_REF (dr));
> - stmt_vec_info stmt_info = vect_dr_stmt (dr);
> + stmt_vec_info stmt_info = vinfo->lookup_stmt (DR_STMT (dr));
> + gcc_assert (!stmt_info->dr_aux.dr);
> + stmt_info->dr_aux.dr = dr;
> + stmt_info->dr_aux.stmt = stmt_info;
>
> /* Check that analysis of the data-ref succeeded. */
> if (!DR_BASE_ADDRESS (dr) || !DR_OFFSET (dr) || !DR_INIT (dr)
> @@ -4292,9 +4295,6 @@ vect_analyze_data_refs (vec_info *vinfo,
> }
> }
>
> - gcc_assert (!STMT_VINFO_DATA_REF (stmt_info));
> - STMT_VINFO_DATA_REF (stmt_info) = dr;
> -
> /* Set vectype for STMT. */
> scalar_type = TREE_TYPE (DR_REF (dr));
> STMT_VINFO_VECTYPE (stmt_info)
