On Fri, May 20, 2016 at 5:30 PM, Richard Sandiford <richard.sandif...@arm.com> wrote: > The vectorizable_* routines had many instances of: > > slp_node || PURE_SLP_STMT (stmt_info) > > which gives the misleading impression that we can have > !slp_node && PURE_SLP_STMT (stmt_info). In this context > it's really enough to test slp_node on its own.
Yeah, that's a cleanup opportunity since I changed the code last year to always pass down slp_node in all phases. > There are three cases: > > loop vectorisation only: > vectorizable_foo called only with !slp_node > > pure SLP: > vectorizable_foo called only with slp_node > > hybrid SLP: > (e.g. a vector that's used in SLP statements and also in a reduction) > - vectorizable_foo called once with slp_node for the SLP uses. > - vectorizable_foo called once with !slp_node for the non-SLP uses. > > Hybrid SLP isn't possible for stores, so I added an explicit assert > for that. > > I also made vectorizable_comparison static, to make it obvious that > no other callers outside tree-vect-stmts.c could use it with the > !slp && PURE_SLP_STMT combination. > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Ok. Thanks, Richard. > Thanks, > Richard > > > gcc/ > * tree-vectorizer.h (vectorizable_comparison): Delete. > * tree-vect-loop.c (vectorizable_reduction): Remove redundant > PURE_SLP_STMT check. > * tree-vect-stmts.c (vectorizable_call): Likewise. > (vectorizable_simd_clone_call): Likewise. > (vectorizable_conversion): Likewise. > (vectorizable_assignment): Likewise. > (vectorizable_shift): Likewise. > (vectorizable_operation): Likewise. > (vectorizable_load): Likewise. > (vectorizable_condition): Likewise. > (vectorizable_store): Likewise. Assert that we don't have > hybrid SLP. > (vectorizable_comparison): Make static. Remove redundant > PURE_SLP_STMT check. > (vect_transform_stmt): Assert that we always have an slp_node > if PURE_SLP_STMT. > > Index: gcc/tree-vectorizer.h > =================================================================== > --- gcc/tree-vectorizer.h > +++ gcc/tree-vectorizer.h > @@ -1004,8 +1004,6 @@ extern void vect_remove_stores (gimple *); > extern bool vect_analyze_stmt (gimple *, bool *, slp_tree); > extern bool vectorizable_condition (gimple *, gimple_stmt_iterator *, > gimple **, tree, int, slp_tree); > -extern bool vectorizable_comparison (gimple *, gimple_stmt_iterator *, > - gimple **, tree, int, slp_tree); > extern void vect_get_load_cost (struct data_reference *, int, bool, > unsigned int *, unsigned int *, > stmt_vector_for_cost *, > Index: gcc/tree-vect-loop.c > =================================================================== > --- gcc/tree-vect-loop.c > +++ gcc/tree-vect-loop.c > @@ -5594,7 +5594,7 @@ vectorizable_reduction (gimple *stmt, > gimple_stmt_iterator *gsi, > if (STMT_VINFO_LIVE_P (vinfo_for_stmt (reduc_def_stmt))) > return false; > > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else > ncopies = (LOOP_VINFO_VECT_FACTOR (loop_vinfo) > Index: gcc/tree-vect-stmts.c > =================================================================== > --- gcc/tree-vect-stmts.c > +++ gcc/tree-vect-stmts.c > @@ -2342,7 +2342,7 @@ vectorizable_call (gimple *gs, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > } > } > > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else if (modifier == NARROW && ifn == IFN_LAST) > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_out; > @@ -2792,7 +2792,7 @@ vectorizable_simd_clone_call (gimple *stmt, > gimple_stmt_iterator *gsi, > return false; > > /* FORNOW */ > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > return false; > > /* Process function arguments. */ > @@ -3761,7 +3761,7 @@ vectorizable_conversion (gimple *stmt, > gimple_stmt_iterator *gsi, > /* Multiple types in SLP are handled by creating the appropriate number of > vectorized stmts for each SLP node. Hence, NCOPIES is always 1 in > case of SLP. */ > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else if (modifier == NARROW) > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_out; > @@ -4242,7 +4242,7 @@ vectorizable_assignment (gimple *stmt, > gimple_stmt_iterator *gsi, > /* Multiple types in SLP are handled by creating the appropriate number of > vectorized stmts for each SLP node. Hence, NCOPIES is always 1 in > case of SLP. */ > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits; > @@ -4502,7 +4502,7 @@ vectorizable_shift (gimple *stmt, gimple_stmt_iterator > *gsi, > /* Multiple types in SLP are handled by creating the appropriate number of > vectorized stmts for each SLP node. Hence, NCOPIES is always 1 in > case of SLP. */ > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_in; > @@ -4933,7 +4933,7 @@ vectorizable_operation (gimple *stmt, > gimple_stmt_iterator *gsi, > /* Multiple types in SLP are handled by creating the appropriate number of > vectorized stmts for each SLP node. Hence, NCOPIES is always 1 in > case of SLP. */ > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits_in; > @@ -5250,6 +5250,10 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > && TREE_CODE (scalar_dest) != MEM_REF) > return false; > > + /* Cannot have hybrid store SLP -- that would mean storing to the > + same location twice. */ > + gcc_assert (slp == PURE_SLP_STMT (stmt_info)); > + > gcc_assert (gimple_assign_single_p (stmt)); > > tree vectype = STMT_VINFO_VECTYPE (stmt_info), rhs_vectype = NULL_TREE; > @@ -5261,7 +5265,7 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > /* Multiple types in SLP are handled by creating the appropriate number of > vectorized stmts for each SLP node. Hence, NCOPIES is always 1 in > case of SLP. */ > - if (slp || PURE_SLP_STMT (stmt_info)) > + if (slp) > ncopies = 1; > else > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits; > @@ -5343,9 +5347,7 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > grouped_store = true; > first_stmt = GROUP_FIRST_ELEMENT (stmt_info); > group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt)); > - if (!slp > - && !PURE_SLP_STMT (stmt_info) > - && !STMT_VINFO_STRIDED_P (stmt_info)) > + if (!slp && !STMT_VINFO_STRIDED_P (stmt_info)) > { > if (vect_store_lanes_supported (vectype, group_size)) > store_lanes_p = true; > @@ -5354,7 +5356,7 @@ vectorizable_store (gimple *stmt, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > } > > if (STMT_VINFO_STRIDED_P (stmt_info) > - && (slp || PURE_SLP_STMT (stmt_info)) > + && slp > && (group_size > nunits > || nunits % group_size != 0)) > { > @@ -6263,7 +6265,7 @@ vectorizable_load (gimple *stmt, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > /* Multiple types in SLP are handled by creating the appropriate number of > vectorized stmts for each SLP node. Hence, NCOPIES is always 1 in > case of SLP. */ > - if (slp || PURE_SLP_STMT (stmt_info)) > + if (slp) > ncopies = 1; > else > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits; > @@ -6316,9 +6318,7 @@ vectorizable_load (gimple *stmt, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > first_stmt = GROUP_FIRST_ELEMENT (stmt_info); > group_size = GROUP_SIZE (vinfo_for_stmt (first_stmt)); > > - if (!slp > - && !PURE_SLP_STMT (stmt_info) > - && !STMT_VINFO_STRIDED_P (stmt_info)) > + if (!slp && !STMT_VINFO_STRIDED_P (stmt_info)) > { > if (vect_load_lanes_supported (vectype, group_size)) > load_lanes_p = true; > @@ -6431,8 +6431,8 @@ vectorizable_load (gimple *stmt, gimple_stmt_iterator > *gsi, gimple **vec_stmt, > } > else if (STMT_VINFO_STRIDED_P (stmt_info)) > { > - if ((grouped_load > - && (slp || PURE_SLP_STMT (stmt_info))) > + if (grouped_load > + && slp > && (group_size > nunits > || nunits % group_size != 0)) > { > @@ -7510,7 +7510,7 @@ vectorizable_condition (gimple *stmt, > gimple_stmt_iterator *gsi, > int nunits = TYPE_VECTOR_SUBPARTS (vectype); > tree vectype1 = NULL_TREE, vectype2 = NULL_TREE; > > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits; > @@ -7716,7 +7716,7 @@ vectorizable_condition (gimple *stmt, > gimple_stmt_iterator *gsi, > > Return FALSE if not a vectorizable STMT, TRUE otherwise. */ > > -bool > +static bool > vectorizable_comparison (gimple *stmt, gimple_stmt_iterator *gsi, > gimple **vec_stmt, tree reduc_def, > slp_tree slp_node) > @@ -7750,7 +7750,7 @@ vectorizable_comparison (gimple *stmt, > gimple_stmt_iterator *gsi, > mask_type = vectype; > nunits = TYPE_VECTOR_SUBPARTS (vectype); > > - if (slp_node || PURE_SLP_STMT (stmt_info)) > + if (slp_node) > ncopies = 1; > else > ncopies = LOOP_VINFO_VECT_FACTOR (loop_vinfo) / nunits; > @@ -8160,6 +8160,7 @@ vect_transform_stmt (gimple *stmt, gimple_stmt_iterator > *gsi, > stmt_vec_info stmt_info = vinfo_for_stmt (stmt); > bool done; > > + gcc_assert (slp_node || !PURE_SLP_STMT (stmt_info)); > gimple *old_vec_stmt = STMT_VINFO_VEC_STMT (stmt_info); > > switch (STMT_VINFO_TYPE (stmt_info))