[PATCH] Fix PR66856

Richard Biener rguenther@suse.de
Fri Jan 15 12:17:00 GMT 2016


With the SLP tree "unsharing" stmts we can get mismatches as to
the expected operand ordering in different SLP node "uses".  The
following patch makes sure we have the same expectation everywhere
by only allowing the first SLP use to swap operands.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

2016-01-15  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/66856
	* tree-vect-loop.c (vect_transform_loop): Free SLP instances here.
	* tree-vect-slp.c (vect_free_slp_tree): Decrement stmt reference count.
	(vect_create_new_slp_node): Increment stmt reference count.
	(vect_get_and_check_slp_defs): Make sure stmts are nor already in
	an SLP tree before swapping operands.
	(vect_build_slp_tree): Likewise.
	(destroy_bb_vec_info): Free stmt info after SLP instances.
	* tree-vect-stmts.c (new_stmt_vec_info): Initialize reference count.
	* tree-vectorizer.h (struct _stmt_vec_info): Add num_slp_uses field.
	(STMT_VINFO_NUM_SLP_USES): New macro.

	* gcc.dg/torture/pr66856-1.c: New testcase.
	* gcc.dg/torture/pr66856-2.c: Likewise.

Index: gcc/tree-vect-loop.c
===================================================================
*** gcc/tree-vect-loop.c	(revision 232401)
--- gcc/tree-vect-loop.c	(working copy)
*************** vect_transform_loop (loop_vec_info loop_
*** 6930,6933 ****
--- 6932,6942 ----
  			 "OUTER LOOP VECTORIZED\n");
        dump_printf (MSG_NOTE, "\n");
      }
+ 
+   /* Free SLP instances here because otherwise stmt reference counting
+      won't work.  */
+   slp_instance instance;
+   FOR_EACH_VEC_ELT (LOOP_VINFO_SLP_INSTANCES (loop_vinfo), i, instance)
+     vect_free_slp_instance (instance);
+   LOOP_VINFO_SLP_INSTANCES (loop_vinfo).release ();
  }
Index: gcc/tree-vect-slp.c
===================================================================
*** gcc/tree-vect-slp.c	(revision 232401)
--- gcc/tree-vect-slp.c	(working copy)
*************** vect_free_slp_tree (slp_tree node)
*** 54,59 ****
--- 54,68 ----
    FOR_EACH_VEC_ELT (SLP_TREE_CHILDREN (node), i, child)
      vect_free_slp_tree (child);
  
+   gimple *stmt;
+   FOR_EACH_VEC_ELT (SLP_TREE_SCALAR_STMTS (node), i, stmt)
+     /* After transform some stmts are removed and thus their vinfo is gone.  */
+     if (vinfo_for_stmt (stmt))
+       {
+ 	gcc_assert (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) > 0);
+ 	STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))--;
+       }
+ 
    SLP_TREE_CHILDREN (node).release ();
    SLP_TREE_SCALAR_STMTS (node).release ();
    SLP_TREE_VEC_STMTS (node).release ();
*************** vect_create_new_slp_node (vec<gimple *>
*** 102,107 ****
--- 111,120 ----
    SLP_TREE_TWO_OPERATORS (node) = false;
    SLP_TREE_DEF_TYPE (node) = vect_internal_def;
  
+   unsigned i;
+   FOR_EACH_VEC_ELT (scalar_stmts, i, stmt)
+     STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt))++;
+ 
    return node;
  }
  
*************** again:
*** 401,406 ****
--- 414,433 ----
    /* Swap operands.  */
    if (swapped)
      {
+       /* If there are already uses of this stmt in a SLP instance then
+          we've committed to the operand order and can't swap it.  */
+       if (STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmt)) != 0)
+ 	{
+ 	  if (dump_enabled_p ())
+ 	    {
+ 	      dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+ 			       "Build SLP failed: cannot swap operands of "
+ 			       "shared stmt ");
+ 	      dump_gimple_stmt (MSG_MISSED_OPTIMIZATION, TDF_SLIM, stmt, 0);
+ 	    }
+ 	  return -1;
+ 	}
+ 
        if (first_op_cond)
  	{
  	  tree cond = gimple_assign_rhs1 (stmt);
*************** again:
*** 411,416 ****
--- 438,449 ----
        else
  	swap_ssa_operands (stmt, gimple_assign_rhs1_ptr (stmt),
  			   gimple_assign_rhs2_ptr (stmt));
+       if (dump_enabled_p ())
+ 	{
+ 	  dump_printf_loc (MSG_NOTE, vect_location,
+ 			   "swapped operands to match def types in ");
+ 	  dump_gimple_stmt (MSG_NOTE, TDF_SLIM, stmt, 0);
+ 	}
      }
  
    return 0;
*************** vect_build_slp_tree (vec_info *vinfo,
*** 1007,1012 ****
--- 1040,1062 ----
  	     behavior.  */
  	  && *npermutes < 4)
  	{
+ 	  /* Verify if we can safely swap or if we committed to a specific
+ 	     operand order already.  */
+ 	  for (j = 0; j < group_size; ++j)
+ 	    if (!matches[j]
+ 		&& STMT_VINFO_NUM_SLP_USES (vinfo_for_stmt (stmts[j])) != 0)
+ 	      {
+ 		if (dump_enabled_p ())
+ 		  {
+ 		    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+ 				     "Build SLP failed: cannot swap operands "
+ 				     "of shared stmt ");
+ 		    dump_gimple_stmt (MSG_MISSED_OPTIMIZATION, TDF_SLIM,
+ 				      stmts[j], 0);
+ 		  }
+ 		goto fail;
+ 	      }
+ 
  	  /* Swap mismatched definition stmts.  */
  	  dump_printf_loc (MSG_NOTE, vect_location,
  			   "Re-trying with swapped operands of stmts ");
*************** vect_build_slp_tree (vec_info *vinfo,
*** 1095,1100 ****
--- 1145,1151 ----
  	  ++*npermutes;
  	}
  
+ fail:
        gcc_assert (child == NULL);
        FOR_EACH_VEC_ELT (children, j, child)
  	vect_free_slp_tree (child);
*************** new_bb_vec_info (gimple_stmt_iterator re
*** 2184,2201 ****
  static void
  destroy_bb_vec_info (bb_vec_info bb_vinfo)
  {
-   vec<slp_instance> slp_instances;
    slp_instance instance;
-   basic_block bb;
-   gimple_stmt_iterator si;
    unsigned i;
  
    if (!bb_vinfo)
      return;
  
!   bb = BB_VINFO_BB (bb_vinfo);
  
!   for (si = bb_vinfo->region_begin;
         gsi_stmt (si) != gsi_stmt (bb_vinfo->region_end); gsi_next (&si))
      {
        gimple *stmt = gsi_stmt (si);
--- 2237,2257 ----
  static void
  destroy_bb_vec_info (bb_vec_info bb_vinfo)
  {
    slp_instance instance;
    unsigned i;
  
    if (!bb_vinfo)
      return;
  
!   vect_destroy_datarefs (bb_vinfo);
!   free_dependence_relations (BB_VINFO_DDRS (bb_vinfo));
!   BB_VINFO_GROUPED_STORES (bb_vinfo).release ();
!   FOR_EACH_VEC_ELT (BB_VINFO_SLP_INSTANCES (bb_vinfo), i, instance)
!     vect_free_slp_instance (instance);
!   BB_VINFO_SLP_INSTANCES (bb_vinfo).release ();
!   destroy_cost_data (BB_VINFO_TARGET_COST_DATA (bb_vinfo));
  
!   for (gimple_stmt_iterator si = bb_vinfo->region_begin;
         gsi_stmt (si) != gsi_stmt (bb_vinfo->region_end); gsi_next (&si))
      {
        gimple *stmt = gsi_stmt (si);
*************** destroy_bb_vec_info (bb_vec_info bb_vinf
*** 2209,2224 ****
        gimple_set_uid (stmt, -1);
      }
  
!   vect_destroy_datarefs (bb_vinfo);
!   free_dependence_relations (BB_VINFO_DDRS (bb_vinfo));
!   BB_VINFO_GROUPED_STORES (bb_vinfo).release ();
!   slp_instances = BB_VINFO_SLP_INSTANCES (bb_vinfo);
!   FOR_EACH_VEC_ELT (slp_instances, i, instance)
!     vect_free_slp_instance (instance);
!   BB_VINFO_SLP_INSTANCES (bb_vinfo).release ();
!   destroy_cost_data (BB_VINFO_TARGET_COST_DATA (bb_vinfo));
    free (bb_vinfo);
-   bb->aux = NULL;
  }
  
  
--- 2265,2272 ----
        gimple_set_uid (stmt, -1);
      }
  
!   BB_VINFO_BB (bb_vinfo)->aux = NULL;
    free (bb_vinfo);
  }
  
  
Index: gcc/tree-vect-stmts.c
===================================================================
*** gcc/tree-vect-stmts.c	(revision 232401)
--- gcc/tree-vect-stmts.c	(working copy)
*************** new_stmt_vec_info (gimple *stmt, vec_inf
*** 8391,8396 ****
--- 8393,8400 ----
  
    STMT_VINFO_SAME_ALIGN_REFS (res).create (0);
    STMT_SLP_TYPE (res) = loop_vect;
+   STMT_VINFO_NUM_SLP_USES (res) = 0;
+ 
    GROUP_FIRST_ELEMENT (res) = NULL;
    GROUP_NEXT_ELEMENT (res) = NULL;
    GROUP_SIZE (res) = 0;
Index: gcc/tree-vectorizer.h
===================================================================
*** gcc/tree-vectorizer.h	(revision 232401)
--- gcc/tree-vectorizer.h	(working copy)
*************** typedef struct _stmt_vec_info {
*** 601,606 ****
--- 601,608 ----
    /* For reduction loops, this is the type of reduction.  */
    enum vect_reduction_type v_reduc_type;
  
+   /* The number of scalar stmt references from active SLP instances.  */
+   unsigned int num_slp_uses;
  } *stmt_vec_info;
  
  /* Access Functions.  */
*************** STMT_VINFO_BB_VINFO (stmt_vec_info stmt_
*** 653,658 ****
--- 655,661 ----
  #define STMT_VINFO_LOOP_PHI_EVOLUTION_BASE_UNCHANGED(S) (S)->loop_phi_evolution_base_unchanged
  #define STMT_VINFO_LOOP_PHI_EVOLUTION_PART(S) (S)->loop_phi_evolution_part
  #define STMT_VINFO_MIN_NEG_DIST(S)	(S)->min_neg_dist
+ #define STMT_VINFO_NUM_SLP_USES(S)	(S)->num_slp_uses
  
  #define GROUP_FIRST_ELEMENT(S)          (S)->first_element
  #define GROUP_NEXT_ELEMENT(S)           (S)->next_element
Index: gcc/testsuite/gcc.dg/torture/pr66856-1.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr66856-1.c	(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr66856-1.c	(working copy)
***************
*** 0 ****
--- 1,24 ----
+ /* { dg-do compile } */
+ /* { dg-additional-options "-mavx2" { target x86_64-*-* i?86-*-* } } */
+ 
+ short c;
+ int d;
+ int fn1(int p1, int p2) {
+     int a, b;
+     a = p1 >> 3 & p2;
+     b = p1 & 072;
+     a |= a >> 5;
+     a |= b >> 5;
+     return a;
+ }
+ void fn2() {
+     short *e = &c;
+     int *f;
+     int g;
+     while (d -= 4) {
+ 	fn1(1, 1);
+ 	fn1(1, 1) * fn1(1, 1) * fn1(1, 1);
+ 	*e++ = fn1(*f++, g);
+ 	*e++ = fn1(*f++, g);
+     }
+ }
Index: gcc/testsuite/gcc.dg/torture/pr66856-2.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr66856-2.c	(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr66856-2.c	(working copy)
***************
*** 0 ****
--- 1,26 ----
+ /* { dg-do compile } */
+ 
+ typedef int uint32_t;
+ int c, e, f, g, h;
+ short *d;
+ uint32_t fn1(uint32_t p1, uint32_t p2)
+ {
+   uint32_t a, b;
+   a = p1 >> 3 & p2;
+   b = p1 & 072;
+   a |= a >> 5;
+   a |= b >> 5;
+   return a;
+ }
+ 
+ void fn2()
+ {
+   uint32_t *i;
+   uint32_t j;
+   while (c -= 4) {
+       fn1(e, j);
+       fn1(f, j) * fn1(g, j) * fn1(h, j);
+       *d++ = fn1(*i++, j);
+       *d++ = fn1(*i++, j);
+   }
+ }



More information about the Gcc-patches mailing list