Jakub Jelinek <ja...@redhat.com> writes:
> Hi!
>
> Unlike x86, where the last operand of vector by scalar shift is DImode for
> V[248]DImode shifts, on aarch64 they seem to be SImode.
> vect_recog_rotate_pattern when the rotate amount is not constant casts the
> amount to the element type of the vector, so for V[248]DImode vectors to
> DImode, but then we ICE during expand_shift_1 because such argument doesn't
> satisfy the predicate and can't be widened to it.
>
> The following patch fixes it by special casing vector by scalar shifts
> when adding patterns for rotates, in that case we punt if the operand isn't
> INTEGER_CST or external_def, and the patch just keeps using the type of the
> amount operand the rotate had for the shifts too.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-12-10 Jakub Jelinek <ja...@redhat.com>
>
> PR target/92723
> * tree-vect-patterns.c (vect_recog_rotate_pattern): If vector x vector
> shifts aren't supported and rotate amount is SSA_NAME, use its type
> rather than first operand's type for the shift amounts.
>
> * gcc.dg/vect/pr92723.c: New test.
>
> --- gcc/tree-vect-patterns.c.jj 2019-12-09 11:12:29.983288823 +0100
> +++ gcc/tree-vect-patterns.c 2019-12-10 16:30:59.922177911 +0100
> @@ -2242,6 +2242,7 @@ vect_recog_rotate_pattern (stmt_vec_info
> optab optab1, optab2;
> edge ext_def = NULL;
> bool bswap16_p = false;
> + bool scalar_shift_p = false;
>
> if (is_gimple_assign (last_stmt))
> {
> @@ -2420,6 +2421,7 @@ vect_recog_rotate_pattern (stmt_vec_info
> || !optab2
> || optab_handler (optab2, TYPE_MODE (vectype)) == CODE_FOR_nothing)
> return NULL;
> + scalar_shift_p = true;
> }
>
> *type_out = vectype;
> @@ -2439,7 +2441,8 @@ vect_recog_rotate_pattern (stmt_vec_info
> def = NULL_TREE;
> scalar_int_mode mode = SCALAR_INT_TYPE_MODE (type);
> if (TREE_CODE (oprnd1) == INTEGER_CST
> - || TYPE_MODE (TREE_TYPE (oprnd1)) == mode)
> + || TYPE_MODE (TREE_TYPE (oprnd1)) == mode
> + || scalar_shift_p)
> def = oprnd1;
> else if (def_stmt && gimple_assign_cast_p (def_stmt))
> {
WDYT about instead having:
if (dt != vect_internal_def || TYPE_MODE (TREE_TYPE (oprnd1)) == mode)
and removing the ext_def stuff? I'd have expected keeping the original
operand to always be best for vect_external_def, and it avoids changing
the function body during what's supposed to be just an analysis phase.
Thanks,
Richard
> --- gcc/testsuite/gcc.dg/vect/pr92723.c.jj 2019-12-10 16:37:09.924375698
> +0100
> +++ gcc/testsuite/gcc.dg/vect/pr92723.c 2019-12-10 16:37:21.823189103
> +0100
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +
> +void
> +foo (unsigned long long *x, unsigned long long *y, int z)
> +{
> + int i;
> + for (i = 0; i < 1024; i++)
> + x[i] = (y[i] >> z) | (y[i] << (-z & (__SIZEOF_LONG_LONG__ * __CHAR_BIT__
> - 1)));
> +}
>
> Jakub
