On October 19, 2019 5:06:40 PM GMT+02:00, Richard Sandiford
<richard.sandif...@arm.com> wrote:
>After the previous patch, it seems more natural to apply the
>PARAM_SLP_MAX_INSNS_IN_BB threshold as soon as we know what
>the region is, rather than delaying it to vect_slp_analyze_bb_1.
>(But rather than carve out the biggest region possible and then
>reject it, wouldn't it be better to stop when the region gets
>too big, so we at least have a chance of vectorising something?)
>
>It also seems more natural for vect_slp_bb_region to create the
>bb_vec_info itself rather than (a) having to pass bits of data down
>for the initialisation and (b) forcing vect_slp_analyze_bb_1 to free
>on every failure return.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Ok. But I wonder what the reason was for this limit? Dependency analysis was
greatly simplified, being no longer quadratic here. Can you check where the
limit originally came from? But indeed splitting the region makes more sense
then, but at dataref group boundaries I'd say.
Richard.
>Richard
>
>
>2019-10-19 Richard Sandiford <richard.sandif...@arm.com>
>
>gcc/
> * tree-vect-slp.c (vect_slp_analyze_bb_1): Take a bb_vec_info
> and return a boolean success value. Move the allocation and
> initialization of the bb_vec_info to...
> (vect_slp_bb_region): ...here. Update call accordingly.
> (vect_slp_bb): Apply PARAM_SLP_MAX_INSNS_IN_BB here rather
> than in vect_slp_analyze_bb_1.
>
>Index: gcc/tree-vect-slp.c
>===================================================================
>--- gcc/tree-vect-slp.c 2019-10-19 15:59:53.875379473 +0100
>+++ gcc/tree-vect-slp.c 2019-10-19 16:01:13.714824484 +0100
>@@ -2836,47 +2836,24 @@ vect_bb_vectorization_profitable_p (bb_v
> return true;
> }
>
>-/* Check if the basic block can be vectorized. Returns a bb_vec_info
>- if so and sets fatal to true if failure is independent of
>- current_vector_size. */
>-
>-static bb_vec_info
>-vect_slp_analyze_bb_1 (gimple_stmt_iterator region_begin,
>- gimple_stmt_iterator region_end,
>- vec<data_reference_p> datarefs, int n_stmts,
>- bool &fatal, vec_info_shared *shared)
>+/* Check if the region described by BB_VINFO can be vectorized,
>returning
>+ true if so. When returning false, set FATAL to true if the same
>failure
>+ would prevent vectorization at other vector sizes, false if it is
>still
>+ worth trying other sizes. N_STMTS is the number of statements in
>the
>+ region. */
>+
>+static bool
>+vect_slp_analyze_bb_1 (bb_vec_info bb_vinfo, int n_stmts, bool &fatal)
> {
> DUMP_VECT_SCOPE ("vect_slp_analyze_bb");
>
>- bb_vec_info bb_vinfo;
> slp_instance instance;
> int i;
> poly_uint64 min_vf = 2;
>- bool first_time_p = shared->datarefs.is_empty ();
>
> /* The first group of checks is independent of the vector size. */
> fatal = true;
>
>- if (n_stmts > PARAM_VALUE (PARAM_SLP_MAX_INSNS_IN_BB))
>- {
>- if (dump_enabled_p ())
>- dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>- "not vectorized: too many instructions in "
>- "basic block.\n");
>- free_data_refs (datarefs);
>- return NULL;
>- }
>-
>- bb_vinfo = new _bb_vec_info (region_begin, region_end, shared);
>- if (!bb_vinfo)
>- return NULL;
>-
>- BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
>- if (first_time_p)
>- bb_vinfo->shared->save_datarefs ();
>- else
>- bb_vinfo->shared->check_datarefs ();
>-
> /* Analyze the data references. */
>
> if (!vect_analyze_data_refs (bb_vinfo, &min_vf, NULL))
>@@ -2885,9 +2862,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: unhandled data-ref in basic "
> "block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> if (BB_VINFO_DATAREFS (bb_vinfo).length () < 2)
>@@ -2896,9 +2871,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: not enough data-refs in "
> "basic block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> if (!vect_analyze_data_ref_accesses (bb_vinfo))
>@@ -2907,9 +2880,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: unhandled data access in "
> "basic block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> /* If there are no grouped stores in the region there is no need
>@@ -2921,9 +2892,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: no grouped stores in "
> "basic block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> /* While the rest of the analysis below depends on it in some way. */
>@@ -2943,9 +2912,7 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> "not vectorized: failed to find SLP opportunities "
> "in basic block.\n");
> }
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> vect_record_base_alignments (bb_vinfo);
>@@ -2976,19 +2943,14 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> i++;
> }
> if (! BB_VINFO_SLP_INSTANCES (bb_vinfo).length ())
>- {
>- delete bb_vinfo;
>- return NULL;
>- }
>+ return false;
>
> if (!vect_slp_analyze_operations (bb_vinfo))
> {
> if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: bad operation in basic block.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> /* Cost model: check if the vectorization is worthwhile. */
>@@ -2999,16 +2961,13 @@ vect_slp_analyze_bb_1 (gimple_stmt_itera
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "not vectorized: vectorization is not "
> "profitable.\n");
>-
>- delete bb_vinfo;
>- return NULL;
>+ return false;
> }
>
> if (dump_enabled_p ())
> dump_printf_loc (MSG_NOTE, vect_location,
> "Basic block will be vectorized using SLP\n");
>-
>- return bb_vinfo;
>+ return true;
> }
>
> /* Subroutine of vect_slp_bb. Try to vectorize the statements between
>@@ -3037,9 +2996,16 @@ vect_slp_bb_region (gimple_stmt_iterator
> {
> bool vectorized = false;
> bool fatal = false;
>- bb_vinfo = vect_slp_analyze_bb_1 (region_begin, region_end,
>- datarefs, n_stmts, fatal, &shared);
>- if (bb_vinfo
>+ bb_vinfo = new _bb_vec_info (region_begin, region_end, &shared);
>+
>+ bool first_time_p = shared.datarefs.is_empty ();
>+ BB_VINFO_DATAREFS (bb_vinfo) = datarefs;
>+ if (first_time_p)
>+ bb_vinfo->shared->save_datarefs ();
>+ else
>+ bb_vinfo->shared->check_datarefs ();
>+
>+ if (vect_slp_analyze_bb_1 (bb_vinfo, n_stmts, fatal)
> && dbg_cnt (vect_slp))
> {
> if (dump_enabled_p ())
>@@ -3132,7 +3098,14 @@ vect_slp_bb (basic_block bb)
>
> gimple_stmt_iterator region_end = gsi;
>
>- if (vect_slp_bb_region (region_begin, region_end, datarefs,
>insns))
>+ if (insns > PARAM_VALUE (PARAM_SLP_MAX_INSNS_IN_BB))
>+ {
>+ if (dump_enabled_p ())
>+ dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>+ "not vectorized: too many instructions in "
>+ "basic block.\n");
>+ }
>+ else if (vect_slp_bb_region (region_begin, region_end, datarefs,
>insns))
> any_vectorized = true;
>
> if (gsi_end_p (region_end))
