[36/46] Add a pattern_stmt_p field to stmt_vec_info

Message ID 87lga1lzfz.fsf@arm.com
State New
Headers show
Series
  • Remove vinfo_for_stmt etc.
Related show

Commit Message

Richard Sandiford July 24, 2018, 10:07 a.m.
This patch adds a pattern_stmt_p field to stmt_vec_info, so that it's
possible to tell whether the statement is a pattern statement without
referring to other statements.  The new field goes in what was
previously a hole in the structure, so the size is the same as before.


2018-07-24  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* tree-vectorizer.h (_stmt_vec_info::pattern_stmt_p): New field.
	(is_pattern_stmt_p): Delete.
	* tree-vect-patterns.c (vect_init_pattern_stmt): Set pattern_stmt_p
	on pattern statements.
	(vect_split_statement, vect_mark_pattern_stmts): Use the new
	pattern_stmt_p field instead of is_pattern_stmt_p.
	* tree-vect-data-refs.c (vect_preserves_scalar_order_p): Likewise.
	* tree-vect-loop.c (vectorizable_live_operation): Likewise.
	* tree-vect-slp.c (vect_build_slp_tree_2): Likewise.
	(vect_find_last_scalar_stmt_in_slp, vect_remove_slp_scalar_calls)
	(vect_schedule_slp): Likewise.
	* tree-vect-stmts.c (vect_mark_stmts_to_be_vectorized): Likewise.
	(vectorizable_call, vectorizable_simd_clone_call, vectorizable_shift)
	(vectorizable_store, vect_remove_stores): Likewise.

Comments

Richard Biener July 25, 2018, 10:15 a.m. | #1
On Tue, Jul 24, 2018 at 12:07 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>

> This patch adds a pattern_stmt_p field to stmt_vec_info, so that it's

> possible to tell whether the statement is a pattern statement without

> referring to other statements.  The new field goes in what was

> previously a hole in the structure, so the size is the same as before.


Not sure what the advantage is?  is_pattern_stmt_p () looks nicer
than ->is_pattern_p

>

> 2018-07-24  Richard Sandiford  <richard.sandiford@arm.com>

>

> gcc/

>         * tree-vectorizer.h (_stmt_vec_info::pattern_stmt_p): New field.

>         (is_pattern_stmt_p): Delete.

>         * tree-vect-patterns.c (vect_init_pattern_stmt): Set pattern_stmt_p

>         on pattern statements.

>         (vect_split_statement, vect_mark_pattern_stmts): Use the new

>         pattern_stmt_p field instead of is_pattern_stmt_p.

>         * tree-vect-data-refs.c (vect_preserves_scalar_order_p): Likewise.

>         * tree-vect-loop.c (vectorizable_live_operation): Likewise.

>         * tree-vect-slp.c (vect_build_slp_tree_2): Likewise.

>         (vect_find_last_scalar_stmt_in_slp, vect_remove_slp_scalar_calls)

>         (vect_schedule_slp): Likewise.

>         * tree-vect-stmts.c (vect_mark_stmts_to_be_vectorized): Likewise.

>         (vectorizable_call, vectorizable_simd_clone_call, vectorizable_shift)

>         (vectorizable_store, vect_remove_stores): Likewise.

>

> Index: gcc/tree-vectorizer.h

> ===================================================================

> --- gcc/tree-vectorizer.h       2018-07-24 10:23:56.440544995 +0100

> +++ gcc/tree-vectorizer.h       2018-07-24 10:24:02.364492386 +0100

> @@ -791,6 +791,12 @@ struct _stmt_vec_info {

>    /* Stmt is part of some pattern (computation idiom)  */

>    bool in_pattern_p;

>

> +  /* True if the statement was created during pattern recognition as

> +     part of the replacement for RELATED_STMT.  This implies that the

> +     statement isn't part of any basic block, although for convenience

> +     its gimple_bb is the same as for RELATED_STMT.  */

> +  bool pattern_stmt_p;

> +

>    /* Is this statement vectorizable or should it be skipped in (partial)

>       vectorization.  */

>    bool vectorizable;

> @@ -1151,16 +1157,6 @@ get_later_stmt (stmt_vec_info stmt1_info

>      return stmt2_info;

>  }

>

> -/* Return TRUE if a statement represented by STMT_INFO is a part of a

> -   pattern.  */

> -

> -static inline bool

> -is_pattern_stmt_p (stmt_vec_info stmt_info)

> -{

> -  stmt_vec_info related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);

> -  return related_stmt_info && STMT_VINFO_IN_PATTERN_P (related_stmt_info);

> -}

> -

>  /* Return true if BB is a loop header.  */

>

>  static inline bool

> Index: gcc/tree-vect-patterns.c

> ===================================================================

> --- gcc/tree-vect-patterns.c    2018-07-24 10:23:59.408518638 +0100

> +++ gcc/tree-vect-patterns.c    2018-07-24 10:24:02.360492422 +0100

> @@ -108,6 +108,7 @@ vect_init_pattern_stmt (gimple *pattern_

>      pattern_stmt_info = orig_stmt_info->vinfo->add_stmt (pattern_stmt);

>    gimple_set_bb (pattern_stmt, gimple_bb (orig_stmt_info->stmt));

>

> +  pattern_stmt_info->pattern_stmt_p = true;

>    STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info;

>    STMT_VINFO_DEF_TYPE (pattern_stmt_info)

>      = STMT_VINFO_DEF_TYPE (orig_stmt_info);

> @@ -630,7 +631,7 @@ vect_recog_temp_ssa_var (tree type, gimp

>  vect_split_statement (stmt_vec_info stmt2_info, tree new_rhs,

>                       gimple *stmt1, tree vectype)

>  {

> -  if (is_pattern_stmt_p (stmt2_info))

> +  if (stmt2_info->pattern_stmt_p)

>      {

>        /* STMT2_INFO is part of a pattern.  Get the statement to which

>          the pattern is attached.  */

> @@ -4726,7 +4727,7 @@ vect_mark_pattern_stmts (stmt_vec_info o

>    gimple *def_seq = STMT_VINFO_PATTERN_DEF_SEQ (orig_stmt_info);

>

>    gimple *orig_pattern_stmt = NULL;

> -  if (is_pattern_stmt_p (orig_stmt_info))

> +  if (orig_stmt_info->pattern_stmt_p)

>      {

>        /* We're replacing a statement in an existing pattern definition

>          sequence.  */

> Index: gcc/tree-vect-data-refs.c

> ===================================================================

> --- gcc/tree-vect-data-refs.c   2018-07-24 10:23:53.204573732 +0100

> +++ gcc/tree-vect-data-refs.c   2018-07-24 10:24:02.356492457 +0100

> @@ -212,9 +212,9 @@ vect_preserves_scalar_order_p (stmt_vec_

>       (but could happen later) while reads will happen no later than their

>       current position (but could happen earlier).  Reordering is therefore

>       only possible if the first access is a write.  */

> -  if (is_pattern_stmt_p (stmtinfo_a))

> +  if (stmtinfo_a->pattern_stmt_p)

>      stmtinfo_a = STMT_VINFO_RELATED_STMT (stmtinfo_a);

> -  if (is_pattern_stmt_p (stmtinfo_b))

> +  if (stmtinfo_b->pattern_stmt_p)

>      stmtinfo_b = STMT_VINFO_RELATED_STMT (stmtinfo_b);

>    stmt_vec_info earlier_stmt_info = get_earlier_stmt (stmtinfo_a, stmtinfo_b);

>    return !DR_IS_WRITE (STMT_VINFO_DATA_REF (earlier_stmt_info));

> Index: gcc/tree-vect-loop.c

> ===================================================================

> --- gcc/tree-vect-loop.c        2018-07-24 10:23:56.436545030 +0100

> +++ gcc/tree-vect-loop.c        2018-07-24 10:24:02.360492422 +0100

> @@ -7907,7 +7907,7 @@ vectorizable_live_operation (stmt_vec_in

>      }

>

>    /* If stmt has a related stmt, then use that for getting the lhs.  */

> -  gimple *stmt = (is_pattern_stmt_p (stmt_info)

> +  gimple *stmt = (stmt_info->pattern_stmt_p

>                   ? STMT_VINFO_RELATED_STMT (stmt_info)->stmt

>                   : stmt_info->stmt);

>

> Index: gcc/tree-vect-slp.c

> ===================================================================

> --- gcc/tree-vect-slp.c 2018-07-24 10:23:53.204573732 +0100

> +++ gcc/tree-vect-slp.c 2018-07-24 10:24:02.360492422 +0100

> @@ -376,7 +376,7 @@ vect_get_and_check_slp_defs (vec_info *v

>        /* Check if DEF_STMT_INFO is a part of a pattern in LOOP and get

>          the def stmt from the pattern.  Check that all the stmts of the

>          node are in the pattern.  */

> -      if (def_stmt_info && is_pattern_stmt_p (def_stmt_info))

> +      if (def_stmt_info && def_stmt_info->pattern_stmt_p)

>          {

>            pattern = true;

>            if (!first && !oprnd_info->first_pattern

> @@ -1315,7 +1315,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>               /* ???  Rejecting patterns this way doesn't work.  We'd have to

>                  do extra work to cancel the pattern so the uses see the

>                  scalar version.  */

> -             && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))

> +             && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)

>             {

>               slp_tree grandchild;

>

> @@ -1359,7 +1359,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>           /* ???  Rejecting patterns this way doesn't work.  We'd have to

>              do extra work to cancel the pattern so the uses see the

>              scalar version.  */

> -         && !is_pattern_stmt_p (stmt_info))

> +         && !stmt_info->pattern_stmt_p)

>         {

>           dump_printf_loc (MSG_NOTE, vect_location,

>                            "Building vector operands from scalars\n");

> @@ -1486,7 +1486,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>                   /* ???  Rejecting patterns this way doesn't work.  We'd have

>                      to do extra work to cancel the pattern so the uses see the

>                      scalar version.  */

> -                 && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))

> +                 && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)

>                 {

>                   unsigned int j;

>                   slp_tree grandchild;

> @@ -1848,7 +1848,7 @@ vect_find_last_scalar_stmt_in_slp (slp_t

>

>    for (int i = 0; SLP_TREE_SCALAR_STMTS (node).iterate (i, &stmt_vinfo); i++)

>      {

> -      if (is_pattern_stmt_p (stmt_vinfo))

> +      if (stmt_vinfo->pattern_stmt_p)

>         stmt_vinfo = STMT_VINFO_RELATED_STMT (stmt_vinfo);

>        last = last ? get_later_stmt (stmt_vinfo, last) : stmt_vinfo;

>      }

> @@ -4044,8 +4044,7 @@ vect_remove_slp_scalar_calls (slp_tree n

>        gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt);

>        if (!stmt || gimple_bb (stmt) == NULL)

>         continue;

> -      if (is_pattern_stmt_p (stmt_info)

> -         || !PURE_SLP_STMT (stmt_info))

> +      if (stmt_info->pattern_stmt_p || !PURE_SLP_STMT (stmt_info))

>         continue;

>        lhs = gimple_call_lhs (stmt);

>        new_stmt = gimple_build_assign (lhs, build_zero_cst (TREE_TYPE (lhs)));

> @@ -4106,7 +4105,7 @@ vect_schedule_slp (vec_info *vinfo)

>           if (!STMT_VINFO_DATA_REF (store_info))

>             break;

>

> -         if (is_pattern_stmt_p (store_info))

> +         if (store_info->pattern_stmt_p)

>             store_info = STMT_VINFO_RELATED_STMT (store_info);

>           /* Free the attached stmt_vec_info and remove the stmt.  */

>           gsi = gsi_for_stmt (store_info);

> Index: gcc/tree-vect-stmts.c

> ===================================================================

> --- gcc/tree-vect-stmts.c       2018-07-24 10:23:56.440544995 +0100

> +++ gcc/tree-vect-stmts.c       2018-07-24 10:24:02.364492386 +0100

> @@ -731,7 +731,7 @@ vect_mark_stmts_to_be_vectorized (loop_v

>              break;

>          }

>

> -      if (is_pattern_stmt_p (stmt_vinfo))

> +      if (stmt_vinfo->pattern_stmt_p)

>          {

>            /* Pattern statements are not inserted into the code, so

>               FOR_EACH_PHI_OR_STMT_USE optimizes their operands out, and we

> @@ -3623,7 +3623,7 @@ vectorizable_call (stmt_vec_info stmt_in

>    if (slp_node)

>      return true;

>

> -  if (is_pattern_stmt_p (stmt_info))

> +  if (stmt_info->pattern_stmt_p)

>      stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);

>    lhs = gimple_get_lhs (stmt_info->stmt);

>

> @@ -4362,7 +4362,7 @@ vectorizable_simd_clone_call (stmt_vec_i

>    if (scalar_dest)

>      {

>        type = TREE_TYPE (scalar_dest);

> -      if (is_pattern_stmt_p (stmt_info))

> +      if (stmt_info->pattern_stmt_p)

>         lhs = gimple_call_lhs (STMT_VINFO_RELATED_STMT (stmt_info)->stmt);

>        else

>         lhs = gimple_call_lhs (stmt);

> @@ -5552,7 +5552,7 @@ vectorizable_shift (stmt_vec_info stmt_i

>        /* If the shift amount is computed by a pattern stmt we cannot

>           use the scalar amount directly thus give up and use a vector

>          shift.  */

> -      if (op1_def_stmt_info && is_pattern_stmt_p (op1_def_stmt_info))

> +      if (op1_def_stmt_info && op1_def_stmt_info->pattern_stmt_p)

>         scalar_shift_arg = false;

>      }

>    else

> @@ -6286,7 +6286,7 @@ vectorizable_store (stmt_vec_info stmt_i

>      {

>        tree scalar_dest = gimple_assign_lhs (assign);

>        if (TREE_CODE (scalar_dest) == VIEW_CONVERT_EXPR

> -         && is_pattern_stmt_p (stmt_info))

> +         && stmt_info->pattern_stmt_p)

>         scalar_dest = TREE_OPERAND (scalar_dest, 0);

>        if (TREE_CODE (scalar_dest) != ARRAY_REF

>           && TREE_CODE (scalar_dest) != BIT_FIELD_REF

> @@ -9839,7 +9839,7 @@ vect_remove_stores (stmt_vec_info first_

>    while (next_stmt_info)

>      {

>        stmt_vec_info tmp = DR_GROUP_NEXT_ELEMENT (next_stmt_info);

> -      if (is_pattern_stmt_p (next_stmt_info))

> +      if (next_stmt_info->pattern_stmt_p)

>         next_stmt_info = STMT_VINFO_RELATED_STMT (next_stmt_info);

>        /* Free the attached stmt_vec_info and remove the stmt.  */

>        next_si = gsi_for_stmt (next_stmt_info->stmt);
Richard Sandiford July 25, 2018, 11:09 a.m. | #2
Richard Biener <richard.guenther@gmail.com> writes:
> On Tue, Jul 24, 2018 at 12:07 PM Richard Sandiford

> <richard.sandiford@arm.com> wrote:

>>

>> This patch adds a pattern_stmt_p field to stmt_vec_info, so that it's

>> possible to tell whether the statement is a pattern statement without

>> referring to other statements.  The new field goes in what was

>> previously a hole in the structure, so the size is the same as before.

>

> Not sure what the advantage is?  is_pattern_stmt_p () looks nicer

> than ->is_pattern_p


I can keep the function wrapper if you prefer that.  But having a
statement "know" whether it's a pattern stmt makes things like
freeing stmt_vec_infos simpler (see later patches in the series).
It should also be cheaper to test, but that's much more minor.

Thanks,
Richard

>

>>

>> 2018-07-24  Richard Sandiford  <richard.sandiford@arm.com>

>>

>> gcc/

>>         * tree-vectorizer.h (_stmt_vec_info::pattern_stmt_p): New field.

>>         (is_pattern_stmt_p): Delete.

>>         * tree-vect-patterns.c (vect_init_pattern_stmt): Set pattern_stmt_p

>>         on pattern statements.

>>         (vect_split_statement, vect_mark_pattern_stmts): Use the new

>>         pattern_stmt_p field instead of is_pattern_stmt_p.

>>         * tree-vect-data-refs.c (vect_preserves_scalar_order_p): Likewise.

>>         * tree-vect-loop.c (vectorizable_live_operation): Likewise.

>>         * tree-vect-slp.c (vect_build_slp_tree_2): Likewise.

>>         (vect_find_last_scalar_stmt_in_slp, vect_remove_slp_scalar_calls)

>>         (vect_schedule_slp): Likewise.

>>         * tree-vect-stmts.c (vect_mark_stmts_to_be_vectorized): Likewise.

>>         (vectorizable_call, vectorizable_simd_clone_call, vectorizable_shift)

>>         (vectorizable_store, vect_remove_stores): Likewise.

>>

>> Index: gcc/tree-vectorizer.h

>> ===================================================================

>> --- gcc/tree-vectorizer.h       2018-07-24 10:23:56.440544995 +0100

>> +++ gcc/tree-vectorizer.h       2018-07-24 10:24:02.364492386 +0100

>> @@ -791,6 +791,12 @@ struct _stmt_vec_info {

>>    /* Stmt is part of some pattern (computation idiom)  */

>>    bool in_pattern_p;

>>

>> +  /* True if the statement was created during pattern recognition as

>> +     part of the replacement for RELATED_STMT.  This implies that the

>> +     statement isn't part of any basic block, although for convenience

>> +     its gimple_bb is the same as for RELATED_STMT.  */

>> +  bool pattern_stmt_p;

>> +

>>    /* Is this statement vectorizable or should it be skipped in (partial)

>>       vectorization.  */

>>    bool vectorizable;

>> @@ -1151,16 +1157,6 @@ get_later_stmt (stmt_vec_info stmt1_info

>>      return stmt2_info;

>>  }

>>

>> -/* Return TRUE if a statement represented by STMT_INFO is a part of a

>> -   pattern.  */

>> -

>> -static inline bool

>> -is_pattern_stmt_p (stmt_vec_info stmt_info)

>> -{

>> -  stmt_vec_info related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);

>> -  return related_stmt_info && STMT_VINFO_IN_PATTERN_P (related_stmt_info);

>> -}

>> -

>>  /* Return true if BB is a loop header.  */

>>

>>  static inline bool

>> Index: gcc/tree-vect-patterns.c

>> ===================================================================

>> --- gcc/tree-vect-patterns.c    2018-07-24 10:23:59.408518638 +0100

>> +++ gcc/tree-vect-patterns.c    2018-07-24 10:24:02.360492422 +0100

>> @@ -108,6 +108,7 @@ vect_init_pattern_stmt (gimple *pattern_

>>      pattern_stmt_info = orig_stmt_info->vinfo->add_stmt (pattern_stmt);

>>    gimple_set_bb (pattern_stmt, gimple_bb (orig_stmt_info->stmt));

>>

>> +  pattern_stmt_info->pattern_stmt_p = true;

>>    STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info;

>>    STMT_VINFO_DEF_TYPE (pattern_stmt_info)

>>      = STMT_VINFO_DEF_TYPE (orig_stmt_info);

>> @@ -630,7 +631,7 @@ vect_recog_temp_ssa_var (tree type, gimp

>>  vect_split_statement (stmt_vec_info stmt2_info, tree new_rhs,

>>                       gimple *stmt1, tree vectype)

>>  {

>> -  if (is_pattern_stmt_p (stmt2_info))

>> +  if (stmt2_info->pattern_stmt_p)

>>      {

>>        /* STMT2_INFO is part of a pattern.  Get the statement to which

>>          the pattern is attached.  */

>> @@ -4726,7 +4727,7 @@ vect_mark_pattern_stmts (stmt_vec_info o

>>    gimple *def_seq = STMT_VINFO_PATTERN_DEF_SEQ (orig_stmt_info);

>>

>>    gimple *orig_pattern_stmt = NULL;

>> -  if (is_pattern_stmt_p (orig_stmt_info))

>> +  if (orig_stmt_info->pattern_stmt_p)

>>      {

>>        /* We're replacing a statement in an existing pattern definition

>>          sequence.  */

>> Index: gcc/tree-vect-data-refs.c

>> ===================================================================

>> --- gcc/tree-vect-data-refs.c   2018-07-24 10:23:53.204573732 +0100

>> +++ gcc/tree-vect-data-refs.c   2018-07-24 10:24:02.356492457 +0100

>> @@ -212,9 +212,9 @@ vect_preserves_scalar_order_p (stmt_vec_

>>       (but could happen later) while reads will happen no later than their

>>       current position (but could happen earlier).  Reordering is therefore

>>       only possible if the first access is a write.  */

>> -  if (is_pattern_stmt_p (stmtinfo_a))

>> +  if (stmtinfo_a->pattern_stmt_p)

>>      stmtinfo_a = STMT_VINFO_RELATED_STMT (stmtinfo_a);

>> -  if (is_pattern_stmt_p (stmtinfo_b))

>> +  if (stmtinfo_b->pattern_stmt_p)

>>      stmtinfo_b = STMT_VINFO_RELATED_STMT (stmtinfo_b);

>> stmt_vec_info earlier_stmt_info = get_earlier_stmt (stmtinfo_a,

> stmtinfo_b);

>>    return !DR_IS_WRITE (STMT_VINFO_DATA_REF (earlier_stmt_info));

>> Index: gcc/tree-vect-loop.c

>> ===================================================================

>> --- gcc/tree-vect-loop.c        2018-07-24 10:23:56.436545030 +0100

>> +++ gcc/tree-vect-loop.c        2018-07-24 10:24:02.360492422 +0100

>> @@ -7907,7 +7907,7 @@ vectorizable_live_operation (stmt_vec_in

>>      }

>>

>>    /* If stmt has a related stmt, then use that for getting the lhs.  */

>> -  gimple *stmt = (is_pattern_stmt_p (stmt_info)

>> +  gimple *stmt = (stmt_info->pattern_stmt_p

>>                   ? STMT_VINFO_RELATED_STMT (stmt_info)->stmt

>>                   : stmt_info->stmt);

>>

>> Index: gcc/tree-vect-slp.c

>> ===================================================================

>> --- gcc/tree-vect-slp.c 2018-07-24 10:23:53.204573732 +0100

>> +++ gcc/tree-vect-slp.c 2018-07-24 10:24:02.360492422 +0100

>> @@ -376,7 +376,7 @@ vect_get_and_check_slp_defs (vec_info *v

>>        /* Check if DEF_STMT_INFO is a part of a pattern in LOOP and get

>>          the def stmt from the pattern.  Check that all the stmts of the

>>          node are in the pattern.  */

>> -      if (def_stmt_info && is_pattern_stmt_p (def_stmt_info))

>> +      if (def_stmt_info && def_stmt_info->pattern_stmt_p)

>>          {

>>            pattern = true;

>>            if (!first && !oprnd_info->first_pattern

>> @@ -1315,7 +1315,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>>               /* ???  Rejecting patterns this way doesn't work.  We'd have to

>>                  do extra work to cancel the pattern so the uses see the

>>                  scalar version.  */

>> -             && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))

>> +             && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)

>>             {

>>               slp_tree grandchild;

>>

>> @@ -1359,7 +1359,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>>           /* ???  Rejecting patterns this way doesn't work.  We'd have to

>>              do extra work to cancel the pattern so the uses see the

>>              scalar version.  */

>> -         && !is_pattern_stmt_p (stmt_info))

>> +         && !stmt_info->pattern_stmt_p)

>>         {

>>           dump_printf_loc (MSG_NOTE, vect_location,

>>                            "Building vector operands from scalars\n");

>> @@ -1486,7 +1486,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

>>                   /* ???  Rejecting patterns this way doesn't work.  We'd have

>> to do extra work to cancel the pattern so the uses see the

>>                      scalar version.  */

>> -                 && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))

>> +                 && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)

>>                 {

>>                   unsigned int j;

>>                   slp_tree grandchild;

>> @@ -1848,7 +1848,7 @@ vect_find_last_scalar_stmt_in_slp (slp_t

>>

>>    for (int i = 0; SLP_TREE_SCALAR_STMTS (node).iterate (i, &stmt_vinfo); i++)

>>      {

>> -      if (is_pattern_stmt_p (stmt_vinfo))

>> +      if (stmt_vinfo->pattern_stmt_p)

>>         stmt_vinfo = STMT_VINFO_RELATED_STMT (stmt_vinfo);

>>        last = last ? get_later_stmt (stmt_vinfo, last) : stmt_vinfo;

>>      }

>> @@ -4044,8 +4044,7 @@ vect_remove_slp_scalar_calls (slp_tree n

>>        gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt);

>>        if (!stmt || gimple_bb (stmt) == NULL)

>>         continue;

>> -      if (is_pattern_stmt_p (stmt_info)

>> -         || !PURE_SLP_STMT (stmt_info))

>> +      if (stmt_info->pattern_stmt_p || !PURE_SLP_STMT (stmt_info))

>>         continue;

>>        lhs = gimple_call_lhs (stmt);

>>        new_stmt = gimple_build_assign (lhs, build_zero_cst (TREE_TYPE (lhs)));

>> @@ -4106,7 +4105,7 @@ vect_schedule_slp (vec_info *vinfo)

>>           if (!STMT_VINFO_DATA_REF (store_info))

>>             break;

>>

>> -         if (is_pattern_stmt_p (store_info))

>> +         if (store_info->pattern_stmt_p)

>>             store_info = STMT_VINFO_RELATED_STMT (store_info);

>>           /* Free the attached stmt_vec_info and remove the stmt.  */

>>           gsi = gsi_for_stmt (store_info);

>> Index: gcc/tree-vect-stmts.c

>> ===================================================================

>> --- gcc/tree-vect-stmts.c       2018-07-24 10:23:56.440544995 +0100

>> +++ gcc/tree-vect-stmts.c       2018-07-24 10:24:02.364492386 +0100

>> @@ -731,7 +731,7 @@ vect_mark_stmts_to_be_vectorized (loop_v

>>              break;

>>          }

>>

>> -      if (is_pattern_stmt_p (stmt_vinfo))

>> +      if (stmt_vinfo->pattern_stmt_p)

>>          {

>>            /* Pattern statements are not inserted into the code, so

>>               FOR_EACH_PHI_OR_STMT_USE optimizes their operands out, and we

>> @@ -3623,7 +3623,7 @@ vectorizable_call (stmt_vec_info stmt_in

>>    if (slp_node)

>>      return true;

>>

>> -  if (is_pattern_stmt_p (stmt_info))

>> +  if (stmt_info->pattern_stmt_p)

>>      stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);

>>    lhs = gimple_get_lhs (stmt_info->stmt);

>>

>> @@ -4362,7 +4362,7 @@ vectorizable_simd_clone_call (stmt_vec_i

>>    if (scalar_dest)

>>      {

>>        type = TREE_TYPE (scalar_dest);

>> -      if (is_pattern_stmt_p (stmt_info))

>> +      if (stmt_info->pattern_stmt_p)

>>         lhs = gimple_call_lhs (STMT_VINFO_RELATED_STMT (stmt_info)->stmt);

>>        else

>>         lhs = gimple_call_lhs (stmt);

>> @@ -5552,7 +5552,7 @@ vectorizable_shift (stmt_vec_info stmt_i

>>        /* If the shift amount is computed by a pattern stmt we cannot

>>           use the scalar amount directly thus give up and use a vector

>>          shift.  */

>> -      if (op1_def_stmt_info && is_pattern_stmt_p (op1_def_stmt_info))

>> +      if (op1_def_stmt_info && op1_def_stmt_info->pattern_stmt_p)

>>         scalar_shift_arg = false;

>>      }

>>    else

>> @@ -6286,7 +6286,7 @@ vectorizable_store (stmt_vec_info stmt_i

>>      {

>>        tree scalar_dest = gimple_assign_lhs (assign);

>>        if (TREE_CODE (scalar_dest) == VIEW_CONVERT_EXPR

>> -         && is_pattern_stmt_p (stmt_info))

>> +         && stmt_info->pattern_stmt_p)

>>         scalar_dest = TREE_OPERAND (scalar_dest, 0);

>>        if (TREE_CODE (scalar_dest) != ARRAY_REF

>>           && TREE_CODE (scalar_dest) != BIT_FIELD_REF

>> @@ -9839,7 +9839,7 @@ vect_remove_stores (stmt_vec_info first_

>>    while (next_stmt_info)

>>      {

>>        stmt_vec_info tmp = DR_GROUP_NEXT_ELEMENT (next_stmt_info);

>> -      if (is_pattern_stmt_p (next_stmt_info))

>> +      if (next_stmt_info->pattern_stmt_p)

>>         next_stmt_info = STMT_VINFO_RELATED_STMT (next_stmt_info);

>>        /* Free the attached stmt_vec_info and remove the stmt.  */

>>        next_si = gsi_for_stmt (next_stmt_info->stmt);
Richard Biener July 25, 2018, 11:48 a.m. | #3
On Wed, Jul 25, 2018 at 1:09 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>

> Richard Biener <richard.guenther@gmail.com> writes:

> > On Tue, Jul 24, 2018 at 12:07 PM Richard Sandiford

> > <richard.sandiford@arm.com> wrote:

> >>

> >> This patch adds a pattern_stmt_p field to stmt_vec_info, so that it's

> >> possible to tell whether the statement is a pattern statement without

> >> referring to other statements.  The new field goes in what was

> >> previously a hole in the structure, so the size is the same as before.

> >

> > Not sure what the advantage is?  is_pattern_stmt_p () looks nicer

> > than ->is_pattern_p

>

> I can keep the function wrapper if you prefer that.  But having a

> statement "know" whether it's a pattern stmt makes things like

> freeing stmt_vec_infos simpler (see later patches in the series).


Ah, ok.

> It should also be cheaper to test, but that's much more minor.


So please keep the wrapper.

I guess at some point we should decide what to do with all
the STMT_VINFO_ macros (and the others, {LOOP,BB}_ stuff
is already used inconsistently).

Richard.

> Thanks,

> Richard

>

> >

> >>

> >> 2018-07-24  Richard Sandiford  <richard.sandiford@arm.com>

> >>

> >> gcc/

> >>         * tree-vectorizer.h (_stmt_vec_info::pattern_stmt_p): New field.

> >>         (is_pattern_stmt_p): Delete.

> >>         * tree-vect-patterns.c (vect_init_pattern_stmt): Set pattern_stmt_p

> >>         on pattern statements.

> >>         (vect_split_statement, vect_mark_pattern_stmts): Use the new

> >>         pattern_stmt_p field instead of is_pattern_stmt_p.

> >>         * tree-vect-data-refs.c (vect_preserves_scalar_order_p): Likewise.

> >>         * tree-vect-loop.c (vectorizable_live_operation): Likewise.

> >>         * tree-vect-slp.c (vect_build_slp_tree_2): Likewise.

> >>         (vect_find_last_scalar_stmt_in_slp, vect_remove_slp_scalar_calls)

> >>         (vect_schedule_slp): Likewise.

> >>         * tree-vect-stmts.c (vect_mark_stmts_to_be_vectorized): Likewise.

> >>         (vectorizable_call, vectorizable_simd_clone_call, vectorizable_shift)

> >>         (vectorizable_store, vect_remove_stores): Likewise.

> >>

> >> Index: gcc/tree-vectorizer.h

> >> ===================================================================

> >> --- gcc/tree-vectorizer.h       2018-07-24 10:23:56.440544995 +0100

> >> +++ gcc/tree-vectorizer.h       2018-07-24 10:24:02.364492386 +0100

> >> @@ -791,6 +791,12 @@ struct _stmt_vec_info {

> >>    /* Stmt is part of some pattern (computation idiom)  */

> >>    bool in_pattern_p;

> >>

> >> +  /* True if the statement was created during pattern recognition as

> >> +     part of the replacement for RELATED_STMT.  This implies that the

> >> +     statement isn't part of any basic block, although for convenience

> >> +     its gimple_bb is the same as for RELATED_STMT.  */

> >> +  bool pattern_stmt_p;

> >> +

> >>    /* Is this statement vectorizable or should it be skipped in (partial)

> >>       vectorization.  */

> >>    bool vectorizable;

> >> @@ -1151,16 +1157,6 @@ get_later_stmt (stmt_vec_info stmt1_info

> >>      return stmt2_info;

> >>  }

> >>

> >> -/* Return TRUE if a statement represented by STMT_INFO is a part of a

> >> -   pattern.  */

> >> -

> >> -static inline bool

> >> -is_pattern_stmt_p (stmt_vec_info stmt_info)

> >> -{

> >> -  stmt_vec_info related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);

> >> -  return related_stmt_info && STMT_VINFO_IN_PATTERN_P (related_stmt_info);

> >> -}

> >> -

> >>  /* Return true if BB is a loop header.  */

> >>

> >>  static inline bool

> >> Index: gcc/tree-vect-patterns.c

> >> ===================================================================

> >> --- gcc/tree-vect-patterns.c    2018-07-24 10:23:59.408518638 +0100

> >> +++ gcc/tree-vect-patterns.c    2018-07-24 10:24:02.360492422 +0100

> >> @@ -108,6 +108,7 @@ vect_init_pattern_stmt (gimple *pattern_

> >>      pattern_stmt_info = orig_stmt_info->vinfo->add_stmt (pattern_stmt);

> >>    gimple_set_bb (pattern_stmt, gimple_bb (orig_stmt_info->stmt));

> >>

> >> +  pattern_stmt_info->pattern_stmt_p = true;

> >>    STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info;

> >>    STMT_VINFO_DEF_TYPE (pattern_stmt_info)

> >>      = STMT_VINFO_DEF_TYPE (orig_stmt_info);

> >> @@ -630,7 +631,7 @@ vect_recog_temp_ssa_var (tree type, gimp

> >>  vect_split_statement (stmt_vec_info stmt2_info, tree new_rhs,

> >>                       gimple *stmt1, tree vectype)

> >>  {

> >> -  if (is_pattern_stmt_p (stmt2_info))

> >> +  if (stmt2_info->pattern_stmt_p)

> >>      {

> >>        /* STMT2_INFO is part of a pattern.  Get the statement to which

> >>          the pattern is attached.  */

> >> @@ -4726,7 +4727,7 @@ vect_mark_pattern_stmts (stmt_vec_info o

> >>    gimple *def_seq = STMT_VINFO_PATTERN_DEF_SEQ (orig_stmt_info);

> >>

> >>    gimple *orig_pattern_stmt = NULL;

> >> -  if (is_pattern_stmt_p (orig_stmt_info))

> >> +  if (orig_stmt_info->pattern_stmt_p)

> >>      {

> >>        /* We're replacing a statement in an existing pattern definition

> >>          sequence.  */

> >> Index: gcc/tree-vect-data-refs.c

> >> ===================================================================

> >> --- gcc/tree-vect-data-refs.c   2018-07-24 10:23:53.204573732 +0100

> >> +++ gcc/tree-vect-data-refs.c   2018-07-24 10:24:02.356492457 +0100

> >> @@ -212,9 +212,9 @@ vect_preserves_scalar_order_p (stmt_vec_

> >>       (but could happen later) while reads will happen no later than their

> >>       current position (but could happen earlier).  Reordering is therefore

> >>       only possible if the first access is a write.  */

> >> -  if (is_pattern_stmt_p (stmtinfo_a))

> >> +  if (stmtinfo_a->pattern_stmt_p)

> >>      stmtinfo_a = STMT_VINFO_RELATED_STMT (stmtinfo_a);

> >> -  if (is_pattern_stmt_p (stmtinfo_b))

> >> +  if (stmtinfo_b->pattern_stmt_p)

> >>      stmtinfo_b = STMT_VINFO_RELATED_STMT (stmtinfo_b);

> >> stmt_vec_info earlier_stmt_info = get_earlier_stmt (stmtinfo_a,

> > stmtinfo_b);

> >>    return !DR_IS_WRITE (STMT_VINFO_DATA_REF (earlier_stmt_info));

> >> Index: gcc/tree-vect-loop.c

> >> ===================================================================

> >> --- gcc/tree-vect-loop.c        2018-07-24 10:23:56.436545030 +0100

> >> +++ gcc/tree-vect-loop.c        2018-07-24 10:24:02.360492422 +0100

> >> @@ -7907,7 +7907,7 @@ vectorizable_live_operation (stmt_vec_in

> >>      }

> >>

> >>    /* If stmt has a related stmt, then use that for getting the lhs.  */

> >> -  gimple *stmt = (is_pattern_stmt_p (stmt_info)

> >> +  gimple *stmt = (stmt_info->pattern_stmt_p

> >>                   ? STMT_VINFO_RELATED_STMT (stmt_info)->stmt

> >>                   : stmt_info->stmt);

> >>

> >> Index: gcc/tree-vect-slp.c

> >> ===================================================================

> >> --- gcc/tree-vect-slp.c 2018-07-24 10:23:53.204573732 +0100

> >> +++ gcc/tree-vect-slp.c 2018-07-24 10:24:02.360492422 +0100

> >> @@ -376,7 +376,7 @@ vect_get_and_check_slp_defs (vec_info *v

> >>        /* Check if DEF_STMT_INFO is a part of a pattern in LOOP and get

> >>          the def stmt from the pattern.  Check that all the stmts of the

> >>          node are in the pattern.  */

> >> -      if (def_stmt_info && is_pattern_stmt_p (def_stmt_info))

> >> +      if (def_stmt_info && def_stmt_info->pattern_stmt_p)

> >>          {

> >>            pattern = true;

> >>            if (!first && !oprnd_info->first_pattern

> >> @@ -1315,7 +1315,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

> >>               /* ???  Rejecting patterns this way doesn't work.  We'd have to

> >>                  do extra work to cancel the pattern so the uses see the

> >>                  scalar version.  */

> >> -             && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))

> >> +             && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)

> >>             {

> >>               slp_tree grandchild;

> >>

> >> @@ -1359,7 +1359,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

> >>           /* ???  Rejecting patterns this way doesn't work.  We'd have to

> >>              do extra work to cancel the pattern so the uses see the

> >>              scalar version.  */

> >> -         && !is_pattern_stmt_p (stmt_info))

> >> +         && !stmt_info->pattern_stmt_p)

> >>         {

> >>           dump_printf_loc (MSG_NOTE, vect_location,

> >>                            "Building vector operands from scalars\n");

> >> @@ -1486,7 +1486,7 @@ vect_build_slp_tree_2 (vec_info *vinfo,

> >>                   /* ???  Rejecting patterns this way doesn't work.  We'd have

> >> to do extra work to cancel the pattern so the uses see the

> >>                      scalar version.  */

> >> -                 && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))

> >> +                 && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)

> >>                 {

> >>                   unsigned int j;

> >>                   slp_tree grandchild;

> >> @@ -1848,7 +1848,7 @@ vect_find_last_scalar_stmt_in_slp (slp_t

> >>

> >>    for (int i = 0; SLP_TREE_SCALAR_STMTS (node).iterate (i, &stmt_vinfo); i++)

> >>      {

> >> -      if (is_pattern_stmt_p (stmt_vinfo))

> >> +      if (stmt_vinfo->pattern_stmt_p)

> >>         stmt_vinfo = STMT_VINFO_RELATED_STMT (stmt_vinfo);

> >>        last = last ? get_later_stmt (stmt_vinfo, last) : stmt_vinfo;

> >>      }

> >> @@ -4044,8 +4044,7 @@ vect_remove_slp_scalar_calls (slp_tree n

> >>        gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt);

> >>        if (!stmt || gimple_bb (stmt) == NULL)

> >>         continue;

> >> -      if (is_pattern_stmt_p (stmt_info)

> >> -         || !PURE_SLP_STMT (stmt_info))

> >> +      if (stmt_info->pattern_stmt_p || !PURE_SLP_STMT (stmt_info))

> >>         continue;

> >>        lhs = gimple_call_lhs (stmt);

> >>        new_stmt = gimple_build_assign (lhs, build_zero_cst (TREE_TYPE (lhs)));

> >> @@ -4106,7 +4105,7 @@ vect_schedule_slp (vec_info *vinfo)

> >>           if (!STMT_VINFO_DATA_REF (store_info))

> >>             break;

> >>

> >> -         if (is_pattern_stmt_p (store_info))

> >> +         if (store_info->pattern_stmt_p)

> >>             store_info = STMT_VINFO_RELATED_STMT (store_info);

> >>           /* Free the attached stmt_vec_info and remove the stmt.  */

> >>           gsi = gsi_for_stmt (store_info);

> >> Index: gcc/tree-vect-stmts.c

> >> ===================================================================

> >> --- gcc/tree-vect-stmts.c       2018-07-24 10:23:56.440544995 +0100

> >> +++ gcc/tree-vect-stmts.c       2018-07-24 10:24:02.364492386 +0100

> >> @@ -731,7 +731,7 @@ vect_mark_stmts_to_be_vectorized (loop_v

> >>              break;

> >>          }

> >>

> >> -      if (is_pattern_stmt_p (stmt_vinfo))

> >> +      if (stmt_vinfo->pattern_stmt_p)

> >>          {

> >>            /* Pattern statements are not inserted into the code, so

> >>               FOR_EACH_PHI_OR_STMT_USE optimizes their operands out, and we

> >> @@ -3623,7 +3623,7 @@ vectorizable_call (stmt_vec_info stmt_in

> >>    if (slp_node)

> >>      return true;

> >>

> >> -  if (is_pattern_stmt_p (stmt_info))

> >> +  if (stmt_info->pattern_stmt_p)

> >>      stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);

> >>    lhs = gimple_get_lhs (stmt_info->stmt);

> >>

> >> @@ -4362,7 +4362,7 @@ vectorizable_simd_clone_call (stmt_vec_i

> >>    if (scalar_dest)

> >>      {

> >>        type = TREE_TYPE (scalar_dest);

> >> -      if (is_pattern_stmt_p (stmt_info))

> >> +      if (stmt_info->pattern_stmt_p)

> >>         lhs = gimple_call_lhs (STMT_VINFO_RELATED_STMT (stmt_info)->stmt);

> >>        else

> >>         lhs = gimple_call_lhs (stmt);

> >> @@ -5552,7 +5552,7 @@ vectorizable_shift (stmt_vec_info stmt_i

> >>        /* If the shift amount is computed by a pattern stmt we cannot

> >>           use the scalar amount directly thus give up and use a vector

> >>          shift.  */

> >> -      if (op1_def_stmt_info && is_pattern_stmt_p (op1_def_stmt_info))

> >> +      if (op1_def_stmt_info && op1_def_stmt_info->pattern_stmt_p)

> >>         scalar_shift_arg = false;

> >>      }

> >>    else

> >> @@ -6286,7 +6286,7 @@ vectorizable_store (stmt_vec_info stmt_i

> >>      {

> >>        tree scalar_dest = gimple_assign_lhs (assign);

> >>        if (TREE_CODE (scalar_dest) == VIEW_CONVERT_EXPR

> >> -         && is_pattern_stmt_p (stmt_info))

> >> +         && stmt_info->pattern_stmt_p)

> >>         scalar_dest = TREE_OPERAND (scalar_dest, 0);

> >>        if (TREE_CODE (scalar_dest) != ARRAY_REF

> >>           && TREE_CODE (scalar_dest) != BIT_FIELD_REF

> >> @@ -9839,7 +9839,7 @@ vect_remove_stores (stmt_vec_info first_

> >>    while (next_stmt_info)

> >>      {

> >>        stmt_vec_info tmp = DR_GROUP_NEXT_ELEMENT (next_stmt_info);

> >> -      if (is_pattern_stmt_p (next_stmt_info))

> >> +      if (next_stmt_info->pattern_stmt_p)

> >>         next_stmt_info = STMT_VINFO_RELATED_STMT (next_stmt_info);

> >>        /* Free the attached stmt_vec_info and remove the stmt.  */

> >>        next_si = gsi_for_stmt (next_stmt_info->stmt);
Richard Sandiford July 26, 2018, 10:29 a.m. | #4
Richard Biener <richard.guenther@gmail.com> writes:
> On Wed, Jul 25, 2018 at 1:09 PM Richard Sandiford

> <richard.sandiford@arm.com> wrote:

>>

>> Richard Biener <richard.guenther@gmail.com> writes:

>> > On Tue, Jul 24, 2018 at 12:07 PM Richard Sandiford

>> > <richard.sandiford@arm.com> wrote:

>> >>

>> >> This patch adds a pattern_stmt_p field to stmt_vec_info, so that it's

>> >> possible to tell whether the statement is a pattern statement without

>> >> referring to other statements.  The new field goes in what was

>> >> previously a hole in the structure, so the size is the same as before.

>> >

>> > Not sure what the advantage is?  is_pattern_stmt_p () looks nicer

>> > than ->is_pattern_p

>>

>> I can keep the function wrapper if you prefer that.  But having a

>> statement "know" whether it's a pattern stmt makes things like

>> freeing stmt_vec_infos simpler (see later patches in the series).

>

> Ah, ok.

>

>> It should also be cheaper to test, but that's much more minor.

>

> So please keep the wrapper.


Like this?

> I guess at some point we should decide what to do with all

> the STMT_VINFO_ macros (and the others, {LOOP,BB}_ stuff

> is already used inconsistently).


Yeah...


2018-07-26  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* tree-vectorizer.h (_stmt_vec_info::pattern_stmt_p): New field.
	(is_pattern_stmt_p): Use it.
	* tree-vect-patterns.c (vect_init_pattern_stmt): Set pattern_stmt_p
	on pattern statements.

Index: gcc/tree-vectorizer.h
===================================================================
--- gcc/tree-vectorizer.h	2018-07-26 11:28:18.000000000 +0100
+++ gcc/tree-vectorizer.h	2018-07-26 11:28:19.072951054 +0100
@@ -791,6 +791,12 @@ struct _stmt_vec_info {
   /* Stmt is part of some pattern (computation idiom)  */
   bool in_pattern_p;
 
+  /* True if the statement was created during pattern recognition as
+     part of the replacement for RELATED_STMT.  This implies that the
+     statement isn't part of any basic block, although for convenience
+     its gimple_bb is the same as for RELATED_STMT.  */
+  bool pattern_stmt_p;
+
   /* Is this statement vectorizable or should it be skipped in (partial)
      vectorization.  */
   bool vectorizable;
@@ -1157,8 +1163,7 @@ get_later_stmt (stmt_vec_info stmt1_info
 static inline bool
 is_pattern_stmt_p (stmt_vec_info stmt_info)
 {
-  stmt_vec_info related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);
-  return related_stmt_info && STMT_VINFO_IN_PATTERN_P (related_stmt_info);
+  return stmt_info->pattern_stmt_p;
 }
 
 /* Return true if BB is a loop header.  */
Index: gcc/tree-vect-patterns.c
===================================================================
--- gcc/tree-vect-patterns.c	2018-07-26 11:28:18.000000000 +0100
+++ gcc/tree-vect-patterns.c	2018-07-26 11:28:19.068951168 +0100
@@ -108,6 +108,7 @@ vect_init_pattern_stmt (gimple *pattern_
     pattern_stmt_info = orig_stmt_info->vinfo->add_stmt (pattern_stmt);
   gimple_set_bb (pattern_stmt, gimple_bb (orig_stmt_info->stmt));
 
+  pattern_stmt_info->pattern_stmt_p = true;
   STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info;
   STMT_VINFO_DEF_TYPE (pattern_stmt_info)
     = STMT_VINFO_DEF_TYPE (orig_stmt_info);
Richard Biener July 26, 2018, 11:14 a.m. | #5
On Thu, Jul 26, 2018 at 12:29 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>

> Richard Biener <richard.guenther@gmail.com> writes:

> > On Wed, Jul 25, 2018 at 1:09 PM Richard Sandiford

> > <richard.sandiford@arm.com> wrote:

> >>

> >> Richard Biener <richard.guenther@gmail.com> writes:

> >> > On Tue, Jul 24, 2018 at 12:07 PM Richard Sandiford

> >> > <richard.sandiford@arm.com> wrote:

> >> >>

> >> >> This patch adds a pattern_stmt_p field to stmt_vec_info, so that it's

> >> >> possible to tell whether the statement is a pattern statement without

> >> >> referring to other statements.  The new field goes in what was

> >> >> previously a hole in the structure, so the size is the same as before.

> >> >

> >> > Not sure what the advantage is?  is_pattern_stmt_p () looks nicer

> >> > than ->is_pattern_p

> >>

> >> I can keep the function wrapper if you prefer that.  But having a

> >> statement "know" whether it's a pattern stmt makes things like

> >> freeing stmt_vec_infos simpler (see later patches in the series).

> >

> > Ah, ok.

> >

> >> It should also be cheaper to test, but that's much more minor.

> >

> > So please keep the wrapper.

>

> Like this?


Yes, OK.

Thanks,
Richard.

> > I guess at some point we should decide what to do with all

> > the STMT_VINFO_ macros (and the others, {LOOP,BB}_ stuff

> > is already used inconsistently).

>

> Yeah...

>

>

> 2018-07-26  Richard Sandiford  <richard.sandiford@arm.com>

>

> gcc/

>         * tree-vectorizer.h (_stmt_vec_info::pattern_stmt_p): New field.

>         (is_pattern_stmt_p): Use it.

>         * tree-vect-patterns.c (vect_init_pattern_stmt): Set pattern_stmt_p

>         on pattern statements.

>

> Index: gcc/tree-vectorizer.h

> ===================================================================

> --- gcc/tree-vectorizer.h       2018-07-26 11:28:18.000000000 +0100

> +++ gcc/tree-vectorizer.h       2018-07-26 11:28:19.072951054 +0100

> @@ -791,6 +791,12 @@ struct _stmt_vec_info {

>    /* Stmt is part of some pattern (computation idiom)  */

>    bool in_pattern_p;

>

> +  /* True if the statement was created during pattern recognition as

> +     part of the replacement for RELATED_STMT.  This implies that the

> +     statement isn't part of any basic block, although for convenience

> +     its gimple_bb is the same as for RELATED_STMT.  */

> +  bool pattern_stmt_p;

> +

>    /* Is this statement vectorizable or should it be skipped in (partial)

>       vectorization.  */

>    bool vectorizable;

> @@ -1157,8 +1163,7 @@ get_later_stmt (stmt_vec_info stmt1_info

>  static inline bool

>  is_pattern_stmt_p (stmt_vec_info stmt_info)

>  {

> -  stmt_vec_info related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);

> -  return related_stmt_info && STMT_VINFO_IN_PATTERN_P (related_stmt_info);

> +  return stmt_info->pattern_stmt_p;

>  }

>

>  /* Return true if BB is a loop header.  */

> Index: gcc/tree-vect-patterns.c

> ===================================================================

> --- gcc/tree-vect-patterns.c    2018-07-26 11:28:18.000000000 +0100

> +++ gcc/tree-vect-patterns.c    2018-07-26 11:28:19.068951168 +0100

> @@ -108,6 +108,7 @@ vect_init_pattern_stmt (gimple *pattern_

>      pattern_stmt_info = orig_stmt_info->vinfo->add_stmt (pattern_stmt);

>    gimple_set_bb (pattern_stmt, gimple_bb (orig_stmt_info->stmt));

>

> +  pattern_stmt_info->pattern_stmt_p = true;

>    STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info;

>    STMT_VINFO_DEF_TYPE (pattern_stmt_info)

>      = STMT_VINFO_DEF_TYPE (orig_stmt_info);

Patch

Index: gcc/tree-vectorizer.h
===================================================================
--- gcc/tree-vectorizer.h	2018-07-24 10:23:56.440544995 +0100
+++ gcc/tree-vectorizer.h	2018-07-24 10:24:02.364492386 +0100
@@ -791,6 +791,12 @@  struct _stmt_vec_info {
   /* Stmt is part of some pattern (computation idiom)  */
   bool in_pattern_p;
 
+  /* True if the statement was created during pattern recognition as
+     part of the replacement for RELATED_STMT.  This implies that the
+     statement isn't part of any basic block, although for convenience
+     its gimple_bb is the same as for RELATED_STMT.  */
+  bool pattern_stmt_p;
+
   /* Is this statement vectorizable or should it be skipped in (partial)
      vectorization.  */
   bool vectorizable;
@@ -1151,16 +1157,6 @@  get_later_stmt (stmt_vec_info stmt1_info
     return stmt2_info;
 }
 
-/* Return TRUE if a statement represented by STMT_INFO is a part of a
-   pattern.  */
-
-static inline bool
-is_pattern_stmt_p (stmt_vec_info stmt_info)
-{
-  stmt_vec_info related_stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);
-  return related_stmt_info && STMT_VINFO_IN_PATTERN_P (related_stmt_info);
-}
-
 /* Return true if BB is a loop header.  */
 
 static inline bool
Index: gcc/tree-vect-patterns.c
===================================================================
--- gcc/tree-vect-patterns.c	2018-07-24 10:23:59.408518638 +0100
+++ gcc/tree-vect-patterns.c	2018-07-24 10:24:02.360492422 +0100
@@ -108,6 +108,7 @@  vect_init_pattern_stmt (gimple *pattern_
     pattern_stmt_info = orig_stmt_info->vinfo->add_stmt (pattern_stmt);
   gimple_set_bb (pattern_stmt, gimple_bb (orig_stmt_info->stmt));
 
+  pattern_stmt_info->pattern_stmt_p = true;
   STMT_VINFO_RELATED_STMT (pattern_stmt_info) = orig_stmt_info;
   STMT_VINFO_DEF_TYPE (pattern_stmt_info)
     = STMT_VINFO_DEF_TYPE (orig_stmt_info);
@@ -630,7 +631,7 @@  vect_recog_temp_ssa_var (tree type, gimp
 vect_split_statement (stmt_vec_info stmt2_info, tree new_rhs,
 		      gimple *stmt1, tree vectype)
 {
-  if (is_pattern_stmt_p (stmt2_info))
+  if (stmt2_info->pattern_stmt_p)
     {
       /* STMT2_INFO is part of a pattern.  Get the statement to which
 	 the pattern is attached.  */
@@ -4726,7 +4727,7 @@  vect_mark_pattern_stmts (stmt_vec_info o
   gimple *def_seq = STMT_VINFO_PATTERN_DEF_SEQ (orig_stmt_info);
 
   gimple *orig_pattern_stmt = NULL;
-  if (is_pattern_stmt_p (orig_stmt_info))
+  if (orig_stmt_info->pattern_stmt_p)
     {
       /* We're replacing a statement in an existing pattern definition
 	 sequence.  */
Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c	2018-07-24 10:23:53.204573732 +0100
+++ gcc/tree-vect-data-refs.c	2018-07-24 10:24:02.356492457 +0100
@@ -212,9 +212,9 @@  vect_preserves_scalar_order_p (stmt_vec_
      (but could happen later) while reads will happen no later than their
      current position (but could happen earlier).  Reordering is therefore
      only possible if the first access is a write.  */
-  if (is_pattern_stmt_p (stmtinfo_a))
+  if (stmtinfo_a->pattern_stmt_p)
     stmtinfo_a = STMT_VINFO_RELATED_STMT (stmtinfo_a);
-  if (is_pattern_stmt_p (stmtinfo_b))
+  if (stmtinfo_b->pattern_stmt_p)
     stmtinfo_b = STMT_VINFO_RELATED_STMT (stmtinfo_b);
   stmt_vec_info earlier_stmt_info = get_earlier_stmt (stmtinfo_a, stmtinfo_b);
   return !DR_IS_WRITE (STMT_VINFO_DATA_REF (earlier_stmt_info));
Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	2018-07-24 10:23:56.436545030 +0100
+++ gcc/tree-vect-loop.c	2018-07-24 10:24:02.360492422 +0100
@@ -7907,7 +7907,7 @@  vectorizable_live_operation (stmt_vec_in
     }
 
   /* If stmt has a related stmt, then use that for getting the lhs.  */
-  gimple *stmt = (is_pattern_stmt_p (stmt_info)
+  gimple *stmt = (stmt_info->pattern_stmt_p
 		  ? STMT_VINFO_RELATED_STMT (stmt_info)->stmt
 		  : stmt_info->stmt);
 
Index: gcc/tree-vect-slp.c
===================================================================
--- gcc/tree-vect-slp.c	2018-07-24 10:23:53.204573732 +0100
+++ gcc/tree-vect-slp.c	2018-07-24 10:24:02.360492422 +0100
@@ -376,7 +376,7 @@  vect_get_and_check_slp_defs (vec_info *v
       /* Check if DEF_STMT_INFO is a part of a pattern in LOOP and get
 	 the def stmt from the pattern.  Check that all the stmts of the
 	 node are in the pattern.  */
-      if (def_stmt_info && is_pattern_stmt_p (def_stmt_info))
+      if (def_stmt_info && def_stmt_info->pattern_stmt_p)
         {
           pattern = true;
           if (!first && !oprnd_info->first_pattern
@@ -1315,7 +1315,7 @@  vect_build_slp_tree_2 (vec_info *vinfo,
 	      /* ???  Rejecting patterns this way doesn't work.  We'd have to
 		 do extra work to cancel the pattern so the uses see the
 		 scalar version.  */
-	      && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))
+	      && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)
 	    {
 	      slp_tree grandchild;
 
@@ -1359,7 +1359,7 @@  vect_build_slp_tree_2 (vec_info *vinfo,
 	  /* ???  Rejecting patterns this way doesn't work.  We'd have to
 	     do extra work to cancel the pattern so the uses see the
 	     scalar version.  */
-	  && !is_pattern_stmt_p (stmt_info))
+	  && !stmt_info->pattern_stmt_p)
 	{
 	  dump_printf_loc (MSG_NOTE, vect_location,
 			   "Building vector operands from scalars\n");
@@ -1486,7 +1486,7 @@  vect_build_slp_tree_2 (vec_info *vinfo,
 		  /* ???  Rejecting patterns this way doesn't work.  We'd have
 		     to do extra work to cancel the pattern so the uses see the
 		     scalar version.  */
-		  && !is_pattern_stmt_p (SLP_TREE_SCALAR_STMTS (child)[0]))
+		  && !SLP_TREE_SCALAR_STMTS (child)[0]->pattern_stmt_p)
 		{
 		  unsigned int j;
 		  slp_tree grandchild;
@@ -1848,7 +1848,7 @@  vect_find_last_scalar_stmt_in_slp (slp_t
 
   for (int i = 0; SLP_TREE_SCALAR_STMTS (node).iterate (i, &stmt_vinfo); i++)
     {
-      if (is_pattern_stmt_p (stmt_vinfo))
+      if (stmt_vinfo->pattern_stmt_p)
 	stmt_vinfo = STMT_VINFO_RELATED_STMT (stmt_vinfo);
       last = last ? get_later_stmt (stmt_vinfo, last) : stmt_vinfo;
     }
@@ -4044,8 +4044,7 @@  vect_remove_slp_scalar_calls (slp_tree n
       gcall *stmt = dyn_cast <gcall *> (stmt_info->stmt);
       if (!stmt || gimple_bb (stmt) == NULL)
 	continue;
-      if (is_pattern_stmt_p (stmt_info)
-	  || !PURE_SLP_STMT (stmt_info))
+      if (stmt_info->pattern_stmt_p || !PURE_SLP_STMT (stmt_info))
 	continue;
       lhs = gimple_call_lhs (stmt);
       new_stmt = gimple_build_assign (lhs, build_zero_cst (TREE_TYPE (lhs)));
@@ -4106,7 +4105,7 @@  vect_schedule_slp (vec_info *vinfo)
 	  if (!STMT_VINFO_DATA_REF (store_info))
 	    break;
 
-	  if (is_pattern_stmt_p (store_info))
+	  if (store_info->pattern_stmt_p)
 	    store_info = STMT_VINFO_RELATED_STMT (store_info);
 	  /* Free the attached stmt_vec_info and remove the stmt.  */
 	  gsi = gsi_for_stmt (store_info);
Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c	2018-07-24 10:23:56.440544995 +0100
+++ gcc/tree-vect-stmts.c	2018-07-24 10:24:02.364492386 +0100
@@ -731,7 +731,7 @@  vect_mark_stmts_to_be_vectorized (loop_v
             break;
         }
 
-      if (is_pattern_stmt_p (stmt_vinfo))
+      if (stmt_vinfo->pattern_stmt_p)
         {
           /* Pattern statements are not inserted into the code, so
              FOR_EACH_PHI_OR_STMT_USE optimizes their operands out, and we
@@ -3623,7 +3623,7 @@  vectorizable_call (stmt_vec_info stmt_in
   if (slp_node)
     return true;
 
-  if (is_pattern_stmt_p (stmt_info))
+  if (stmt_info->pattern_stmt_p)
     stmt_info = STMT_VINFO_RELATED_STMT (stmt_info);
   lhs = gimple_get_lhs (stmt_info->stmt);
 
@@ -4362,7 +4362,7 @@  vectorizable_simd_clone_call (stmt_vec_i
   if (scalar_dest)
     {
       type = TREE_TYPE (scalar_dest);
-      if (is_pattern_stmt_p (stmt_info))
+      if (stmt_info->pattern_stmt_p)
 	lhs = gimple_call_lhs (STMT_VINFO_RELATED_STMT (stmt_info)->stmt);
       else
 	lhs = gimple_call_lhs (stmt);
@@ -5552,7 +5552,7 @@  vectorizable_shift (stmt_vec_info stmt_i
       /* If the shift amount is computed by a pattern stmt we cannot
          use the scalar amount directly thus give up and use a vector
 	 shift.  */
-      if (op1_def_stmt_info && is_pattern_stmt_p (op1_def_stmt_info))
+      if (op1_def_stmt_info && op1_def_stmt_info->pattern_stmt_p)
 	scalar_shift_arg = false;
     }
   else
@@ -6286,7 +6286,7 @@  vectorizable_store (stmt_vec_info stmt_i
     {
       tree scalar_dest = gimple_assign_lhs (assign);
       if (TREE_CODE (scalar_dest) == VIEW_CONVERT_EXPR
-	  && is_pattern_stmt_p (stmt_info))
+	  && stmt_info->pattern_stmt_p)
 	scalar_dest = TREE_OPERAND (scalar_dest, 0);
       if (TREE_CODE (scalar_dest) != ARRAY_REF
 	  && TREE_CODE (scalar_dest) != BIT_FIELD_REF
@@ -9839,7 +9839,7 @@  vect_remove_stores (stmt_vec_info first_
   while (next_stmt_info)
     {
       stmt_vec_info tmp = DR_GROUP_NEXT_ELEMENT (next_stmt_info);
-      if (is_pattern_stmt_p (next_stmt_info))
+      if (next_stmt_info->pattern_stmt_p)
 	next_stmt_info = STMT_VINFO_RELATED_STMT (next_stmt_info);
       /* Free the attached stmt_vec_info and remove the stmt.  */
       next_si = gsi_for_stmt (next_stmt_info->stmt);