Patchwork SLP for vectors

login
register
mail settings
Submitter Marc Glisse
Date March 30, 2013, 4:14 p.m.
Message ID <alpine.DEB.2.02.1303301620340.10575@stedding.saclay.inria.fr>
Download mbox | patch
Permalink /patch/232528/
State New
Headers show

Comments

Marc Glisse - March 30, 2013, 4:14 p.m.
On Tue, 29 Jan 2013, Richard Biener wrote:

> So yes, handling BIT_FIELD_REF in the vectorizer looks like the correct
> way to do - but mind that you should constrain the BIT_FIELD_REFs you
> allow (I suppose in the end that's properly done by other part of the analysis).

Does that mean adding something like this to vectorizable_store? (and 
similarly to *_load)

if (VECTOR_TYPE_P (TREE_TYPE (scalar_dest)))
   return false;

Or maybe this?

if (TREE_CODE (scalar_dest) == BIT_FIELD_REF
     && !VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (scalar_dest, 0)))
   return false;

Or checking that the type of the BIT_FIELD_REF is the element type of the 
vector type it accesses?

I am not sure what restrictions are needed.

> I suppose the data-ref analysis parts are not strictly required,

I removed the dr_analyze_indices part, which was indeed not necessary. The 
tree-vect-data-refs.c part is useless (the base object is not computed for 
SLP) but shouldn't hurt.

The current patch is attached (passed bootstrap+testsuite a week ago).
Are there some parts of this that could go in?


2013-03-30  Marc Glisse  <marc.glisse@inria.fr>

 	* tree-vect-stmts.c (vectorizable_store): Accept BIT_FIELD_REF.
 	(vectorizable_load): Likewise.
 	* tree-vect-slp.c (vect_build_slp_tree): Likewise.
 	* tree-vect-data-refs.c (vect_create_data_ref_ptr): Handle VECTOR_TYPE.
 	* tree-flow-inline.h (get_addr_base_and_unit_offset_1): Handle
 	BIT_FIELD_REF.

testsuite/
 	* gcc.dg/vect/bb-slp-31.c: New file.
Marc Glisse - April 1, 2013, 3:52 p.m.
On Sat, 30 Mar 2013, Marc Glisse wrote:

> 	* tree-flow-inline.h (get_addr_base_and_unit_offset_1): Handle
> 	BIT_FIELD_REF.

I wrote a safer version of this for PR52436:


  	case BIT_FIELD_REF:
-	  return NULL_TREE;
+	  {
+	    HOST_WIDE_INT this_off = TREE_INT_CST_LOW (TREE_OPERAND (exp, 2));
+	    if (this_off % BITS_PER_UNIT)
+	      return NULL_TREE;
+	    byte_offset += this_off / BITS_PER_UNIT;
+	  }
+	  break;
Richard Guenther - April 2, 2013, 12:47 p.m.
On Mon, Apr 1, 2013 at 5:52 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sat, 30 Mar 2013, Marc Glisse wrote:
>
>>         * tree-flow-inline.h (get_addr_base_and_unit_offset_1): Handle
>>         BIT_FIELD_REF.
>
>
> I wrote a safer version of this for PR52436:

That variant is ok - please test and commit separately.

Thanks,
Richard.

>
>
>         case BIT_FIELD_REF:
> -         return NULL_TREE;
> +         {
> +           HOST_WIDE_INT this_off = TREE_INT_CST_LOW (TREE_OPERAND (exp,
> 2));
> +           if (this_off % BITS_PER_UNIT)
> +             return NULL_TREE;
> +           byte_offset += this_off / BITS_PER_UNIT;
> +         }
> +         break;
Richard Guenther - April 2, 2013, 12:52 p.m.
On Sat, Mar 30, 2013 at 5:14 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Tue, 29 Jan 2013, Richard Biener wrote:
>
>> So yes, handling BIT_FIELD_REF in the vectorizer looks like the correct
>> way to do - but mind that you should constrain the BIT_FIELD_REFs you
>> allow (I suppose in the end that's properly done by other part of the
>> analysis).
>
>
> Does that mean adding something like this to vectorizable_store? (and
> similarly to *_load)
>
> if (VECTOR_TYPE_P (TREE_TYPE (scalar_dest)))
>   return false;
>
> Or maybe this?
>
> if (TREE_CODE (scalar_dest) == BIT_FIELD_REF
>     && !VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (scalar_dest, 0)))
>   return false;
>
> Or checking that the type of the BIT_FIELD_REF is the element type of the
> vector type it accesses?
>
> I am not sure what restrictions are needed.
>
>
>> I suppose the data-ref analysis parts are not strictly required,
>
>
> I removed the dr_analyze_indices part, which was indeed not necessary. The
> tree-vect-data-refs.c part is useless (the base object is not computed for
> SLP) but shouldn't hurt.
>
> The current patch is attached (passed bootstrap+testsuite a week ago).
> Are there some parts of this that could go in?

The patch is ok apart from the tree-flow-inline.h parts where you have posted
a correct variant in a followup.

Thanks,
Richard.

>
> 2013-03-30  Marc Glisse  <marc.glisse@inria.fr>
>
>         * tree-vect-stmts.c (vectorizable_store): Accept BIT_FIELD_REF.
>         (vectorizable_load): Likewise.
>         * tree-vect-slp.c (vect_build_slp_tree): Likewise.
>         * tree-vect-data-refs.c (vect_create_data_ref_ptr): Handle
> VECTOR_TYPE.
>         * tree-flow-inline.h (get_addr_base_and_unit_offset_1): Handle
>         BIT_FIELD_REF.
>
> testsuite/
>         * gcc.dg/vect/bb-slp-31.c: New file.
>
> --
> Marc Glisse
> Index: tree-vect-data-refs.c
> ===================================================================
> --- tree-vect-data-refs.c       (revision 197265)
> +++ tree-vect-data-refs.c       (working copy)
> @@ -3565,20 +3565,22 @@ vect_create_data_ref_ptr (gimple stmt, t
>
>    if (dump_enabled_p ())
>      {
>        tree dr_base_type = TREE_TYPE (DR_BASE_OBJECT (dr));
>        dump_printf_loc (MSG_NOTE, vect_location,
>                         "create %s-pointer variable to type: ",
>                         tree_code_name[(int) TREE_CODE (aggr_type)]);
>        dump_generic_expr (MSG_NOTE, TDF_SLIM, aggr_type);
>        if (TREE_CODE (dr_base_type) == ARRAY_TYPE)
>          dump_printf (MSG_NOTE, "  vectorizing an array ref: ");
> +      else if (TREE_CODE (dr_base_type) == VECTOR_TYPE)
> +        dump_printf (MSG_NOTE, "  vectorizing a vector ref: ");
>        else if (TREE_CODE (dr_base_type) == RECORD_TYPE)
>          dump_printf (MSG_NOTE, "  vectorizing a record based array ref: ");
>        else
>          dump_printf (MSG_NOTE, "  vectorizing a pointer ref: ");
>        dump_generic_expr (MSG_NOTE, TDF_SLIM, DR_BASE_OBJECT (dr));
>      }
>
>    /* (1) Create the new aggregate-pointer variable.
>       Vector and array types inherit the alias set of their component
>       type by default so we need to use a ref-all pointer if the data
> Index: tree-vect-stmts.c
> ===================================================================
> --- tree-vect-stmts.c   (revision 197265)
> +++ tree-vect-stmts.c   (working copy)
> @@ -3844,20 +3844,21 @@ vectorizable_store (gimple stmt, gimple_
>    /* Is vectorizable store? */
>
>    if (!is_gimple_assign (stmt))
>      return false;
>
>    scalar_dest = gimple_assign_lhs (stmt);
>    if (TREE_CODE (scalar_dest) == VIEW_CONVERT_EXPR
>        && is_pattern_stmt_p (stmt_info))
>      scalar_dest = TREE_OPERAND (scalar_dest, 0);
>    if (TREE_CODE (scalar_dest) != ARRAY_REF
> +      && TREE_CODE (scalar_dest) != BIT_FIELD_REF
>        && TREE_CODE (scalar_dest) != INDIRECT_REF
>        && TREE_CODE (scalar_dest) != COMPONENT_REF
>        && TREE_CODE (scalar_dest) != IMAGPART_EXPR
>        && TREE_CODE (scalar_dest) != REALPART_EXPR
>        && TREE_CODE (scalar_dest) != MEM_REF)
>      return false;
>
>    gcc_assert (gimple_assign_single_p (stmt));
>    op = gimple_assign_rhs1 (stmt);
>    if (!vect_is_simple_use (op, stmt, loop_vinfo, bb_vinfo, &def_stmt,
> @@ -4378,20 +4379,21 @@ vectorizable_load (gimple stmt, gimple_s
>    /* Is vectorizable load? */
>    if (!is_gimple_assign (stmt))
>      return false;
>
>    scalar_dest = gimple_assign_lhs (stmt);
>    if (TREE_CODE (scalar_dest) != SSA_NAME)
>      return false;
>
>    code = gimple_assign_rhs_code (stmt);
>    if (code != ARRAY_REF
> +      && code != BIT_FIELD_REF
>        && code != INDIRECT_REF
>        && code != COMPONENT_REF
>        && code != IMAGPART_EXPR
>        && code != REALPART_EXPR
>        && code != MEM_REF
>        && TREE_CODE_CLASS (code) != tcc_declaration)
>      return false;
>
>    if (!STMT_VINFO_DATA_REF (stmt_info))
>      return false;
> Index: tree-vect-slp.c
> ===================================================================
> --- tree-vect-slp.c     (revision 197265)
> +++ tree-vect-slp.c     (working copy)
> @@ -660,20 +660,21 @@ vect_build_slp_tree (loop_vec_info loop_
>         }
>        else
>         {
>           if (first_stmt_code != rhs_code
>               && (first_stmt_code != IMAGPART_EXPR
>                   || rhs_code != REALPART_EXPR)
>               && (first_stmt_code != REALPART_EXPR
>                   || rhs_code != IMAGPART_EXPR)
>                && !(STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt))
>                     && (first_stmt_code == ARRAY_REF
> +                       || first_stmt_code == BIT_FIELD_REF
>                         || first_stmt_code == INDIRECT_REF
>                         || first_stmt_code == COMPONENT_REF
>                         || first_stmt_code == MEM_REF)))
>             {
>               if (dump_enabled_p ())
>                 {
>                   dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>                                    "Build SLP failed: different operation "
>                                    "in stmt ");
>                   dump_gimple_stmt (MSG_MISSED_OPTIMIZATION, TDF_SLIM, stmt,
> 0);
> Index: tree-flow-inline.h
> ===================================================================
> --- tree-flow-inline.h  (revision 197265)
> +++ tree-flow-inline.h  (working copy)
> @@ -1239,21 +1239,23 @@ get_addr_base_and_unit_offset_1 (tree ex
>  {
>    HOST_WIDE_INT byte_offset = 0;
>
>    /* Compute cumulative byte-offset for nested component-refs and
> array-refs,
>       and find the ultimate containing object.  */
>    while (1)
>      {
>        switch (TREE_CODE (exp))
>         {
>         case BIT_FIELD_REF:
> -         return NULL_TREE;
> +         byte_offset += (TREE_INT_CST_LOW (TREE_OPERAND (exp, 2))
> +                         / BITS_PER_UNIT);
> +         break;
>
>         case COMPONENT_REF:
>           {
>             tree field = TREE_OPERAND (exp, 1);
>             tree this_offset = component_ref_field_offset (exp);
>             HOST_WIDE_INT hthis_offset;
>
>             if (!this_offset
>                 || TREE_CODE (this_offset) != INTEGER_CST
>                 || (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
> Index: testsuite/gcc.dg/vect/bb-slp-31.c
> ===================================================================
> --- testsuite/gcc.dg/vect/bb-slp-31.c   (revision 0)
> +++ testsuite/gcc.dg/vect/bb-slp-31.c   (revision 0)
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target vect_double } */
> +
> +typedef double vec __attribute__ ((vector_size (2 * sizeof (double))));
> +vec a;
> +
> +void f(){
> +  a[0]=1+2*a[0]*a[0];
> +  a[1]=1+2*a[1]*a[1];
> +}
> +
> +/* { dg-final { scan-tree-dump "basic block vectorized using SLP" "slp" } }
> */
>
> Property changes on: testsuite/gcc.dg/vect/bb-slp-31.c
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
>

Patch

Index: tree-vect-data-refs.c

===================================================================
--- tree-vect-data-refs.c	(revision 197265)

+++ tree-vect-data-refs.c	(working copy)

@@ -3565,20 +3565,22 @@  vect_create_data_ref_ptr (gimple stmt, t

 
   if (dump_enabled_p ())
     {
       tree dr_base_type = TREE_TYPE (DR_BASE_OBJECT (dr));
       dump_printf_loc (MSG_NOTE, vect_location,
                        "create %s-pointer variable to type: ",
                        tree_code_name[(int) TREE_CODE (aggr_type)]);
       dump_generic_expr (MSG_NOTE, TDF_SLIM, aggr_type);
       if (TREE_CODE (dr_base_type) == ARRAY_TYPE)
         dump_printf (MSG_NOTE, "  vectorizing an array ref: ");
+      else if (TREE_CODE (dr_base_type) == VECTOR_TYPE)

+        dump_printf (MSG_NOTE, "  vectorizing a vector ref: ");

       else if (TREE_CODE (dr_base_type) == RECORD_TYPE)
         dump_printf (MSG_NOTE, "  vectorizing a record based array ref: ");
       else
         dump_printf (MSG_NOTE, "  vectorizing a pointer ref: ");
       dump_generic_expr (MSG_NOTE, TDF_SLIM, DR_BASE_OBJECT (dr));
     }
 
   /* (1) Create the new aggregate-pointer variable.
      Vector and array types inherit the alias set of their component
      type by default so we need to use a ref-all pointer if the data
Index: tree-vect-stmts.c

===================================================================
--- tree-vect-stmts.c	(revision 197265)

+++ tree-vect-stmts.c	(working copy)

@@ -3844,20 +3844,21 @@  vectorizable_store (gimple stmt, gimple_

   /* Is vectorizable store? */
 
   if (!is_gimple_assign (stmt))
     return false;
 
   scalar_dest = gimple_assign_lhs (stmt);
   if (TREE_CODE (scalar_dest) == VIEW_CONVERT_EXPR
       && is_pattern_stmt_p (stmt_info))
     scalar_dest = TREE_OPERAND (scalar_dest, 0);
   if (TREE_CODE (scalar_dest) != ARRAY_REF
+      && TREE_CODE (scalar_dest) != BIT_FIELD_REF

       && TREE_CODE (scalar_dest) != INDIRECT_REF
       && TREE_CODE (scalar_dest) != COMPONENT_REF
       && TREE_CODE (scalar_dest) != IMAGPART_EXPR
       && TREE_CODE (scalar_dest) != REALPART_EXPR
       && TREE_CODE (scalar_dest) != MEM_REF)
     return false;
 
   gcc_assert (gimple_assign_single_p (stmt));
   op = gimple_assign_rhs1 (stmt);
   if (!vect_is_simple_use (op, stmt, loop_vinfo, bb_vinfo, &def_stmt,
@@ -4378,20 +4379,21 @@  vectorizable_load (gimple stmt, gimple_s

   /* Is vectorizable load? */
   if (!is_gimple_assign (stmt))
     return false;
 
   scalar_dest = gimple_assign_lhs (stmt);
   if (TREE_CODE (scalar_dest) != SSA_NAME)
     return false;
 
   code = gimple_assign_rhs_code (stmt);
   if (code != ARRAY_REF
+      && code != BIT_FIELD_REF

       && code != INDIRECT_REF
       && code != COMPONENT_REF
       && code != IMAGPART_EXPR
       && code != REALPART_EXPR
       && code != MEM_REF
       && TREE_CODE_CLASS (code) != tcc_declaration)
     return false;
 
   if (!STMT_VINFO_DATA_REF (stmt_info))
     return false;
Index: tree-vect-slp.c

===================================================================
--- tree-vect-slp.c	(revision 197265)

+++ tree-vect-slp.c	(working copy)

@@ -660,20 +660,21 @@  vect_build_slp_tree (loop_vec_info loop_

 	}
       else
 	{
 	  if (first_stmt_code != rhs_code
 	      && (first_stmt_code != IMAGPART_EXPR
 		  || rhs_code != REALPART_EXPR)
 	      && (first_stmt_code != REALPART_EXPR
 		  || rhs_code != IMAGPART_EXPR)
               && !(STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt))
                    && (first_stmt_code == ARRAY_REF
+                       || first_stmt_code == BIT_FIELD_REF

                        || first_stmt_code == INDIRECT_REF
                        || first_stmt_code == COMPONENT_REF
                        || first_stmt_code == MEM_REF)))
 	    {
 	      if (dump_enabled_p ())
 		{
 		  dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, 
 				   "Build SLP failed: different operation "
 				   "in stmt ");
 		  dump_gimple_stmt (MSG_MISSED_OPTIMIZATION, TDF_SLIM, stmt, 0);
Index: tree-flow-inline.h

===================================================================
--- tree-flow-inline.h	(revision 197265)

+++ tree-flow-inline.h	(working copy)

@@ -1239,21 +1239,23 @@  get_addr_base_and_unit_offset_1 (tree ex

 {
   HOST_WIDE_INT byte_offset = 0;
 
   /* Compute cumulative byte-offset for nested component-refs and array-refs,
      and find the ultimate containing object.  */
   while (1)
     {
       switch (TREE_CODE (exp))
 	{
 	case BIT_FIELD_REF:
-	  return NULL_TREE;

+	  byte_offset += (TREE_INT_CST_LOW (TREE_OPERAND (exp, 2))

+			  / BITS_PER_UNIT);

+	  break;

 
 	case COMPONENT_REF:
 	  {
 	    tree field = TREE_OPERAND (exp, 1);
 	    tree this_offset = component_ref_field_offset (exp);
 	    HOST_WIDE_INT hthis_offset;
 
 	    if (!this_offset
 		|| TREE_CODE (this_offset) != INTEGER_CST
 		|| (TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (field))
Index: testsuite/gcc.dg/vect/bb-slp-31.c

===================================================================
--- testsuite/gcc.dg/vect/bb-slp-31.c	(revision 0)

+++ testsuite/gcc.dg/vect/bb-slp-31.c	(revision 0)

@@ -0,0 +1,12 @@ 

+/* { dg-do compile } */

+/* { dg-require-effective-target vect_double } */

+

+typedef double vec __attribute__ ((vector_size (2 * sizeof (double))));

+vec a;

+

+void f(){

+  a[0]=1+2*a[0]*a[0];

+  a[1]=1+2*a[1]*a[1];

+}

+

+/* { dg-final { scan-tree-dump "basic block vectorized using SLP" "slp" } } */