Re: [patch] Fix ICE in vector-shift1.c (vector/scalar to vector/vector transform)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Thu, Dec 30, 2010 at 9:52 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> on spu-elf I'm seeing ICEs with the new vector shift support:
> FAIL: gcc.c-torture/execute/vector-shift1.c compilation, ?-O0 ?(internal compiler error)
> UNRESOLVED: gcc.c-torture/execute/vector-shift1.c execution, ?-O0
>
> The problem is that the back-end only defines vector/vector
> shift patterns, not vector/scalar patterns. ?Now when the
> source code contains a vector << scalar shift, the generic
> vector lowering pass thinks that back-end support is sufficient
> to implement this operation, and leaves it unchanged.
>
> However, when expand_shift then tries to actually generate RTL,
> it will only attempt to use vector/scalar insn patterns, because
> the incoming tree is of that form. ?Since those are not present,
> we get the ICE.
>
> Now at the same place in expand_vector_operations_1, if the code
> decides to replace a vector/vector operation with a vector/scalar
> one because all vector elements of the shift count are equal, it
> actually updates gimple to the new form. ?Thus it would appear
> to me that if we do the reverse transform, we similarly ought
> to update the gimple form.
>
> The patch below implements this transformation, fixing the ICE
> in the vector-shift1.c test case.
>
> Tested on spu-elf with no regressions.
>
> OK for mainline?

Hm.  The conversion should only happen if the optab is not zero,
otherwise the code will be fully scalarized (and the stmt conversion
is not necessary).

Thanks,
Richard.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
> ? ? ? ?* tree-vect-generic.c (expand_vector_operations_1): When using vector/
> ? ? ? ?vector optab to expand vector/scalar shift, update gimple to vector.
>
> Index: gcc/tree-vect-generic.c
> ===================================================================
> *** gcc/tree-vect-generic.c ? ? (revision 168294)
> --- gcc/tree-vect-generic.c ? ? (working copy)
> *************** expand_vector_operations_1 (gimple_stmt_
> *** 520,526 ****
> ? ? ? ?/* Try for a vector/scalar shift, and if we don't have one, see if we
> ? ? ? ? ? have a vector/vector shift */
> ? ? ? ?else if (!vector_scalar_shift)
> ! ? ? ? ? op = optab_for_tree_code (code, type, optab_vector);
> ? ? ?}
> ? ?else
> ? ? ?op = optab_for_tree_code (code, type, optab_default);
> --- 520,539 ----
> ? ? ? ?/* Try for a vector/scalar shift, and if we don't have one, see if we
> ? ? ? ? ? have a vector/vector shift */
> ? ? ? ?else if (!vector_scalar_shift)
> ! ? ? ? {
> ! ? ? ? ? /* Transform vector <op> scalar => vector <op> {x,x,x,x}. ?*/
> ! ? ? ? ? int n_parts = TYPE_VECTOR_SUBPARTS (type);
> ! ? ? ? ? int part_size = tree_low_cst (TYPE_SIZE (TREE_TYPE (type)), 1);
> ! ? ? ? ? tree part_type = lang_hooks.types.type_for_size (part_size, 1);
> ! ? ? ? ? tree vect_type = build_vector_type (part_type, n_parts);
> !
> ! ? ? ? ? rhs2 = fold_convert (part_type, rhs2);
> ! ? ? ? ? rhs2 = build_vector_from_val (vect_type, rhs2);
> ! ? ? ? ? gimple_assign_set_rhs2 (stmt, rhs2);
> ! ? ? ? ? update_stmt (stmt);
> !
> ! ? ? ? ? op = optab_for_tree_code (code, type, optab_vector);
> ! ? ? ? }
> ? ? ?}
> ? ?else
> ? ? ?op = optab_for_tree_code (code, type, optab_default);
> --
> ?Dr. Ulrich Weigand
> ?GNU Toolchain for Linux on System z and Cell BE
> ?Ulrich.Weigand@de.ibm.com
>