Re: [PATCH] Don't ICE on long long shifts in vectorizable_shift

[Richard Guenther <rguenther@suse.de>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3497 bytes --]

On Thu, 27 Oct 2011, Jakub Jelinek wrote:

> Hi!
> 
> With the patch I'm going to post momentarily which adds vlshrv{4,2}di and
> vashlv{4,2}di patterns for -mavx2 vectorizable_shift ICEs, because the
> frontends for long_long_var1 << long_long_var2 emit long_long_var1 << (int) long_long_var2
> and vectorizable_shift isn't prepared to handle type promotion (or
> demotion).  IMHO it would complicate it too much, so this patch just
> gives up on vectorizing in that case.
> 
> I can work on Monday on pattern recognizer that will change
> shifts/rotates where the rhs1 has different size from rhs2 into
> a pattern with def_stmt casting the rhs2 to the same type as rhs1
> and pattern_stmt that uses the temporary for rhs2, perhaps with extra
> optimization if it sees the type being promoted/demoted again to just look
> through those.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Hm, but you are testing vector modes in the path that is supposed to
handle shifts by a scalar.  That looks odd.  Also it should be easy
to promote/demote the scalar value to the proper type, so why not
do that?  Like how we do

              /* 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.  */
              if (dt[1] == vect_constant_def)
                op1 = fold_convert (TREE_TYPE (vectype), op1);

just do that for all kind of defs (and deal with the fact that you
may need a gimplified stmt for the vector shift amount generation).

?

Thanks,
Richard.


> 2011-10-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* tree-vect-stmts.c (vectorizable_shift): Give up if op1 has different
> 	vector mode from vectype's mode.
> 
> --- gcc/tree-vect-stmts.c.jj	2011-10-27 08:42:51.000000000 +0200
> +++ gcc/tree-vect-stmts.c	2011-10-27 17:24:15.000000000 +0200
> @@ -2318,6 +2318,7 @@ vectorizable_shift (gimple stmt, gimple_
>    int nunits_in;
>    int nunits_out;
>    tree vectype_out;
> +  tree op1_vectype;
>    int ncopies;
>    int j, i;
>    VEC (tree, heap) *vec_oprnds0 = NULL, *vec_oprnds1 = NULL;
> @@ -2387,7 +2388,8 @@ vectorizable_shift (gimple stmt, gimple_
>      return false;
>  
>    op1 = gimple_assign_rhs2 (stmt);
> -  if (!vect_is_simple_use (op1, loop_vinfo, bb_vinfo, &def_stmt, &def, &dt[1]))
> +  if (!vect_is_simple_use_1 (op1, loop_vinfo, bb_vinfo, &def_stmt, &def,
> +			     &dt[1], &op1_vectype))
>      {
>        if (vect_print_dump_info (REPORT_DETAILS))
>          fprintf (vect_dump, "use not simple.");
> @@ -2444,6 +2446,13 @@ 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 (vect_print_dump_info (REPORT_DETAILS))
> +	    fprintf (vect_dump, "unusable type for last operand in"
> +				" vector/vector shift/rotate.");
> +	  return false;
> +	}
>      }
>    /* See if the machine has a vector shifted by scalar insn and if not
>       then see if it has a vector shifted by vector insn.  */
> 
> 	Jakub
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer