[PATCH] Fix PR53733

[Richard Guenther]

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

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend