On Tue, 4 Jul 2023, Robin Dapp wrote:
> Hi Richard,
>
> changed the patch according to your comments and I agree that
> it is more readable that way. I hope using lhs as target for
> the extract directly is possible the way I did it. Richard's
> patch for aarch64 is already, therefore testsuites on aarch64 and
> i386 are unchanged.
>
> Regards
> Robin
>
> Subject: [PATCH v2] gimple-isel: Recognize vec_extract pattern.
>
> In gimple-isel we already deduce a vec_set pattern from an
> ARRAY_REF(VIEW_CONVERT_EXPR). This patch does the same for a
> vec_extract.
>
> The code is largely similar to the vec_set one
> including the addition of a can_vec_extract_var_idx_p function
> in optabs.cc to check if the backend can handle a register
> operand as index. We already have can_vec_extract in
> optabs-query but that one checks whether we can extract
> specific modes.
>
> With the introduction of an internal function for vec_extract
> the expander must not FAIL. For vec_set this has already been
> the case so adjust the documentation accordingly.
>
> Additionally, clarify the wording of the vector-vector case for
> vec_extract.
>
> gcc/ChangeLog:
>
> * doc/md.texi: Document that vec_set and vec_extract must not
> fail.
> * gimple-isel.cc (gimple_expand_vec_set_expr): Rename this...
> (gimple_expand_vec_set_extract_expr): ...to this.
> (gimple_expand_vec_exprs): Call renamed function.
> * internal-fn.cc (vec_extract_direct): Add.
> (expand_vec_extract_optab_fn): New function to expand
> vec_extract optab.
> (direct_vec_extract_optab_supported_p): Add.
> * internal-fn.def (VEC_EXTRACT): Add.
> * optabs.cc (can_vec_extract_var_idx_p): New function.
> * optabs.h (can_vec_extract_var_idx_p): Declare.
> ---
> gcc/doc/md.texi | 7 +++-
> gcc/gimple-isel.cc | 100 ++++++++++++++++++++++++++++++++------------
> gcc/internal-fn.cc | 39 +++++++++++++++++
> gcc/internal-fn.def | 1 +
> gcc/optabs.cc | 24 +++++++++++
> gcc/optabs.h | 1 +
> 6 files changed, 144 insertions(+), 28 deletions(-)
>
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index f14dd32b2dc..b30a824488b 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -5091,6 +5091,8 @@ Mask elements @var{i} with @var{i} > (operand 5 +
> operand 6) are ignored.
> Set given field in the vector value. Operand 0 is the vector to modify,
> operand 1 is new value of field and operand 2 specify the field index.
>
> +This pattern is not allowed to @code{FAIL}.
> +
> @cindex @code{vec_extract@var{m}@var{n}} instruction pattern
> @item @samp{vec_extract@var{m}@var{n}}
> Extract given field from the vector value. Operand 1 is the vector, operand
> 2
> @@ -5098,7 +5100,10 @@ specify field index and operand 0 place to store value
> into. The
> @var{n} mode is the mode of the field or vector of fields that should be
> extracted, should be either element mode of the vector mode @var{m}, or
> a vector mode with the same element mode and smaller number of elements.
> -If @var{n} is a vector mode, the index is counted in units of that mode.
> +If @var{n} is a vector mode the index is counted in multiples of
> +mode @var{n}.
> +
> +This pattern is not allowed to @code{FAIL}.
>
> @cindex @code{vec_init@var{m}@var{n}} instruction pattern
> @item @samp{vec_init@var{m}@var{n}}
> diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
> index ef688ddb57f..a18b26dec7b 100644
> --- a/gcc/gimple-isel.cc
> +++ b/gcc/gimple-isel.cc
> @@ -42,17 +42,27 @@ along with GCC; see the file COPYING3. If not see
>
> /* Expand all ARRAY_REF(VIEW_CONVERT_EXPR) gimple assignments into calls to
> internal function based on vector type of selected expansion.
> - i.e.:
> +
> + For vec_set:
> +
> VIEW_CONVERT_EXPR<int[4]>(u)[_1] = i_4(D);
> =>
> _7 = u;
> _8 = .VEC_SET (_7, i_4(D), _1);
> - u = _8; */
> + u = _8;
> +
> + For vec_extract:
> +
> + _3 = VIEW_CONVERT_EXPR<intD.1[4]>(vD.2208)[idx_2(D)];
> + =>
> + _4 = vD.2208;
> + _5 = .VEC_EXTRACT (_4, idx_2(D));
> + _3 = _5; */
I think you are doing
_3 = .VEC_EXTRACT (_4, idx_2(D));
and avoiding the SSA name copy correctly. Can you double-check?
OK with the comment adjusted.
Thanks,
Richard.
>
> static bool
> -gimple_expand_vec_set_expr (struct function *fun, gimple_stmt_iterator *gsi)
> +gimple_expand_vec_set_extract_expr (struct function *fun,
> + gimple_stmt_iterator *gsi)
> {
> - enum tree_code code;
> gcall *new_stmt = NULL;
> gassign *ass_stmt = NULL;
> bool cfg_changed = false;
> @@ -62,49 +72,84 @@ gimple_expand_vec_set_expr (struct function *fun,
> gimple_stmt_iterator *gsi)
> if (!stmt)
> return false;
>
> + bool is_extract = false;
> +
> tree lhs = gimple_assign_lhs (stmt);
> - code = TREE_CODE (lhs);
> - if (code != ARRAY_REF)
> + tree rhs = gimple_assign_rhs1 (stmt);
> + tree val, ref;
> + if (TREE_CODE (lhs) == ARRAY_REF)
> + {
> + /* Assume it is a vec_set. */
> + val = rhs;
> + ref = lhs;
> + }
> + else if (TREE_CODE (rhs) == ARRAY_REF)
> + {
> + /* vec_extract. */
> + is_extract = true;
> + val = lhs;
> + ref = rhs;
> + }
> + else
> return false;
>
> - tree val = gimple_assign_rhs1 (stmt);
> - tree op0 = TREE_OPERAND (lhs, 0);
> + tree op0 = TREE_OPERAND (ref, 0);
> if (TREE_CODE (op0) == VIEW_CONVERT_EXPR && DECL_P (TREE_OPERAND (op0, 0))
> && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
> - && TYPE_MODE (TREE_TYPE (lhs))
> + && TYPE_MODE (TREE_TYPE (ref))
> == TYPE_MODE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (op0, 0)))))
> {
> - tree pos = TREE_OPERAND (lhs, 1);
> + tree pos = TREE_OPERAND (ref, 1);
> +
> tree view_op0 = TREE_OPERAND (op0, 0);
> machine_mode outermode = TYPE_MODE (TREE_TYPE (view_op0));
> + machine_mode extract_mode = TYPE_MODE (TREE_TYPE (ref));
> +
> if (auto_var_in_fn_p (view_op0, fun->decl)
> - && !TREE_ADDRESSABLE (view_op0) && can_vec_set_var_idx_p (outermode))
> + && !TREE_ADDRESSABLE (view_op0)
> + && ((!is_extract && can_vec_set_var_idx_p (outermode))
> + || (is_extract
> + && can_vec_extract_var_idx_p (outermode, extract_mode))))
> {
> location_t loc = gimple_location (stmt);
> tree var_src = make_ssa_name (TREE_TYPE (view_op0));
> - tree var_dst = make_ssa_name (TREE_TYPE (view_op0));
>
> ass_stmt = gimple_build_assign (var_src, view_op0);
> gimple_set_vuse (ass_stmt, gimple_vuse (stmt));
> gimple_set_location (ass_stmt, loc);
> gsi_insert_before (gsi, ass_stmt, GSI_SAME_STMT);
>
> - new_stmt
> - = gimple_build_call_internal (IFN_VEC_SET, 3, var_src, val, pos);
> - gimple_call_set_lhs (new_stmt, var_dst);
> - gimple_set_location (new_stmt, loc);
> - gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
> + if (!is_extract)
> + {
> + tree var_dst = make_ssa_name (TREE_TYPE (view_op0));
>
> - ass_stmt = gimple_build_assign (view_op0, var_dst);
> - gimple_set_location (ass_stmt, loc);
> - gsi_insert_before (gsi, ass_stmt, GSI_SAME_STMT);
> + new_stmt = gimple_build_call_internal (IFN_VEC_SET, 3, var_src,
> + val, pos);
> +
> + gimple_call_set_lhs (new_stmt, var_dst);
> + gimple_set_location (new_stmt, loc);
> + gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
> +
> + ass_stmt = gimple_build_assign (view_op0, var_dst);
> + gimple_set_location (ass_stmt, loc);
> + gimple_move_vops (ass_stmt, stmt);
> + gsi_insert_before (gsi, ass_stmt, GSI_SAME_STMT);
>
> - basic_block bb = gimple_bb (stmt);
> - gimple_move_vops (ass_stmt, stmt);
> - if (gsi_remove (gsi, true)
> - && gimple_purge_dead_eh_edges (bb))
> - cfg_changed = true;
> - *gsi = gsi_for_stmt (ass_stmt);
> + basic_block bb = gimple_bb (stmt);
> + if (gsi_remove (gsi, true)
> + && gimple_purge_dead_eh_edges (bb))
> + cfg_changed = true;
> + *gsi = gsi_for_stmt (ass_stmt);
> + }
> + else
> + {
> + new_stmt
> + = gimple_build_call_internal (IFN_VEC_EXTRACT, 2, var_src, pos);
> + gimple_call_set_lhs (new_stmt, lhs);
> +
> + gsi_replace (gsi, new_stmt, true);
> + cfg_changed = true;
> + }
> }
> }
>
> @@ -317,7 +362,8 @@ gimple_expand_vec_exprs (struct function *fun)
> gsi_replace (&gsi, g, false);
> }
>
> - cfg_changed |= gimple_expand_vec_set_expr (fun, &gsi);
> + cfg_changed |= gimple_expand_vec_set_extract_expr (fun, &gsi);
> +
> if (gsi_end_p (gsi))
> break;
> }
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 303df102d81..23f9309572b 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -175,6 +175,7 @@ init_internal_fns ()
> #define len_store_direct { 3, 3, false }
> #define len_maskstore_direct { 4, 5, false }
> #define vec_set_direct { 3, 3, false }
> +#define vec_extract_direct { 3, 3, false }
> #define unary_direct { 0, 0, true }
> #define unary_convert_direct { -1, 0, true }
> #define binary_direct { 0, 0, true }
> @@ -3107,6 +3108,43 @@ expand_vec_set_optab_fn (internal_fn, gcall *stmt,
> convert_optab optab)
> gcc_unreachable ();
> }
>
> +/* Expand VEC_EXTRACT optab internal function. */
> +
> +static void
> +expand_vec_extract_optab_fn (internal_fn, gcall *stmt, convert_optab optab)
> +{
> + tree lhs = gimple_call_lhs (stmt);
> + tree op0 = gimple_call_arg (stmt, 0);
> + tree op1 = gimple_call_arg (stmt, 1);
> +
> + rtx target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> +
> + machine_mode outermode = TYPE_MODE (TREE_TYPE (op0));
> + machine_mode extract_mode = TYPE_MODE (TREE_TYPE (lhs));
> +
> + rtx src = expand_normal (op0);
> + rtx pos = expand_normal (op1);
> +
> + class expand_operand ops[3];
> + enum insn_code icode = convert_optab_handler (optab, outermode,
> + extract_mode);
> +
> + if (icode != CODE_FOR_nothing)
> + {
> + create_output_operand (&ops[0], target, extract_mode);
> + create_input_operand (&ops[1], src, outermode);
> + create_convert_operand_from (&ops[2], pos,
> + TYPE_MODE (TREE_TYPE (op1)), true);
> + if (maybe_expand_insn (icode, 3, ops))
> + {
> + if (!rtx_equal_p (target, ops[0].value))
> + emit_move_insn (target, ops[0].value);
> + return;
> + }
> + }
> + gcc_unreachable ();
> +}
> +
> static void
> expand_ABNORMAL_DISPATCHER (internal_fn, gcall *)
> {
> @@ -3946,6 +3984,7 @@ multi_vector_optab_supported_p (convert_optab optab,
> tree_pair types,
> #define direct_mask_fold_left_optab_supported_p direct_optab_supported_p
> #define direct_check_ptrs_optab_supported_p direct_optab_supported_p
> #define direct_vec_set_optab_supported_p direct_optab_supported_p
> +#define direct_vec_extract_optab_supported_p direct_optab_supported_p
>
> /* Return the optab used by internal function FN. */
>
> diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
> index 9b73e540d55..238b7ee0bc9 100644
> --- a/gcc/internal-fn.def
> +++ b/gcc/internal-fn.def
> @@ -180,6 +180,7 @@ DEF_INTERNAL_OPTAB_FN (VCONDEQ, 0, vcondeq, vec_cond)
> DEF_INTERNAL_OPTAB_FN (VCOND_MASK, 0, vcond_mask, vec_cond_mask)
>
> DEF_INTERNAL_OPTAB_FN (VEC_SET, 0, vec_set, vec_set)
> +DEF_INTERNAL_OPTAB_FN (VEC_EXTRACT, 0, vec_extract, vec_extract)
>
> DEF_INTERNAL_OPTAB_FN (LEN_STORE, 0, len_store, len_store)
> DEF_INTERNAL_OPTAB_FN (LEN_MASK_STORE, 0, len_maskstore, len_maskstore)
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index 0a2bcf3620c..4e9f58f8060 100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -4417,6 +4417,30 @@ can_vec_set_var_idx_p (machine_mode vec_mode)
> && insn_operand_matches (icode, 2, reg3);
> }
>
> +/* Return whether the backend can emit a vec_extract instruction with
> + a non-constant index. */
> +bool
> +can_vec_extract_var_idx_p (machine_mode vec_mode, machine_mode extr_mode)
> +{
> + if (!VECTOR_MODE_P (vec_mode))
> + return false;
> +
> + rtx reg1 = alloca_raw_REG (extr_mode, LAST_VIRTUAL_REGISTER + 1);
> + rtx reg2 = alloca_raw_REG (vec_mode, LAST_VIRTUAL_REGISTER + 2);
> +
> + enum insn_code icode = convert_optab_handler (vec_extract_optab,
> + vec_mode, extr_mode);
> +
> + const struct insn_data_d *data = &insn_data[icode];
> + machine_mode idx_mode = data->operand[2].mode;
> +
> + rtx reg3 = alloca_raw_REG (idx_mode, LAST_VIRTUAL_REGISTER + 3);
> +
> + return icode != CODE_FOR_nothing && insn_operand_matches (icode, 0, reg1)
> + && insn_operand_matches (icode, 1, reg2)
> + && insn_operand_matches (icode, 2, reg3);
> +}
> +
> /* This function is called when we are going to emit a compare instruction
> that
> compares the values found in X and Y, using the rtl operator COMPARISON.
>
> diff --git a/gcc/optabs.h b/gcc/optabs.h
> index 781d5548be9..c80b7f4dc1b 100644
> --- a/gcc/optabs.h
> +++ b/gcc/optabs.h
> @@ -259,6 +259,7 @@ extern bool can_vcond_compare_p (enum rtx_code,
> machine_mode, machine_mode);
> /* Return whether the backend can emit vector set instructions for inserting
> element into vector at variable index position. */
> extern bool can_vec_set_var_idx_p (machine_mode);
> +extern bool can_vec_extract_var_idx_p (machine_mode, machine_mode);
>
> extern rtx prepare_operand (enum insn_code, rtx, int, machine_mode,
> machine_mode, int);
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
