On Fri, Oct 25, 2019 at 2:34 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> The validation phase of vectorizable_shift used TYPE_MODE to check
> whether the shift amount vector was compatible with the shifted vector:
>
> if ((op1_vectype == NULL_TREE
> || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype))
> && (!slp_node
> || SLP_TREE_DEF_TYPE
> (SLP_TREE_CHILDREN (slp_node)[1]) != vect_constant_def))
>
> But the generation phase was stricter and required the element types to
> be equivalent:
>
> && !useless_type_conversion_p (TREE_TYPE (vectype),
> TREE_TYPE (op1)))
>
> This difference led to an ICE with a later patch.
>
> The first condition seems a bit too lax given that the function
> supports vect_worthwhile_without_simd_p, where two different vector
> types could have the same integer mode. But it seems too strict
> to reject signed shifts by unsigned amounts or unsigned shifts by
> signed amounts; verify_gimple_assign_binary is happy with those.
>
> This patch therefore goes for a middle ground of checking both TYPE_MODE
> and TYPE_VECTOR_SUBPARTS, using the same condition in both places.
OK. The whole vectorizable_shift needs a rewrite ;) (no good reason to
not support widening/narrowing of a shift argument)
Richard.
>
> 2019-10-24 Richard Sandiford <richard.sandif...@arm.com>
>
> gcc/
> * tree-vect-stmts.c (vectorizable_shift): Check the number
> of vector elements as well as the type mode when deciding
> whether an op1_vectype is compatible. Reuse the result of
> this check when generating vector statements.
>
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c 2019-10-25 13:27:08.653811531 +0100
> +++ gcc/tree-vect-stmts.c 2019-10-25 13:27:12.121787027 +0100
> @@ -5522,6 +5522,7 @@ vectorizable_shift (stmt_vec_info stmt_i
> bool scalar_shift_arg = true;
> bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_info);
> vec_info *vinfo = stmt_info->vinfo;
> + bool incompatible_op1_vectype_p = false;
>
> if (!STMT_VINFO_RELEVANT_P (stmt_info) && !bb_vinfo)
> return false;
> @@ -5666,8 +5667,12 @@ vectorizable_shift (stmt_vec_info stmt_i
>
> if (!op1_vectype)
> op1_vectype = get_same_sized_vectype (TREE_TYPE (op1), vectype_out);
> - if ((op1_vectype == NULL_TREE
> - || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype))
> + incompatible_op1_vectype_p
> + = (op1_vectype == NULL_TREE
> + || maybe_ne (TYPE_VECTOR_SUBPARTS (op1_vectype),
> + TYPE_VECTOR_SUBPARTS (vectype))
> + || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype));
> + if (incompatible_op1_vectype_p
> && (!slp_node
> || SLP_TREE_DEF_TYPE
> (SLP_TREE_CHILDREN (slp_node)[1]) != vect_constant_def))
> @@ -5813,9 +5818,7 @@ vectorizable_shift (stmt_vec_info stmt_i
> }
> }
> }
> - else if (slp_node
> - && !useless_type_conversion_p (TREE_TYPE (vectype),
> - TREE_TYPE (op1)))
> + else if (slp_node && incompatible_op1_vectype_p)
> {
> if (was_scalar_shift_arg)
> {
