Re: [PATCH] Fix VEC_PERM_EXPR expansion regression (PR tree-optimization/68552)

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

On November 26, 2015 7:07:57 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>My recent tree-vect-generic.c changes broke Alpha.  My changes assume
>that if there is vec_shr_optab, middle argument 0 vector and last
>argument
>shift-ish selector, then expand_vec_perm will always handle it, but it
>turns
>out it does not, because expand_vec_perm does not call
>expand_vec_perm_1
>at all if the target does not have vec_perm_{const_,}optab for the
>particular mode (Alpha has none).  Looking at the code, there is also
>another potential issue, if we happen not to optimize __builtin_shuffle
>(zero_vector, zero_vector, shiftish_selector) into zero_vector,
>we might again not be able to expand it.
>
>Thus, the following patch moves the vec_shr_optab expansion from
>expand_vec_perm_1 (where it is attempted unnecessarily many times
>anyway,
>once for the mode and again for qimode with constant sel, once again
>for the
>mode and again for qimode if that failed and trying to use variable
>permutation.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/5
>(if it
>passes in Uros' testing)?

OK.

Thanks,
Richard.

>2015-11-26  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/68552
>	* optabs.c (expand_vec_perm_1): Move vec_shr handling from here...
>	(expand_vec_perm): ... here.  Do it regardless of vec_perm_const_optab
>	or whether v0 == v1.
>
>--- gcc/optabs.c.jj	2015-11-24 11:44:34.000000000 +0100
>+++ gcc/optabs.c	2015-11-26 16:48:05.565855431 +0100
>@@ -5274,17 +5274,6 @@ expand_vec_perm_1 (enum insn_code icode,
>   else
>     {
>       create_input_operand (&ops[1], v0, tmode);
>-      /* See if this can be handled with a vec_shr.  We only do this
>if the
>-         second vector is all zeroes.  */
>-      enum insn_code shift_code = optab_handler (vec_shr_optab,
>GET_MODE (v0));
>-      if (v1 == CONST0_RTX (GET_MODE (v1)) && shift_code)
>-	if (rtx shift_amt = shift_amt_for_vec_perm_mask (sel))
>-	  {
>-	    create_convert_operand_from_type (&ops[2], shift_amt,
>-					      sizetype_tab[(int) stk_sizetype]);
>-	    if (maybe_expand_insn (shift_code, 3, ops))
>-	      return ops[0].value;
>-	  }
>       create_input_operand (&ops[2], v1, tmode);
>     }
> 
>@@ -5326,6 +5315,44 @@ expand_vec_perm (machine_mode mode, rtx
>   gcc_assert (GET_MODE_CLASS (GET_MODE (sel)) == MODE_VECTOR_INT);
>   if (GET_CODE (sel) == CONST_VECTOR)
>     {
>+      /* See if this can be handled with a vec_shr.  We only do this
>if the
>+	 second vector is all zeroes.  */
>+      enum insn_code shift_code = optab_handler (vec_shr_optab, mode);
>+      enum insn_code shift_code_qi = ((qimode != VOIDmode && qimode !=
>mode)
>+				      ? optab_handler (vec_shr_optab, qimode)
>+				      : CODE_FOR_nothing);
>+      rtx shift_amt = NULL_RTX;
>+      if (v1 == CONST0_RTX (GET_MODE (v1))
>+	  && (shift_code != CODE_FOR_nothing
>+	      || shift_code_qi != CODE_FOR_nothing))
>+	{
>+	  shift_amt = shift_amt_for_vec_perm_mask (sel);
>+	  if (shift_amt)
>+	    {
>+	      struct expand_operand ops[3];
>+	      if (shift_code != CODE_FOR_nothing)
>+		{
>+		  create_output_operand (&ops[0], target, mode);
>+		  create_input_operand (&ops[1], v0, mode);
>+		  create_convert_operand_from_type (&ops[2], shift_amt,
>+						    sizetype);
>+		  if (maybe_expand_insn (shift_code, 3, ops))
>+		    return ops[0].value;
>+		}
>+	      if (shift_code_qi != CODE_FOR_nothing)
>+		{
>+		  tmp = gen_reg_rtx (qimode);
>+		  create_output_operand (&ops[0], tmp, qimode);
>+		  create_input_operand (&ops[1], gen_lowpart (qimode, v0),
>+					qimode);
>+		  create_convert_operand_from_type (&ops[2], shift_amt,
>+						    sizetype);
>+		  if (maybe_expand_insn (shift_code_qi, 3, ops))
>+		    return gen_lowpart (mode, ops[0].value);
>+		}
>+	    }
>+	}
>+
>       icode = direct_optab_handler (vec_perm_const_optab, mode);
>       if (icode != CODE_FOR_nothing)
> 	{
>
>	Jakub