Fix EXTRACT_LAST_REDUCTION handling of pattern stmts
diff mbox series

Message ID mpteexc5siv.fsf@arm.com
State New
Headers show
Series
  • Fix EXTRACT_LAST_REDUCTION handling of pattern stmts
Related show

Commit Message

Richard Sandiford Dec. 10, 2019, 11:28 a.m. UTC
Unlike most vector ops, extract-last reductions replace the original
scalar code in-situ rather than adding an adjacent vector implementation.
I.e.:

   dest_1 = COND_EXPR <...>;

becomes:

   dest_1 = .EXTRACT_LAST (...);

gcc.dg/vect/vect-cond-reduc-4.c was ICEing for SVE because we tried
to replace the pattern statement in this way, rather than replacing
the original scalar statement.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2019-12-10  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* tree-vect-stmts.c (vect_finish_replace_stmt): Always use the
	original scalar statement rather than a pattern statement.
	(vectorizable_condition): Likewise, in the handling of extract-last
	reductions.

Comments

Richard Biener Dec. 10, 2019, 12:12 p.m. UTC | #1
On December 10, 2019 12:28:24 PM GMT+01:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>Unlike most vector ops, extract-last reductions replace the original
>scalar code in-situ rather than adding an adjacent vector
>implementation.
>I.e.:
>
>   dest_1 = COND_EXPR <...>;
>
>becomes:
>
>   dest_1 = .EXTRACT_LAST (...);
>
>gcc.dg/vect/vect-cond-reduc-4.c was ICEing for SVE because we tried
>to replace the pattern statement in this way, rather than replacing
>the original scalar statement.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Ok. 

Richard. 

>Richard
>
>
>2019-12-10  Richard Sandiford  <richard.sandiford@arm.com>
>
>gcc/
>	* tree-vect-stmts.c (vect_finish_replace_stmt): Always use the
>	original scalar statement rather than a pattern statement.
>	(vectorizable_condition): Likewise, in the handling of extract-last
>	reductions.
>
>Index: gcc/tree-vect-stmts.c
>===================================================================
>--- gcc/tree-vect-stmts.c	2019-12-10 09:17:18.768002919 +0000
>+++ gcc/tree-vect-stmts.c	2019-12-10 11:27:23.619355131 +0000
>@@ -1779,9 +1779,10 @@ vect_finish_stmt_generation_1 (stmt_vec_
> stmt_vec_info
> vect_finish_replace_stmt (stmt_vec_info stmt_info, gimple *vec_stmt)
> {
>-  gcc_assert (gimple_get_lhs (stmt_info->stmt) == gimple_get_lhs
>(vec_stmt));
>+  gimple *scalar_stmt = vect_orig_stmt (stmt_info)->stmt;
>+  gcc_assert (gimple_get_lhs (scalar_stmt) == gimple_get_lhs
>(vec_stmt));
> 
>-  gimple_stmt_iterator gsi = gsi_for_stmt (stmt_info->stmt);
>+  gimple_stmt_iterator gsi = gsi_for_stmt (scalar_stmt);
>   gsi_replace (&gsi, vec_stmt, true);
> 
>   return vect_finish_stmt_generation_1 (stmt_info, vec_stmt);
>@@ -10324,20 +10325,21 @@ vectorizable_condition (stmt_vec_info st
> 
> 	  if (reduction_type == EXTRACT_LAST_REDUCTION)
> 	    {
>+	      gimple *old_stmt = vect_orig_stmt (stmt_info)->stmt;
>+	      tree lhs = gimple_get_lhs (old_stmt);
> 	      gcall *new_stmt = gimple_build_call_internal
> 		(IFN_FOLD_EXTRACT_LAST, 3, else_clause, vec_compare,
> 		 vec_then_clause);
>-	      gimple_call_set_lhs (new_stmt, scalar_dest);
>-	      SSA_NAME_DEF_STMT (scalar_dest) = new_stmt;
>-	      if (stmt_info->stmt == gsi_stmt (*gsi))
>+	      gimple_call_set_lhs (new_stmt, lhs);
>+	      SSA_NAME_DEF_STMT (lhs) = new_stmt;
>+	      if (old_stmt == gsi_stmt (*gsi))
> 		new_stmt_info = vect_finish_replace_stmt (stmt_info, new_stmt);
> 	      else
> 		{
> 		  /* In this case we're moving the definition to later in the
> 		     block.  That doesn't matter because the only uses of the
> 		     lhs are in phi statements.  */
>-		  gimple_stmt_iterator old_gsi
>-		    = gsi_for_stmt (stmt_info->stmt);
>+		  gimple_stmt_iterator old_gsi = gsi_for_stmt (old_stmt);
> 		  gsi_remove (&old_gsi, true);
> 		  new_stmt_info
> 		    = vect_finish_stmt_generation (stmt_info, new_stmt, gsi);

Patch
diff mbox series

Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c	2019-12-10 09:17:18.768002919 +0000
+++ gcc/tree-vect-stmts.c	2019-12-10 11:27:23.619355131 +0000
@@ -1779,9 +1779,10 @@  vect_finish_stmt_generation_1 (stmt_vec_
 stmt_vec_info
 vect_finish_replace_stmt (stmt_vec_info stmt_info, gimple *vec_stmt)
 {
-  gcc_assert (gimple_get_lhs (stmt_info->stmt) == gimple_get_lhs (vec_stmt));
+  gimple *scalar_stmt = vect_orig_stmt (stmt_info)->stmt;
+  gcc_assert (gimple_get_lhs (scalar_stmt) == gimple_get_lhs (vec_stmt));
 
-  gimple_stmt_iterator gsi = gsi_for_stmt (stmt_info->stmt);
+  gimple_stmt_iterator gsi = gsi_for_stmt (scalar_stmt);
   gsi_replace (&gsi, vec_stmt, true);
 
   return vect_finish_stmt_generation_1 (stmt_info, vec_stmt);
@@ -10324,20 +10325,21 @@  vectorizable_condition (stmt_vec_info st
 
 	  if (reduction_type == EXTRACT_LAST_REDUCTION)
 	    {
+	      gimple *old_stmt = vect_orig_stmt (stmt_info)->stmt;
+	      tree lhs = gimple_get_lhs (old_stmt);
 	      gcall *new_stmt = gimple_build_call_internal
 		(IFN_FOLD_EXTRACT_LAST, 3, else_clause, vec_compare,
 		 vec_then_clause);
-	      gimple_call_set_lhs (new_stmt, scalar_dest);
-	      SSA_NAME_DEF_STMT (scalar_dest) = new_stmt;
-	      if (stmt_info->stmt == gsi_stmt (*gsi))
+	      gimple_call_set_lhs (new_stmt, lhs);
+	      SSA_NAME_DEF_STMT (lhs) = new_stmt;
+	      if (old_stmt == gsi_stmt (*gsi))
 		new_stmt_info = vect_finish_replace_stmt (stmt_info, new_stmt);
 	      else
 		{
 		  /* In this case we're moving the definition to later in the
 		     block.  That doesn't matter because the only uses of the
 		     lhs are in phi statements.  */
-		  gimple_stmt_iterator old_gsi
-		    = gsi_for_stmt (stmt_info->stmt);
+		  gimple_stmt_iterator old_gsi = gsi_for_stmt (old_stmt);
 		  gsi_remove (&old_gsi, true);
 		  new_stmt_info
 		    = vect_finish_stmt_generation (stmt_info, new_stmt, gsi);