Patchwork Fix PR53733

login
register
mail settings
Submitter William J. Schmidt
Date July 30, 2012, 6:53 p.m.
Message ID <1343674415.18670.4.camel@gnopaine>
Download mbox | patch
Permalink /patch/174074/
State New
Headers show

Comments

William J. Schmidt - July 30, 2012, 6:53 p.m.
This fixes the de-canonicalization of commutative GIMPLE operations in
the vectorizer that occurs when processing reductions.  A loop_vec_info
is flagged for cleanup when a de-canonicalization has occurred in that
loop, and the cleanup is done when the loop_vec_info is destroyed.

Bootstrapped on powerpc64-unknown-linux-gnu with no new regressions.  Ok
for trunk?

Thanks,
Bill


gcc:

2012-07-30  Bill Schmidt  <wschmidt@linux.ibm.com>

	PR tree-optimization/53773
	* tree-vectorizer.h (struct _loop_vec_info): Add operands_swapped.
	(LOOP_VINFO_OPERANDS_SWAPPED): New macro.
	* tree-vect-loop.c (new_loop_vec_info): Initialize
	LOOP_VINFO_OPERANDS_SWAPPED field.
	(destroy_loop_vec_info): Restore canonical form.
	(vect_is_slp_reduction): Set LOOP_VINFO_OPERANDS_SWAPPED field.
	(vect_is_simple_reduction_1): Likewise.

gcc/testsuite:

2012-07-30  Bill Schmidt  <wschmidt@linux.ibm.com>

	PR tree-optimization/53773
	* testsuite/gcc.dg/vect/pr53773.c: New test.
Richard Guenther - July 31, 2012, 8:38 a.m.
On Mon, 30 Jul 2012, William J. Schmidt wrote:

> This fixes the de-canonicalization of commutative GIMPLE operations in
> the vectorizer that occurs when processing reductions.  A loop_vec_info
> is flagged for cleanup when a de-canonicalization has occurred in that
> loop, and the cleanup is done when the loop_vec_info is destroyed.
> 
> Bootstrapped on powerpc64-unknown-linux-gnu with no new regressions.  Ok
> for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Bill
> 
> 
> gcc:
> 
> 2012-07-30  Bill Schmidt  <wschmidt@linux.ibm.com>
> 
> 	PR tree-optimization/53773
> 	* tree-vectorizer.h (struct _loop_vec_info): Add operands_swapped.
> 	(LOOP_VINFO_OPERANDS_SWAPPED): New macro.
> 	* tree-vect-loop.c (new_loop_vec_info): Initialize
> 	LOOP_VINFO_OPERANDS_SWAPPED field.
> 	(destroy_loop_vec_info): Restore canonical form.
> 	(vect_is_slp_reduction): Set LOOP_VINFO_OPERANDS_SWAPPED field.
> 	(vect_is_simple_reduction_1): Likewise.
> 
> gcc/testsuite:
> 
> 2012-07-30  Bill Schmidt  <wschmidt@linux.ibm.com>
> 
> 	PR tree-optimization/53773
> 	* testsuite/gcc.dg/vect/pr53773.c: New test.
> 
> 
> Index: gcc/testsuite/gcc.dg/vect/pr53773.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/vect/pr53773.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/vect/pr53773.c	(revision 0)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +
> +int
> +foo (int integral, int decimal, int power_ten)
> +{
> +  while (power_ten > 0)
> +    {
> +      integral *= 10;
> +      decimal *= 10;
> +      power_ten--;
> +    }
> +
> +  return integral+decimal;
> +}
> +
> +/* Two occurrences in annotations, two in code.  */
> +/* { dg-final { scan-tree-dump-times "\\* 10" 4 "vect" } } */
> +/* { dg-final { cleanup-tree-dump "vect" } } */
> +
> Index: gcc/tree-vectorizer.h
> ===================================================================
> --- gcc/tree-vectorizer.h	(revision 189938)
> +++ gcc/tree-vectorizer.h	(working copy)
> @@ -296,6 +296,12 @@ typedef struct _loop_vec_info {
>       this.  */
>    bool peeling_for_gaps;
>  
> +  /* Reductions are canonicalized so that the last operand is the reduction
> +     operand.  If this places a constant into RHS1, this decanonicalizes
> +     GIMPLE for other phases, so we must track when this has occurred and
> +     fix it up.  */
> +  bool operands_swapped;
> +
>  } *loop_vec_info;
>  
>  /* Access Functions.  */
> @@ -326,6 +332,7 @@ typedef struct _loop_vec_info {
>  #define LOOP_VINFO_PEELING_HTAB(L)         (L)->peeling_htab
>  #define LOOP_VINFO_TARGET_COST_DATA(L)     (L)->target_cost_data
>  #define LOOP_VINFO_PEELING_FOR_GAPS(L)     (L)->peeling_for_gaps
> +#define LOOP_VINFO_OPERANDS_SWAPPED(L)     (L)->operands_swapped
>  
>  #define LOOP_REQUIRES_VERSIONING_FOR_ALIGNMENT(L) \
>  VEC_length (gimple, (L)->may_misalign_stmts) > 0
> Index: gcc/tree-vect-loop.c
> ===================================================================
> --- gcc/tree-vect-loop.c	(revision 189938)
> +++ gcc/tree-vect-loop.c	(working copy)
> @@ -853,6 +853,7 @@ new_loop_vec_info (struct loop *loop)
>    LOOP_VINFO_PEELING_HTAB (res) = NULL;
>    LOOP_VINFO_TARGET_COST_DATA (res) = init_cost (loop);
>    LOOP_VINFO_PEELING_FOR_GAPS (res) = false;
> +  LOOP_VINFO_OPERANDS_SWAPPED (res) = false;
>  
>    return res;
>  }
> @@ -873,6 +874,7 @@ destroy_loop_vec_info (loop_vec_info loop_vinfo, b
>    int j;
>    VEC (slp_instance, heap) *slp_instances;
>    slp_instance instance;
> +  bool swapped;
>  
>    if (!loop_vinfo)
>      return;
> @@ -881,6 +883,7 @@ destroy_loop_vec_info (loop_vec_info loop_vinfo, b
>  
>    bbs = LOOP_VINFO_BBS (loop_vinfo);
>    nbbs = loop->num_nodes;
> +  swapped = LOOP_VINFO_OPERANDS_SWAPPED (loop_vinfo);
>  
>    if (!clean_stmts)
>      {
> @@ -905,6 +908,22 @@ destroy_loop_vec_info (loop_vec_info loop_vinfo, b
>        for (si = gsi_start_bb (bb); !gsi_end_p (si); )
>          {
>            gimple stmt = gsi_stmt (si);
> +
> +	  /* We may have broken canonical form by moving a constant
> +	     into RHS1 of a commutative op.  Fix such occurrences.  */
> +	  if (swapped && is_gimple_assign (stmt))
> +	    {
> +	      enum tree_code code = gimple_assign_rhs_code (stmt);
> +
> +	      if ((code == PLUS_EXPR
> +		   || code == POINTER_PLUS_EXPR
> +		   || code == MULT_EXPR)
> +		  && CONSTANT_CLASS_P (gimple_assign_rhs1 (stmt)))
> +		swap_tree_operands (stmt,
> +				    gimple_assign_rhs1_ptr (stmt),
> +				    gimple_assign_rhs2_ptr (stmt));
> +	    }
> +
>  	  /* Free stmt_vec_info.  */
>  	  free_stmt_vec_info (stmt);
>            gsi_next (&si);
> @@ -1920,6 +1939,9 @@ vect_is_slp_reduction (loop_vec_info loop_info, gi
>  	 		          gimple_assign_rhs1_ptr (next_stmt),
>                                    gimple_assign_rhs2_ptr (next_stmt));
>  	      update_stmt (next_stmt);
> +
> +	      if (CONSTANT_CLASS_P (gimple_assign_rhs1 (next_stmt)))
> +		LOOP_VINFO_OPERANDS_SWAPPED (loop_info) = true;
>  	    }
>  	  else
>  	    return false;
> @@ -2324,6 +2346,9 @@ vect_is_simple_reduction_1 (loop_vec_info loop_inf
>  
>            swap_tree_operands (def_stmt, gimple_assign_rhs1_ptr (def_stmt),
>   			      gimple_assign_rhs2_ptr (def_stmt));
> +
> +	  if (CONSTANT_CLASS_P (gimple_assign_rhs1 (def_stmt)))
> +	    LOOP_VINFO_OPERANDS_SWAPPED (loop_info) = true;
>          }
>        else
>          {
> 
> 
>

Patch

Index: gcc/testsuite/gcc.dg/vect/pr53773.c
===================================================================
--- gcc/testsuite/gcc.dg/vect/pr53773.c	(revision 0)
+++ gcc/testsuite/gcc.dg/vect/pr53773.c	(revision 0)
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+
+int
+foo (int integral, int decimal, int power_ten)
+{
+  while (power_ten > 0)
+    {
+      integral *= 10;
+      decimal *= 10;
+      power_ten--;
+    }
+
+  return integral+decimal;
+}
+
+/* Two occurrences in annotations, two in code.  */
+/* { dg-final { scan-tree-dump-times "\\* 10" 4 "vect" } } */
+/* { dg-final { cleanup-tree-dump "vect" } } */
+
Index: gcc/tree-vectorizer.h
===================================================================
--- gcc/tree-vectorizer.h	(revision 189938)
+++ gcc/tree-vectorizer.h	(working copy)
@@ -296,6 +296,12 @@  typedef struct _loop_vec_info {
      this.  */
   bool peeling_for_gaps;
 
+  /* Reductions are canonicalized so that the last operand is the reduction
+     operand.  If this places a constant into RHS1, this decanonicalizes
+     GIMPLE for other phases, so we must track when this has occurred and
+     fix it up.  */
+  bool operands_swapped;
+
 } *loop_vec_info;
 
 /* Access Functions.  */
@@ -326,6 +332,7 @@  typedef struct _loop_vec_info {
 #define LOOP_VINFO_PEELING_HTAB(L)         (L)->peeling_htab
 #define LOOP_VINFO_TARGET_COST_DATA(L)     (L)->target_cost_data
 #define LOOP_VINFO_PEELING_FOR_GAPS(L)     (L)->peeling_for_gaps
+#define LOOP_VINFO_OPERANDS_SWAPPED(L)     (L)->operands_swapped
 
 #define LOOP_REQUIRES_VERSIONING_FOR_ALIGNMENT(L) \
 VEC_length (gimple, (L)->may_misalign_stmts) > 0
Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	(revision 189938)
+++ gcc/tree-vect-loop.c	(working copy)
@@ -853,6 +853,7 @@  new_loop_vec_info (struct loop *loop)
   LOOP_VINFO_PEELING_HTAB (res) = NULL;
   LOOP_VINFO_TARGET_COST_DATA (res) = init_cost (loop);
   LOOP_VINFO_PEELING_FOR_GAPS (res) = false;
+  LOOP_VINFO_OPERANDS_SWAPPED (res) = false;
 
   return res;
 }
@@ -873,6 +874,7 @@  destroy_loop_vec_info (loop_vec_info loop_vinfo, b
   int j;
   VEC (slp_instance, heap) *slp_instances;
   slp_instance instance;
+  bool swapped;
 
   if (!loop_vinfo)
     return;
@@ -881,6 +883,7 @@  destroy_loop_vec_info (loop_vec_info loop_vinfo, b
 
   bbs = LOOP_VINFO_BBS (loop_vinfo);
   nbbs = loop->num_nodes;
+  swapped = LOOP_VINFO_OPERANDS_SWAPPED (loop_vinfo);
 
   if (!clean_stmts)
     {
@@ -905,6 +908,22 @@  destroy_loop_vec_info (loop_vec_info loop_vinfo, b
       for (si = gsi_start_bb (bb); !gsi_end_p (si); )
         {
           gimple stmt = gsi_stmt (si);
+
+	  /* We may have broken canonical form by moving a constant
+	     into RHS1 of a commutative op.  Fix such occurrences.  */
+	  if (swapped && is_gimple_assign (stmt))
+	    {
+	      enum tree_code code = gimple_assign_rhs_code (stmt);
+
+	      if ((code == PLUS_EXPR
+		   || code == POINTER_PLUS_EXPR
+		   || code == MULT_EXPR)
+		  && CONSTANT_CLASS_P (gimple_assign_rhs1 (stmt)))
+		swap_tree_operands (stmt,
+				    gimple_assign_rhs1_ptr (stmt),
+				    gimple_assign_rhs2_ptr (stmt));
+	    }
+
 	  /* Free stmt_vec_info.  */
 	  free_stmt_vec_info (stmt);
           gsi_next (&si);
@@ -1920,6 +1939,9 @@  vect_is_slp_reduction (loop_vec_info loop_info, gi
 	 		          gimple_assign_rhs1_ptr (next_stmt),
                                   gimple_assign_rhs2_ptr (next_stmt));
 	      update_stmt (next_stmt);
+
+	      if (CONSTANT_CLASS_P (gimple_assign_rhs1 (next_stmt)))
+		LOOP_VINFO_OPERANDS_SWAPPED (loop_info) = true;
 	    }
 	  else
 	    return false;
@@ -2324,6 +2346,9 @@  vect_is_simple_reduction_1 (loop_vec_info loop_inf
 
           swap_tree_operands (def_stmt, gimple_assign_rhs1_ptr (def_stmt),
  			      gimple_assign_rhs2_ptr (def_stmt));
+
+	  if (CONSTANT_CLASS_P (gimple_assign_rhs1 (def_stmt)))
+	    LOOP_VINFO_OPERANDS_SWAPPED (loop_info) = true;
         }
       else
         {