This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Clean up PURE_SLP_STMT handling
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>, richard dot sandiford at arm dot com
- Date: Mon, 23 May 2016 10:31:23 +0200
- Subject: Re: Clean up PURE_SLP_STMT handling
- Authentication-results: sourceware.org; auth=none
- References: <87a8jkeohz dot fsf at e105548-lin dot cambridge dot arm dot com>
On Fri, May 20, 2016 at 5:30 PM, Richard Sandiford
<richard.sandiford@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))