On Thu, Jul 8, 2021 at 2:44 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Vector reduction accumulators can differ in signedness from the
> final scalar result. The conversions to handle that case were
> distributed through vect_create_epilog_for_reduction; this patch
> does the conversion up-front instead.
But is that still correct? The conversions should be unsigned -> signed,
that is, we've performed the reduction in unsigned because we associated
the originally undefined overflow signed reduction. But the final
reduction of the vector lanes in the epilogue still needs to be done
unsigned.
So it's just not obvious that the patch preserves this - if it does then
the patch is OK.
Richard.
> gcc/
> * tree-vect-loop.c (vect_create_epilog_for_reduction): Convert
> the phi results to vectype after creating them. Remove later
> conversion code that thus becomes redundant.
> ---
> gcc/tree-vect-loop.c | 28 +++++++++++-----------------
> 1 file changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index b7f73ca52c7..1bd9a6ea52c 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -5214,9 +5214,11 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> if (double_reduc)
> loop = outer_loop;
> exit_bb = single_exit (loop)->dest;
> + exit_gsi = gsi_after_labels (exit_bb);
> reduc_inputs.create (slp_node ? vec_num : ncopies);
> for (unsigned i = 0; i < vec_num; i++)
> {
> + gimple_seq stmts = NULL;
> if (slp_node)
> def = vect_get_slp_vect_def (slp_node, i);
> else
> @@ -5228,12 +5230,12 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> if (j)
> def = gimple_get_lhs (STMT_VINFO_VEC_STMTS (rdef_info)[j]);
> SET_PHI_ARG_DEF (phi, single_exit (loop)->dest_idx, def);
> + new_def = gimple_convert (&stmts, vectype, new_def);
> reduc_inputs.quick_push (new_def);
> }
> + gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
> }
>
> - exit_gsi = gsi_after_labels (exit_bb);
> -
> /* 2.2 Get the relevant tree-code to use in the epilog for schemes 2,3
> (i.e. when reduc_fn is not available) and in the final adjustment
> code (if needed). Also get the original scalar reduction variable as
> @@ -5277,17 +5279,14 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> || ncopies > 1)
> {
> gimple_seq stmts = NULL;
> - tree first_vect = gimple_convert (&stmts, vectype, reduc_inputs[0]);
> + tree single_input = reduc_inputs[0];
> for (k = 1; k < reduc_inputs.length (); k++)
> - {
> - tree second_vect = gimple_convert (&stmts, vectype,
> reduc_inputs[k]);
> - first_vect = gimple_build (&stmts, code, vectype,
> - first_vect, second_vect);
> - }
> + single_input = gimple_build (&stmts, code, vectype,
> + single_input, reduc_inputs[k]);
> gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
>
> reduc_inputs.truncate (0);
> - reduc_inputs.safe_push (first_vect);
> + reduc_inputs.safe_push (single_input);
> }
>
> if (STMT_VINFO_REDUC_TYPE (reduc_info) == COND_REDUCTION
> @@ -5323,10 +5322,6 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> /* Vector of {0, 0, 0,...}. */
> tree zero_vec = build_zero_cst (vectype);
>
> - gimple_seq stmts = NULL;
> - reduc_inputs[0] = gimple_convert (&stmts, vectype, reduc_inputs[0]);
> - gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
> -
> /* Find maximum value from the vector of found indexes. */
> tree max_index = make_ssa_name (index_scalar_type);
> gcall *max_index_stmt = gimple_build_call_internal (IFN_REDUC_MAX,
> @@ -5394,7 +5389,7 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
>
> /* Convert the reduced value back to the result type and set as the
> result. */
> - stmts = NULL;
> + gimple_seq stmts = NULL;
> new_temp = gimple_build (&stmts, VIEW_CONVERT_EXPR, scalar_type,
> data_reduc);
> gsi_insert_seq_before (&exit_gsi, stmts, GSI_SAME_STMT);
> @@ -5412,7 +5407,7 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> val = data_reduc[i], idx_val = induction_index[i];
> return val; */
>
> - tree data_eltype = TREE_TYPE (TREE_TYPE (reduc_inputs[0]));
> + tree data_eltype = TREE_TYPE (vectype);
> tree idx_eltype = TREE_TYPE (TREE_TYPE (induction_index));
> unsigned HOST_WIDE_INT el_size = tree_to_uhwi (TYPE_SIZE (idx_eltype));
> poly_uint64 nunits = TYPE_VECTOR_SUBPARTS (TREE_TYPE
> (induction_index));
> @@ -5488,8 +5483,7 @@ vect_create_epilog_for_reduction (loop_vec_info
> loop_vinfo,
> "Reduce using direct vector reduction.\n");
>
> gimple_seq stmts = NULL;
> - reduc_inputs[0] = gimple_convert (&stmts, vectype, reduc_inputs[0]);
> - vec_elem_type = TREE_TYPE (TREE_TYPE (reduc_inputs[0]));
> + vec_elem_type = TREE_TYPE (vectype);
> new_temp = gimple_build (&stmts, as_combined_fn (reduc_fn),
> vec_elem_type, reduc_inputs[0]);
> new_temp = gimple_convert (&stmts, scalar_type, new_temp);
