SLP for vectors

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.

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;

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;

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
>
>