Fix PR84737, 172.mgrid regression on powerpc

Message ID alpine.LSU.2.20.1804191017240.31014@zhemvz.fhfr.qr
State New
Headers show
Series
  • Fix PR84737, 172.mgrid regression on powerpc
Related show

Commit Message

Richard Biener April 19, 2018, 8:21 a.m.
The following fixes the fact that the vectorizer doesn't bother to
preserve restrict information on its generated loads/stores which
in turn causes missed predictive commonings.  The regression happened
because there's now a IPA-CP clone which wrecks points-to info,
but restrict is properly preserved.  But with the vectorizer
throwing it away we lost all of the info.

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

Dependend on how close we are to a RC I consider to fix this
only for GCC 8.2.

Richard.

2018-04-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/84737
	* tree-vect-data-refs.c (vect_copy_ref_info): New function
	copying restrict info.
	(vect_setup_realignment): Use it.
	* tree-vectorizer.h (vect_copy_ref_info): Declare.
	* tree-vect-stmts.c (vectorizable_store): Copy ref info from
	the first DR to all generated stores.
	(vectorizable_load): Likewise for loads.

Comments

Jakub Jelinek April 19, 2018, 8:41 a.m. | #1
On Thu, Apr 19, 2018 at 10:21:32AM +0200, Richard Biener wrote:
> 

> The following fixes the fact that the vectorizer doesn't bother to

> preserve restrict information on its generated loads/stores which

> in turn causes missed predictive commonings.  The regression happened

> because there's now a IPA-CP clone which wrecks points-to info,

> but restrict is properly preserved.  But with the vectorizer

> throwing it away we lost all of the info.

> 

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

> 

> Dependend on how close we are to a RC I consider to fix this

> only for GCC 8.2.


I'd put it in now, there is still work to do on the C++ P1s.

> 2018-04-19  Richard Biener  <rguenther@suse.de>

> 

> 	PR tree-optimization/84737

> 	* tree-vect-data-refs.c (vect_copy_ref_info): New function

> 	copying restrict info.

> 	(vect_setup_realignment): Use it.

> 	* tree-vectorizer.h (vect_copy_ref_info): Declare.

> 	* tree-vect-stmts.c (vectorizable_store): Copy ref info from

> 	the first DR to all generated stores.

> 	(vectorizable_load): Likewise for loads.


	Jakub
Richard Biener April 19, 2018, 12:40 p.m. | #2
On Thu, 19 Apr 2018, Jakub Jelinek wrote:

> On Thu, Apr 19, 2018 at 10:21:32AM +0200, Richard Biener wrote:

> > 

> > The following fixes the fact that the vectorizer doesn't bother to

> > preserve restrict information on its generated loads/stores which

> > in turn causes missed predictive commonings.  The regression happened

> > because there's now a IPA-CP clone which wrecks points-to info,

> > but restrict is properly preserved.  But with the vectorizer

> > throwing it away we lost all of the info.

> > 

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

> > 

> > Dependend on how close we are to a RC I consider to fix this

> > only for GCC 8.2.

> 

> I'd put it in now, there is still work to do on the C++ P1s.


This is what I committed after also building SPEC 2k and 2k6.

Richard.

2018-04-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/84737
	* tree-vect-data-refs.c (vect_copy_ref_info): New function
	copying restrict info.
	(vect_setup_realignment): Use it.
	* tree-vectorizer.h (vect_copy_ref_info): Declare.
	* tree-vect-stmts.c (vectorizable_store): Copy ref info from
	the first DR to all generated stores.
	(vectorizable_load): Likewise for loads.

Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c	(revision 259489)
+++ gcc/tree-vect-data-refs.c	(working copy)
@@ -5010,6 +5010,27 @@ bump_vector_ptr (tree dataref_ptr, gimpl
 }
 
 
+/* Copy memory reference info such as base/clique from the SRC reference
+   to the DEST MEM_REF.  */
+
+void
+vect_copy_ref_info (tree dest, tree src)
+{
+  if (TREE_CODE (dest) != MEM_REF)
+    return;
+
+  tree src_base = src;
+  while (handled_component_p (src_base))
+    src_base = TREE_OPERAND (src_base, 0);
+  if (TREE_CODE (src_base) != MEM_REF
+      && TREE_CODE (src_base) != TARGET_MEM_REF)
+    return;
+
+  MR_DEPENDENCE_CLIQUE (dest) = MR_DEPENDENCE_CLIQUE (src_base);
+  MR_DEPENDENCE_BASE (dest) = MR_DEPENDENCE_BASE (src_base);
+}
+
+
 /* Function vect_create_destination_var.
 
    Create a new temporary of type VECTYPE.  */
@@ -5561,6 +5582,7 @@ vect_setup_realignment (gimple *stmt, gi
       data_ref
 	= build2 (MEM_REF, TREE_TYPE (vec_dest), new_temp,
 		  build_int_cst (reference_alias_ptr_type (DR_REF (dr)), 0));
+      vect_copy_ref_info (data_ref, DR_REF (dr));
       new_stmt = gimple_build_assign (vec_dest, data_ref);
       new_temp = make_ssa_name (vec_dest, new_stmt);
       gimple_assign_set_lhs (new_stmt, new_temp);
Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c	(revision 259489)
+++ gcc/tree-vect-stmts.c	(working copy)
@@ -6660,6 +6660,7 @@ vectorizable_store (gimple *stmt, gimple
 						 group_el * elsz);
 		  newref = build2 (MEM_REF, ltype,
 				   running_off, this_off);
+		  vect_copy_ref_info (newref, DR_REF (first_dr));
 
 		  /* And store it to *running_off.  */
 		  assign = gimple_build_assign (newref, elem);
@@ -7052,6 +7053,7 @@ vectorizable_store (gimple *stmt, gimple
 		    TREE_TYPE (data_ref)
 		      = build_aligned_type (TREE_TYPE (data_ref),
 					    TYPE_ALIGN (elem_type));
+		  vect_copy_ref_info (data_ref, DR_REF (first_dr));
 		  new_stmt = gimple_build_assign (data_ref, vec_oprnd);
 		}
 	      vect_finish_stmt_generation (stmt, new_stmt, gsi);
@@ -7659,9 +7661,9 @@ vectorizable_load (gimple *stmt, gimple_
 	    {
 	      tree this_off = build_int_cst (TREE_TYPE (alias_off),
 					     group_el * elsz + cst_offset);
-	      new_stmt = gimple_build_assign (make_ssa_name (ltype),
-					      build2 (MEM_REF, ltype,
-						      running_off, this_off));
+	      tree data_ref = build2 (MEM_REF, ltype, running_off, this_off);
+	      vect_copy_ref_info (data_ref, DR_REF (first_dr));
+	      new_stmt = gimple_build_assign (make_ssa_name (ltype), data_ref);
 	      vect_finish_stmt_generation (stmt, new_stmt, gsi);
 	      if (nloads > 1)
 		CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
@@ -8205,6 +8207,7 @@ vectorizable_load (gimple *stmt, gimple_
 		    data_ref
 		      = build2 (MEM_REF, vectype, ptr,
 				build_int_cst (ref_type, 0));
+		    vect_copy_ref_info (data_ref, DR_REF (first_dr));
 		    vec_dest = vect_create_destination_var (scalar_dest,
 							    vectype);
 		    new_stmt = gimple_build_assign (vec_dest, data_ref);
@@ -8254,7 +8257,10 @@ vectorizable_load (gimple *stmt, gimple_
 	      vec_dest = vect_create_destination_var (scalar_dest, vectype);
 	      /* DATA_REF is null if we've already built the statement.  */
 	      if (data_ref)
-		new_stmt = gimple_build_assign (vec_dest, data_ref);
+		{
+		  vect_copy_ref_info (data_ref, DR_REF (first_dr));
+		  new_stmt = gimple_build_assign (vec_dest, data_ref);
+		}
 	      new_temp = make_ssa_name (vec_dest, new_stmt);
 	      gimple_set_lhs (new_stmt, new_temp);
 	      vect_finish_stmt_generation (stmt, new_stmt, gsi);
Index: gcc/tree-vectorizer.h
===================================================================
--- gcc/tree-vectorizer.h	(revision 259489)
+++ gcc/tree-vectorizer.h	(working copy)
@@ -1494,6 +1494,7 @@ extern tree vect_create_data_ref_ptr (gi
 				      tree = NULL_TREE, tree = NULL_TREE);
 extern tree bump_vector_ptr (tree, gimple *, gimple_stmt_iterator *, gimple *,
 			     tree);
+extern void vect_copy_ref_info (tree, tree);
 extern tree vect_create_destination_var (tree, tree);
 extern bool vect_grouped_store_supported (tree, unsigned HOST_WIDE_INT);
 extern bool vect_store_lanes_supported (tree, unsigned HOST_WIDE_INT, bool);

Patch

Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c	(revision 259489)
+++ gcc/tree-vect-data-refs.c	(working copy)
@@ -5010,6 +5010,24 @@  bump_vector_ptr (tree dataref_ptr, gimpl
 }
 
 
+/* Copy memory reference info such as base/clique from the SRC reference
+   to the DEST MEM_REF.  */
+
+void
+vect_copy_ref_info (tree dest, tree src)
+{
+  tree src_base = src;
+  while (handled_component_p (src_base))
+    src_base = TREE_OPERAND (src_base, 0);
+  if (TREE_CODE (src_base) != MEM_REF
+      && TREE_CODE (src_base) != TARGET_MEM_REF)
+    return;
+
+  MR_DEPENDENCE_CLIQUE (dest) = MR_DEPENDENCE_CLIQUE (src_base);
+  MR_DEPENDENCE_BASE (dest) = MR_DEPENDENCE_BASE (src_base);
+}
+
+
 /* Function vect_create_destination_var.
 
    Create a new temporary of type VECTYPE.  */
@@ -5561,6 +5579,7 @@  vect_setup_realignment (gimple *stmt, gi
       data_ref
 	= build2 (MEM_REF, TREE_TYPE (vec_dest), new_temp,
 		  build_int_cst (reference_alias_ptr_type (DR_REF (dr)), 0));
+      vect_copy_ref_info (data_ref, DR_REF (dr));
       new_stmt = gimple_build_assign (vec_dest, data_ref);
       new_temp = make_ssa_name (vec_dest, new_stmt);
       gimple_assign_set_lhs (new_stmt, new_temp);
Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c	(revision 259489)
+++ gcc/tree-vect-stmts.c	(working copy)
@@ -6660,6 +6660,7 @@  vectorizable_store (gimple *stmt, gimple
 						 group_el * elsz);
 		  newref = build2 (MEM_REF, ltype,
 				   running_off, this_off);
+		  vect_copy_ref_info (newref, DR_REF (first_dr));
 
 		  /* And store it to *running_off.  */
 		  assign = gimple_build_assign (newref, elem);
@@ -7052,6 +7053,7 @@  vectorizable_store (gimple *stmt, gimple
 		    TREE_TYPE (data_ref)
 		      = build_aligned_type (TREE_TYPE (data_ref),
 					    TYPE_ALIGN (elem_type));
+		  vect_copy_ref_info (data_ref, DR_REF (first_dr));
 		  new_stmt = gimple_build_assign (data_ref, vec_oprnd);
 		}
 	      vect_finish_stmt_generation (stmt, new_stmt, gsi);
@@ -7659,9 +7661,9 @@  vectorizable_load (gimple *stmt, gimple_
 	    {
 	      tree this_off = build_int_cst (TREE_TYPE (alias_off),
 					     group_el * elsz + cst_offset);
-	      new_stmt = gimple_build_assign (make_ssa_name (ltype),
-					      build2 (MEM_REF, ltype,
-						      running_off, this_off));
+	      tree data_ref = build2 (MEM_REF, ltype, running_off, this_off);
+	      vect_copy_ref_info (data_ref, DR_REF (first_dr));
+	      new_stmt = gimple_build_assign (make_ssa_name (ltype), data_ref);
 	      vect_finish_stmt_generation (stmt, new_stmt, gsi);
 	      if (nloads > 1)
 		CONSTRUCTOR_APPEND_ELT (v, NULL_TREE,
@@ -8205,6 +8207,7 @@  vectorizable_load (gimple *stmt, gimple_
 		    data_ref
 		      = build2 (MEM_REF, vectype, ptr,
 				build_int_cst (ref_type, 0));
+		    vect_copy_ref_info (data_ref, DR_REF (first_dr));
 		    vec_dest = vect_create_destination_var (scalar_dest,
 							    vectype);
 		    new_stmt = gimple_build_assign (vec_dest, data_ref);
@@ -8251,6 +8254,7 @@  vectorizable_load (gimple *stmt, gimple_
 		default:
 		  gcc_unreachable ();
 		}
+	      vect_copy_ref_info (data_ref, DR_REF (first_dr));
 	      vec_dest = vect_create_destination_var (scalar_dest, vectype);
 	      /* DATA_REF is null if we've already built the statement.  */
 	      if (data_ref)
Index: gcc/tree-vectorizer.h
===================================================================
--- gcc/tree-vectorizer.h	(revision 259489)
+++ gcc/tree-vectorizer.h	(working copy)
@@ -1494,6 +1494,7 @@  extern tree vect_create_data_ref_ptr (gi
 				      tree = NULL_TREE, tree = NULL_TREE);
 extern tree bump_vector_ptr (tree, gimple *, gimple_stmt_iterator *, gimple *,
 			     tree);
+extern void vect_copy_ref_info (tree, tree);
 extern tree vect_create_destination_var (tree, tree);
 extern bool vect_grouped_store_supported (tree, unsigned HOST_WIDE_INT);
 extern bool vect_store_lanes_supported (tree, unsigned HOST_WIDE_INT, bool);