[PATCH] Re: vector shift regression on sparc

[Ira Rosen]

On 31 October 2011 11:53, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, Oct 30, 2011 at 12:38:32AM -0400, David Miller wrote:
>> gcc.dg/pr48616.c segfaults on sparc as of a day or two ago
>>
>> vectorizable_shift() crashes because op1_vectype is NULL and
>> we hit this code path:
>>
>>   /* Vector shifted by vector.  */
>>   if (!scalar_shift_arg)
>>     {
>>       optab = optab_for_tree_code (code, vectype, optab_vector);
>>       if (vect_print_dump_info (REPORT_DETAILS))
>>       fprintf (vect_dump, "vector/vector shift/rotate found.");
>> =>    if (TYPE_MODE (op1_vectype) != TYPE_MODE (vectype))
>>
>> dt[1] is vect_external_def and slp_node is non-NULL.
>>
>> Indeed, when the 'dt' arg to vect_is_simple_use_1() is
>> vect_external_def *vectype will be set to NULL.
>
> Here is a fix for that (and other issues that show up on these
> testcases with -O3 -mxop if I disable all vector/scalar shift expanders
> in sse.md).
> For SLP it currently gives up more often than for loop vectorization,
> I assume we could handle all dt[1] == vect_constant_def
> and dt[2] == vect_external_def cases for SLP (and at least the former
> even if the constants differ between nodes) by building the vectors by hand,
> though the current vect_get_vec_defs/vect_get_vec_defs_for_stmt_copy can't
> be used for that as is.
>
> 2011-10-28  Jakub Jelinek  <jakub@redhat.com>
>
>        * tree-vect-stmts.c (vectorizable_shift): If op1 is vect_external_def
>        in a loop and has different type from op0, cast it to op0's type
>        before the loop first.  For slp give up.  Don't crash if op1_vectype
>        is NULL.
>
>        * gcc.dg/vshift-3.c: New test.
>        * gcc.dg/vshift-4.c: New test.
>        * gcc.dg/vshift-5.c: New test.
>
> --- gcc/tree-vect-stmts.c.jj    2011-10-28 16:21:06.000000000 +0200
> +++ gcc/tree-vect-stmts.c       2011-10-31 10:27:57.000000000 +0100
> @@ -2446,7 +2446,10 @@ vectorizable_shift (gimple stmt, gimple_
>       optab = optab_for_tree_code (code, vectype, optab_vector);
>       if (vect_print_dump_info (REPORT_DETAILS))
>         fprintf (vect_dump, "vector/vector shift/rotate found.");
> -      if (TYPE_MODE (op1_vectype) != TYPE_MODE (vectype))
> +      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))
>        {
>          if (vect_print_dump_info (REPORT_DETAILS))
>            fprintf (vect_dump, "unusable type for last operand in"
> @@ -2480,9 +2483,28 @@ vectorizable_shift (gimple stmt, gimple_
>               /* Unlike the other binary operators, shifts/rotates have
>                  the rhs being int, instead of the same type as the lhs,
>                  so make sure the scalar is the right type if we are
> -                 dealing with vectors of short/char.  */
> +                dealing with vectors of long long/long/short/char.  */
>               if (dt[1] == vect_constant_def)
>                 op1 = fold_convert (TREE_TYPE (vectype), op1);
> +             else if (!useless_type_conversion_p (TREE_TYPE (vectype),
> +                                                  TREE_TYPE (op1)))

What happens in case dt[1] == vect_internal_def?

Thanks,
Ira

> +               {
> +                 if (slp_node
> +                     && TYPE_MODE (TREE_TYPE (vectype))
> +                        != TYPE_MODE (TREE_TYPE (op1)))
> +                   {
> +                     if (vect_print_dump_info (REPORT_DETAILS))
> +                     fprintf (vect_dump, "unusable type for last operand in"
> +                                         " vector/vector shift/rotate.");
> +                       return false;
> +                   }
> +                 if (vec_stmt && !slp_node)
> +                   {
> +                     op1 = fold_convert (TREE_TYPE (vectype), op1);
> +                     op1 = vect_init_vector (stmt, op1,
> +                                             TREE_TYPE (vectype), NULL);
> +                   }
> +               }
>             }
>         }
>     }

>
>
>        Jakub
>