diff mbox

Fold VEC_PERM_EXPR a little more

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

Commit Message

Marc Glisse Sept. 1, 2012, 2:38 p.m. UTC
Hello,

I noticed while writing the patch posted at:
http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01755.html

that fold_ternary_loc sometimes doesn't go all the way for VEC_PERM_EXPR, 
calling it a second time would fold even more. Fixed by a simple 
reordering.

(bootstrap and testsuite are ok)

2012-09-01  Marc Glisse  <marc.glisse@inria.fr>

gcc/
 	* fold-const.c (fold_ternary_loc): Constant-propagate after
 	removing dead operands.

gcc/testsuite/
 	* gcc.dg/fold-perm.c: Improve test.

Comments

Hans-Peter Nilsson Sept. 3, 2012, 1:41 a.m. UTC | #1
On Sat, 1 Sep 2012, Marc Glisse wrote:
> gcc/
> 	* fold-const.c (fold_ternary_loc): Constant-propagate after
> 	removing dead operands.
>
> gcc/testsuite/
> 	* gcc.dg/fold-perm.c: Improve test.

(adding a line and a parameter to the function containing the test-code)

JFTR: generally speaking, editing existing tests is frowned
upon.  Adding a new separate test is almost always better.
Not sure there's reason for an exception this time, but I see
you're the author of the original test and it's < 2 weeks since
it was added.

brgds, H-P
Marc Glisse Sept. 3, 2012, 9:15 a.m. UTC | #2
On Sun, 2 Sep 2012, Hans-Peter Nilsson wrote:

> On Sat, 1 Sep 2012, Marc Glisse wrote:
>> gcc/
>> 	* fold-const.c (fold_ternary_loc): Constant-propagate after
>> 	removing dead operands.
>>
>> gcc/testsuite/
>> 	* gcc.dg/fold-perm.c: Improve test.
>
> (adding a line and a parameter to the function containing the test-code)
>
> JFTR: generally speaking, editing existing tests is frowned
> upon.  Adding a new separate test is almost always better.
> Not sure there's reason for an exception this time, but I see
> you're the author of the original test and it's < 2 weeks since
> it was added.

Thanks for the comment. I am not sure there is much I can add to that. I 
know it is better to avoid modifying existing tests. And the fact that I 
am completing my patch from 2 weeks ago made me think it was ok this time. 
I'll break it up into fold-perm-2.c if the reviewer asks for it.
Richard Biener Sept. 3, 2012, 12:58 p.m. UTC | #3
On Sat, Sep 1, 2012 at 4:38 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> I noticed while writing the patch posted at:
> http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01755.html
>
> that fold_ternary_loc sometimes doesn't go all the way for VEC_PERM_EXPR,
> calling it a second time would fold even more. Fixed by a simple reordering.
>
> (bootstrap and testsuite are ok)

Ok.

Thanks,
Richard.

> 2012-09-01  Marc Glisse  <marc.glisse@inria.fr>
>
> gcc/
>         * fold-const.c (fold_ternary_loc): Constant-propagate after
>         removing dead operands.
>
> gcc/testsuite/
>         * gcc.dg/fold-perm.c: Improve test.
>
> --
> Marc Glisse
> Index: fold-const.c
> ===================================================================
> --- fold-const.c        (revision 190845)
> +++ fold-const.c        (working copy)
> @@ -14189,40 +14189,40 @@ fold_ternary_loc (location_t loc, enum t
>             }
>
>           if (maybe_identity)
>             {
>               if (all_in_vec0)
>                 return op0;
>               if (all_in_vec1)
>                 return op1;
>             }
>
> -         if ((TREE_CODE (arg0) == VECTOR_CST
> -              || TREE_CODE (arg0) == CONSTRUCTOR)
> -             && (TREE_CODE (arg1) == VECTOR_CST
> -                 || TREE_CODE (arg1) == CONSTRUCTOR))
> -           {
> -             t = fold_vec_perm (type, arg0, arg1, sel);
> -             if (t != NULL_TREE)
> -               return t;
> -           }
> -
>           if (all_in_vec0)
>             op1 = op0;
>           else if (all_in_vec1)
>             {
>               op0 = op1;
>               for (i = 0; i < nelts; i++)
>                 sel[i] -= nelts;
>               need_mask_canon = true;
>             }
>
> +         if ((TREE_CODE (op0) == VECTOR_CST
> +              || TREE_CODE (op0) == CONSTRUCTOR)
> +             && (TREE_CODE (op1) == VECTOR_CST
> +                 || TREE_CODE (op1) == CONSTRUCTOR))
> +           {
> +             t = fold_vec_perm (type, op0, op1, sel);
> +             if (t != NULL_TREE)
> +               return t;
> +           }
> +
>           if (op0 == op1 && !single_arg)
>             changed = true;
>
>           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);
> Index: testsuite/gcc.dg/fold-perm.c
> ===================================================================
> --- testsuite/gcc.dg/fold-perm.c        (revision 190845)
> +++ testsuite/gcc.dg/fold-perm.c        (working copy)
> @@ -1,19 +1,20 @@
>  /* { dg-do compile } */
>  /* { dg-options "-O -fdump-tree-ccp1" } */
>
>  typedef int veci __attribute__ ((vector_size (4 * sizeof (int))));
>
> -void fun (veci *f, veci *g, veci *h)
> +void fun (veci *f, veci *g, veci *h, veci *i)
>  {
>    veci m = { 7, 7, 4, 6 };
>    veci n = { 0, 1, 2, 3 };
>    veci p = { 1, 1, 7, 6 };
> +  *i = __builtin_shuffle (*i,  p, m);
>    *h = __builtin_shuffle (*h, *h, p);
>    *g = __builtin_shuffle (*f, *g, m);
>    *f = __builtin_shuffle (*f, *g, n);
>  }
>
>  /* { dg-final { scan-tree-dump "VEC_PERM_EXPR.*{ 3, 3, 0, 2 }" "ccp1" } }
> */
>  /* { dg-final { scan-tree-dump "VEC_PERM_EXPR.*{ 1, 1, 3, 2 }" "ccp1" } }
> */
>  /* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "ccp1" } } */
>  /* { dg-final { cleanup-tree-dump "ccp1" } } */
>
diff mbox

Patch

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 190845)
+++ fold-const.c	(working copy)
@@ -14189,40 +14189,40 @@  fold_ternary_loc (location_t loc, enum t
 	    }
 
 	  if (maybe_identity)
 	    {
 	      if (all_in_vec0)
 		return op0;
 	      if (all_in_vec1)
 		return op1;
 	    }
 
-	  if ((TREE_CODE (arg0) == VECTOR_CST
-	       || TREE_CODE (arg0) == CONSTRUCTOR)
-	      && (TREE_CODE (arg1) == VECTOR_CST
-		  || TREE_CODE (arg1) == CONSTRUCTOR))
-	    {
-	      t = fold_vec_perm (type, arg0, arg1, sel);
-	      if (t != NULL_TREE)
-		return t;
-	    }
-
 	  if (all_in_vec0)
 	    op1 = op0;
 	  else if (all_in_vec1)
 	    {
 	      op0 = op1;
 	      for (i = 0; i < nelts; i++)
 		sel[i] -= nelts;
 	      need_mask_canon = true;
 	    }
 
+	  if ((TREE_CODE (op0) == VECTOR_CST
+	       || TREE_CODE (op0) == CONSTRUCTOR)
+	      && (TREE_CODE (op1) == VECTOR_CST
+		  || TREE_CODE (op1) == CONSTRUCTOR))
+	    {
+	      t = fold_vec_perm (type, op0, op1, sel);
+	      if (t != NULL_TREE)
+		return t;
+	    }
+
 	  if (op0 == op1 && !single_arg)
 	    changed = true;
 
 	  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);
Index: testsuite/gcc.dg/fold-perm.c
===================================================================
--- testsuite/gcc.dg/fold-perm.c	(revision 190845)
+++ testsuite/gcc.dg/fold-perm.c	(working copy)
@@ -1,19 +1,20 @@ 
 /* { dg-do compile } */
 /* { dg-options "-O -fdump-tree-ccp1" } */
 
 typedef int veci __attribute__ ((vector_size (4 * sizeof (int))));
 
-void fun (veci *f, veci *g, veci *h)
+void fun (veci *f, veci *g, veci *h, veci *i)
 {
   veci m = { 7, 7, 4, 6 };
   veci n = { 0, 1, 2, 3 };
   veci p = { 1, 1, 7, 6 };
+  *i = __builtin_shuffle (*i,  p, m);
   *h = __builtin_shuffle (*h, *h, p);
   *g = __builtin_shuffle (*f, *g, m);
   *f = __builtin_shuffle (*f, *g, n);
 }
 
 /* { dg-final { scan-tree-dump "VEC_PERM_EXPR.*{ 3, 3, 0, 2 }" "ccp1" } } */
 /* { dg-final { scan-tree-dump "VEC_PERM_EXPR.*{ 1, 1, 3, 2 }" "ccp1" } } */
 /* { dg-final { scan-tree-dump-times "VEC_PERM_EXPR" 2 "ccp1" } } */
 /* { dg-final { cleanup-tree-dump "ccp1" } } */