diff mbox

Fix VEC_PERM_EXPR expansion regression (PR tree-optimization/68552)

Message ID 20151126180757.GK5675@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 26, 2015, 6:07 p.m. UTC
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)?

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.


	Jakub

Comments

Richard Biener Nov. 26, 2015, 7:09 p.m. UTC | #1
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
diff mbox

Patch

--- 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)
 	{