diff mbox

Fix fold making valid VEC_PERMs invalid

Message ID alpine.DEB.2.02.1411030813040.8005@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Nov. 3, 2014, 7:15 a.m. UTC
On Sun, 2 Nov 2014, Jakub Jelinek wrote:

> On Sun, Nov 02, 2014 at 11:08:57AM +0100, Marc Glisse wrote:
>> @@ -14189,47 +14190,47 @@ fold_ternary_loc (location_t loc, enum t
>>  	return fold_build2_loc (loc, PLUS_EXPR, type,
>>  				const_binop (MULT_EXPR, arg0, arg1), arg2);
>>        if (integer_zerop (arg2))
>>  	return fold_build2_loc (loc, MULT_EXPR, type, arg0, arg1);
>>
>>        return fold_fma (loc, type, arg0, arg1, arg2);
>>
>>      case VEC_PERM_EXPR:
>>        if (TREE_CODE (arg2) == VECTOR_CST)
>>  	{
>> -	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i, mask;
>> +	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i, mask, mask2;
>>  	  unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
>> +	  unsigned char *sel2 = XALLOCAVEC (unsigned char, nelts);
>
> Can't you just XALLOCAVEC 2 * nelts and set sel2 = sel + nelts; ?
>
>>  	  bool need_mask_canon = false;
>> +	  bool need_mask_canon2 = false;
>>  	  bool all_in_vec0 = true;
>>  	  bool all_in_vec1 = true;
>>  	  bool maybe_identity = true;
>>  	  bool single_arg = (op0 == op1);
>>  	  bool changed = false;
>>
>>  	  mask = single_arg ? (nelts - 1) : (2 * nelts - 1);
>> +	  mask2 = 2 * nelts - 1;
>
> Perhaps mask2 = 2 * nelts - 1; first and use mask2 in mask = ?
>
> Otherwise LGTM.

Should I commit the following version, which passed testing as well?

Comments

Jakub Jelinek Nov. 3, 2014, 7:34 a.m. UTC | #1
On Mon, Nov 03, 2014 at 08:15:32AM +0100, Marc Glisse wrote:
> Should I commit the following version, which passed testing as well?

Yes, thanks.

	Jakub
Richard Biener Nov. 3, 2014, 8:03 a.m. UTC | #2
On November 3, 2014 8:34:11 AM CET, Jakub Jelinek <jakub@redhat.com> wrote:
>On Mon, Nov 03, 2014 at 08:15:32AM +0100, Marc Glisse wrote:
>> Should I commit the following version, which passed testing as well?
>
>Yes, thanks.

Thanks for fixing this!

Richard.

>	Jakub
diff mbox

Patch

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 217029)
+++ gcc/fold-const.c	(working copy)
@@ -75,20 +75,21 @@  along with GCC; see the file COPYING3.
 #include "gimple.h"
 #include "gimplify.h"
 #include "tree-dfa.h"
 #include "hash-table.h"  /* Required for ENABLE_FOLD_CHECKING.  */
 #include "builtins.h"
 #include "hash-map.h"
 #include "plugin-api.h"
 #include "ipa-ref.h"
 #include "cgraph.h"
 #include "generic-match.h"
+#include "optabs.h"
 
 /* Nonzero if we are folding constants inside an initializer; zero
    otherwise.  */
 int folding_initializer = 0;
 
 /* The following constants represent a bit based encoding of GCC's
    comparison operators.  This encoding simplifies transformations
    on relational comparison operators, such as AND and OR.  */
 enum comparison_code {
   COMPCODE_FALSE = 0,
@@ -14189,47 +14190,47 @@  fold_ternary_loc (location_t loc, enum t
 	return fold_build2_loc (loc, PLUS_EXPR, type,
 				const_binop (MULT_EXPR, arg0, arg1), arg2);
       if (integer_zerop (arg2))
 	return fold_build2_loc (loc, MULT_EXPR, type, arg0, arg1);
 
       return fold_fma (loc, type, arg0, arg1, arg2);
 
     case VEC_PERM_EXPR:
       if (TREE_CODE (arg2) == VECTOR_CST)
 	{
-	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i, mask;
-	  unsigned char *sel = XALLOCAVEC (unsigned char, nelts);
+	  unsigned int nelts = TYPE_VECTOR_SUBPARTS (type), i, mask, mask2;
+	  unsigned char *sel = XALLOCAVEC (unsigned char, 2 * nelts);
+	  unsigned char *sel2 = sel + nelts;
 	  bool need_mask_canon = false;
+	  bool need_mask_canon2 = false;
 	  bool all_in_vec0 = true;
 	  bool all_in_vec1 = true;
 	  bool maybe_identity = true;
 	  bool single_arg = (op0 == op1);
 	  bool changed = false;
 
-	  mask = single_arg ? (nelts - 1) : (2 * nelts - 1);
+	  mask2 = 2 * nelts - 1;
+	  mask = single_arg ? (nelts - 1) : mask2;
 	  gcc_assert (nelts == VECTOR_CST_NELTS (arg2));
 	  for (i = 0; i < nelts; i++)
 	    {
 	      tree val = VECTOR_CST_ELT (arg2, i);
 	      if (TREE_CODE (val) != INTEGER_CST)
 		return NULL_TREE;
 
 	      /* Make sure that the perm value is in an acceptable
 		 range.  */
 	      wide_int t = val;
-	      if (wi::gtu_p (t, mask))
-		{
-		  need_mask_canon = true;
-		  sel[i] = t.to_uhwi () & mask;
-		}
-	      else
-		sel[i] = t.to_uhwi ();
+	      need_mask_canon |= wi::gtu_p (t, mask);
+	      need_mask_canon2 |= wi::gtu_p (t, mask2);
+	      sel[i] = t.to_uhwi () & mask;
+	      sel2[i] = t.to_uhwi () & mask2;
 
 	      if (sel[i] < nelts)
 		all_in_vec1 = false;
 	      else
 		all_in_vec0 = false;
 
 	      if ((sel[i] & (nelts-1)) != i)
 		maybe_identity = false;
 	    }
 
@@ -14257,20 +14258,31 @@  fold_ternary_loc (location_t loc, enum t
 		  || TREE_CODE (op1) == CONSTRUCTOR))
 	    {
 	      tree t = fold_vec_perm (type, op0, op1, sel);
 	      if (t != NULL_TREE)
 		return t;
 	    }
 
 	  if (op0 == op1 && !single_arg)
 	    changed = true;
 
+	  /* Some targets are deficient and fail to expand a single
+	     argument permutation while still allowing an equivalent
+	     2-argument version.  */
+	  if (need_mask_canon && arg2 == op2
+	      && !can_vec_perm_p (TYPE_MODE (type), false, sel)
+	      && can_vec_perm_p (TYPE_MODE (type), false, sel2))
+	    {
+	      need_mask_canon = need_mask_canon2;
+	      sel = sel2;
+	    }
+
 	  if (need_mask_canon && arg2 == op2)
 	    {
 	      tree *tsel = XALLOCAVEC (tree, nelts);
 	      tree eltype = TREE_TYPE (TREE_TYPE (arg2));
 	      for (i = 0; i < nelts; i++)
 		tsel[i] = build_int_cst (eltype, sel[i]);
 	      op2 = build_vector (TREE_TYPE (arg2), tsel);
 	      changed = true;
 	    }