Patchwork [3/9] STMT_VINFO_RELATED_STMT handling in vectorizable_store

login
register
mail settings
Submitter Richard Sandiford
Date April 12, 2011, 1:38 p.m.
Message ID <g4d3krfv2p.fsf@linaro.org>
Download mbox | patch
Permalink /patch/90802/
State New
Headers show

Comments

Richard Sandiford - April 12, 2011, 1:38 p.m.
vectorizable_store contains the code:

  for (j = 0; j < ncopies; j++)
    {
      for (i = 0; i < vec_num; i++)
	{
	  ...
	      if (j == 0)
	        STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt = new_stmt;
	      else
	        STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt;
	      prev_stmt_info = vinfo_for_stmt (new_stmt);
         }
    }

That is, STMT_VINFO_VEC_STMT (stmt_info) and *vec_stmt contain the last
statement emitted for the _last_ vector of the first copy.  However,
for later copies, the last statement for _every_ vector is chained using
STMT_VINFO_RELATED_STMT.  This seems a bit inconsistent, and isn't
what I expected from the comments.  It also seems different from
other vectorisation functions, where each copy has exactly one
STMT_VINFO_RELATED_STMT.  I wasn't sure whether the difference here
was deliberate or not.

The reason I'm changing it is that it makes the control flow for
the new code more obvious.

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

Richard


gcc/
	* tree-vect-stmts.c (vectorizable_store): Only chain one related
	statement per copy.
Ira Rosen - April 17, 2011, 10:17 a.m.
gcc-patches-owner@gcc.gnu.org wrote on 12/04/2011 04:38:54 PM:

> vectorizable_store contains the code:
>
>   for (j = 0; j < ncopies; j++)
>     {
>       for (i = 0; i < vec_num; i++)
>    {
>      ...
>          if (j == 0)
>            STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt = new_stmt;
>          else
>            STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt;
>          prev_stmt_info = vinfo_for_stmt (new_stmt);
>          }
>     }
>
> That is, STMT_VINFO_VEC_STMT (stmt_info) and *vec_stmt contain the last
> statement emitted for the _last_ vector of the first copy.  However,
> for later copies, the last statement for _every_ vector is chained using
> STMT_VINFO_RELATED_STMT.  This seems a bit inconsistent, and isn't
> what I expected from the comments.  It also seems different from
> other vectorisation functions, where each copy has exactly one
> STMT_VINFO_RELATED_STMT.  I wasn't sure whether the difference here
> was deliberate or not.

I think it doesn't really matter because STMT_VINFO_RELATED_STMT is used
for retrieving copies of vector operands, and stores don't define any.

>
> The reason I'm changing it is that it makes the control flow for
> the new code more obvious.
>
> Tested on x86_64-linux-gnu and arm-linux-gnueabi.  OK to install?

OK.

Thanks,
Ira

>
> Richard
>
>
> gcc/
>    * tree-vect-stmts.c (vectorizable_store): Only chain one related
>    statement per copy.
>
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c   2011-04-12 11:55:08.000000000 +0100
> +++ gcc/tree-vect-stmts.c   2011-04-12 11:55:09.000000000 +0100
> @@ -3612,6 +3612,7 @@ vectorizable_store (gimple stmt, gimple_
>
>        if (1)
>     {
> +     new_stmt = NULL;
>       if (strided_store)
>         {
>           result_chain = VEC_alloc (tree, heap, group_size);
> @@ -3669,17 +3670,19 @@ vectorizable_store (gimple stmt, gimple_
>           if (slp)
>        continue;
>
> -         if (j == 0)
> -      STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt =  new_stmt;
> -         else
> -      STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt;
> -
> -         prev_stmt_info = vinfo_for_stmt (new_stmt);
>           next_stmt = DR_GROUP_NEXT_DR (vinfo_for_stmt (next_stmt));
>           if (!next_stmt)
>        break;
>         }
>     }
> +      if (!slp)
> +   {
> +     if (j == 0)
> +       STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt =  new_stmt;
> +     else
> +       STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt;
> +     prev_stmt_info = vinfo_for_stmt (new_stmt);
> +   }
>      }
>
>    VEC_free (tree, heap, dr_chain);

Patch

Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c	2011-04-12 11:55:08.000000000 +0100
+++ gcc/tree-vect-stmts.c	2011-04-12 11:55:09.000000000 +0100
@@ -3612,6 +3612,7 @@  vectorizable_store (gimple stmt, gimple_
 
       if (1)
 	{
+	  new_stmt = NULL;
 	  if (strided_store)
 	    {
 	      result_chain = VEC_alloc (tree, heap, group_size);
@@ -3669,17 +3670,19 @@  vectorizable_store (gimple stmt, gimple_
 	      if (slp)
 		continue;
 
-	      if (j == 0)
-		STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt =  new_stmt;
-	      else
-		STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt;
-
-	      prev_stmt_info = vinfo_for_stmt (new_stmt);
 	      next_stmt = DR_GROUP_NEXT_DR (vinfo_for_stmt (next_stmt));
 	      if (!next_stmt)
 		break;
 	    }
 	}
+      if (!slp)
+	{
+	  if (j == 0)
+	    STMT_VINFO_VEC_STMT (stmt_info) = *vec_stmt =  new_stmt;
+	  else
+	    STMT_VINFO_RELATED_STMT (prev_stmt_info) = new_stmt;
+	  prev_stmt_info = vinfo_for_stmt (new_stmt);
+	}
     }
 
   VEC_free (tree, heap, dr_chain);