diff mbox series

Make fix for PR 83965 handle SLP reduction chains

Message ID 87606rsivf.fsf@linaro.org
State New
Headers show
Series Make fix for PR 83965 handle SLP reduction chains | expand

Commit Message

Richard Sandiford Feb. 20, 2018, 4:53 p.m. UTC
This patch prevents pattern-matching of fold-left SLP reduction chains,
which the previous patch for 83965 didn't handle properly.  It only
stops the last statement in the group from being matched, but that's
enough to cause the group to be dissolved later.

A better fix would be to put all the information about the reduction
on the the first statement in the reduction chain, so that every
statement in the group can tell what the group is doing.  That doesn't
seem like stage 4 material though.

As it stands, things seem to be a bit of a mess.  In
vect_force_simple_reduction we attach the reduction type and
phi pointer to the last statement in a reduction chain:

      reduc_def_info = vinfo_for_stmt (def);
      STMT_VINFO_REDUC_TYPE (reduc_def_info) = v_reduc_type;
      STMT_VINFO_REDUC_DEF (reduc_def_info) = phi;

and mark it as vect_reduction_type in vect_analyze_scalar_cycles_1:

                  STMT_VINFO_DEF_TYPE (vinfo_for_stmt (reduc_stmt)) =
                                                           vect_reduction_def;

This code in vectorizable_reduction gave the impression that
everything really is keyed off the last statement:

  /* In case of reduction chain we switch to the first stmt in the chain, but
     we don't update STMT_INFO, since only the last stmt is marked as reduction
     and has reduction properties.  */
  if (GROUP_FIRST_ELEMENT (stmt_info)
      && GROUP_FIRST_ELEMENT (stmt_info) != stmt)
    {
      stmt = GROUP_FIRST_ELEMENT (stmt_info);
      first_p = false;
    }

But this code is dead these days.  GROUP_FIRST_ELEMENT is only nonnull
for SLP reduction chains, since we dissolve the group if SLP fails.
And SLP only analyses the first statement in the group, not the last:

  stmt = SLP_TREE_SCALAR_STMTS (node)[0];
  stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
  [...]
  bool res = vect_analyze_stmt (stmt, &dummy, node, node_instance);

So from that point of view the DEF_TYPE, REDUC_TYPE and REDUC_DEF
are being attached to the wrong statement, since we only analyse
the first one.  But it turns out that REDUC_TYPE and REDUC_DEF
don't matter for the first statement in the group, since that
takes the phi as an input, and when the phi is a direct input,
we use *its* REDUC_TYPE and REDUC_DEF instead of the statement's
own info.  The DEF_TYPE problem is handled by:

      /* Mark the first element of the reduction chain as reduction to properly
	 transform the node.  In the reduction analysis phase only the last
	 element of the chain is marked as reduction.  */
      if (!STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt)))
	STMT_VINFO_DEF_TYPE (vinfo_for_stmt (stmt)) = vect_reduction_def;

in vect_analyze_slp_instance (cancelled by:

		STMT_VINFO_DEF_TYPE (vinfo_for_stmt (first_element))
		  = vect_internal_def;

in vect_analyze_slp on failure), with the operation being repeated
in vect_schedule_slp_instance (redundantly AFAICT):

  /* Mark the first element of the reduction chain as reduction to properly
     transform the node.  In the analysis phase only the last element of the
     chain is marked as reduction.  */
  if (GROUP_FIRST_ELEMENT (stmt_info) && !STMT_VINFO_GROUPED_ACCESS (stmt_info)
      && GROUP_FIRST_ELEMENT (stmt_info) == stmt)
    {
      STMT_VINFO_DEF_TYPE (stmt_info) = vect_reduction_def;
      STMT_VINFO_TYPE (stmt_info) = reduc_vec_info_type;
    }

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

Richard


2018-02-20  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	PR tree-optimization/83965
	* tree-vect-patterns.c (vect_reassociating_reduction_p): Assume
	that grouped statements are part of a reduction chain.  Return
	true if the statement is not marked as a reduction itself but
	is part of a group.
	(vect_recog_dot_prod_pattern): Don't check whether the statement
	is part of a group here.
	(vect_recog_sad_pattern): Likewise.
	(vect_recog_widen_sum_pattern): Likewise.

gcc/testsuite/
	PR tree-optimization/83965
	* gcc.dg/vect/pr83965-2.c: New test.

Comments

Richard Biener Feb. 26, 2018, 12:22 p.m. UTC | #1
On Tue, Feb 20, 2018 at 5:53 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> This patch prevents pattern-matching of fold-left SLP reduction chains,

> which the previous patch for 83965 didn't handle properly.  It only

> stops the last statement in the group from being matched, but that's

> enough to cause the group to be dissolved later.

>

> A better fix would be to put all the information about the reduction

> on the the first statement in the reduction chain, so that every

> statement in the group can tell what the group is doing.  That doesn't

> seem like stage 4 material though.

>

> As it stands, things seem to be a bit of a mess.  In

> vect_force_simple_reduction we attach the reduction type and

> phi pointer to the last statement in a reduction chain:

>

>       reduc_def_info = vinfo_for_stmt (def);

>       STMT_VINFO_REDUC_TYPE (reduc_def_info) = v_reduc_type;

>       STMT_VINFO_REDUC_DEF (reduc_def_info) = phi;

>

> and mark it as vect_reduction_type in vect_analyze_scalar_cycles_1:

>

>                   STMT_VINFO_DEF_TYPE (vinfo_for_stmt (reduc_stmt)) =

>                                                            vect_reduction_def;

>

> This code in vectorizable_reduction gave the impression that

> everything really is keyed off the last statement:

>

>   /* In case of reduction chain we switch to the first stmt in the chain, but

>      we don't update STMT_INFO, since only the last stmt is marked as reduction

>      and has reduction properties.  */

>   if (GROUP_FIRST_ELEMENT (stmt_info)

>       && GROUP_FIRST_ELEMENT (stmt_info) != stmt)

>     {

>       stmt = GROUP_FIRST_ELEMENT (stmt_info);

>       first_p = false;

>     }

>

> But this code is dead these days.  GROUP_FIRST_ELEMENT is only nonnull

> for SLP reduction chains, since we dissolve the group if SLP fails.

> And SLP only analyses the first statement in the group, not the last:

>

>   stmt = SLP_TREE_SCALAR_STMTS (node)[0];

>   stmt_vec_info stmt_info = vinfo_for_stmt (stmt);

>   [...]

>   bool res = vect_analyze_stmt (stmt, &dummy, node, node_instance);

>

> So from that point of view the DEF_TYPE, REDUC_TYPE and REDUC_DEF

> are being attached to the wrong statement, since we only analyse

> the first one.  But it turns out that REDUC_TYPE and REDUC_DEF

> don't matter for the first statement in the group, since that

> takes the phi as an input, and when the phi is a direct input,

> we use *its* REDUC_TYPE and REDUC_DEF instead of the statement's

> own info.  The DEF_TYPE problem is handled by:

>

>       /* Mark the first element of the reduction chain as reduction to properly

>          transform the node.  In the reduction analysis phase only the last

>          element of the chain is marked as reduction.  */

>       if (!STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt)))

>         STMT_VINFO_DEF_TYPE (vinfo_for_stmt (stmt)) = vect_reduction_def;

>

> in vect_analyze_slp_instance (cancelled by:

>

>                 STMT_VINFO_DEF_TYPE (vinfo_for_stmt (first_element))

>                   = vect_internal_def;

>

> in vect_analyze_slp on failure), with the operation being repeated

> in vect_schedule_slp_instance (redundantly AFAICT):

>

>   /* Mark the first element of the reduction chain as reduction to properly

>      transform the node.  In the analysis phase only the last element of the

>      chain is marked as reduction.  */

>   if (GROUP_FIRST_ELEMENT (stmt_info) && !STMT_VINFO_GROUPED_ACCESS (stmt_info)

>       && GROUP_FIRST_ELEMENT (stmt_info) == stmt)

>     {

>       STMT_VINFO_DEF_TYPE (stmt_info) = vect_reduction_def;

>       STMT_VINFO_TYPE (stmt_info) = reduc_vec_info_type;

>     }

>

> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu.

> OK to install?


Ok for stage1.

Richard.

> Richard

>

>

> 2018-02-20  Richard Sandiford  <richard.sandiford@linaro.org>

>

> gcc/

>         PR tree-optimization/83965

>         * tree-vect-patterns.c (vect_reassociating_reduction_p): Assume

>         that grouped statements are part of a reduction chain.  Return

>         true if the statement is not marked as a reduction itself but

>         is part of a group.

>         (vect_recog_dot_prod_pattern): Don't check whether the statement

>         is part of a group here.

>         (vect_recog_sad_pattern): Likewise.

>         (vect_recog_widen_sum_pattern): Likewise.

>

> gcc/testsuite/

>         PR tree-optimization/83965

>         * gcc.dg/vect/pr83965-2.c: New test.

>

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

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

> --- gcc/tree-vect-patterns.c    2018-02-20 09:40:41.843451227 +0000

> +++ gcc/tree-vect-patterns.c    2018-02-20 16:28:55.636762056 +0000

> @@ -222,13 +222,16 @@ vect_recog_temp_ssa_var (tree type, gimp

>  }

>

>  /* Return true if STMT_VINFO describes a reduction for which reassociation

> -   is allowed.  */

> +   is allowed.  If STMT_INFO is part of a group, assume that it's part of

> +   a reduction chain and optimistically assume that all statements

> +   except the last allow reassociation.  */

>

>  static bool

>  vect_reassociating_reduction_p (stmt_vec_info stmt_vinfo)

>  {

>    return (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def

> -         && STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION);

> +         ? STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION

> +         : GROUP_FIRST_ELEMENT (stmt_vinfo) != NULL);

>  }

>

>  /* Function vect_recog_dot_prod_pattern

> @@ -350,8 +353,7 @@ vect_recog_dot_prod_pattern (vec<gimple

>      {

>        gimple *def_stmt;

>

> -      if (!vect_reassociating_reduction_p (stmt_vinfo)

> -         && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

> +      if (!vect_reassociating_reduction_p (stmt_vinfo))

>         return NULL;

>        oprnd0 = gimple_assign_rhs1 (last_stmt);

>        oprnd1 = gimple_assign_rhs2 (last_stmt);

> @@ -571,8 +573,7 @@ vect_recog_sad_pattern (vec<gimple *> *s

>      {

>        gimple *def_stmt;

>

> -      if (!vect_reassociating_reduction_p (stmt_vinfo)

> -         && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

> +      if (!vect_reassociating_reduction_p (stmt_vinfo))

>         return NULL;

>        plus_oprnd0 = gimple_assign_rhs1 (last_stmt);

>        plus_oprnd1 = gimple_assign_rhs2 (last_stmt);

> @@ -1256,8 +1257,7 @@ vect_recog_widen_sum_pattern (vec<gimple

>    if (gimple_assign_rhs_code (last_stmt) != PLUS_EXPR)

>      return NULL;

>

> -  if (!vect_reassociating_reduction_p (stmt_vinfo)

> -      && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

> +  if (!vect_reassociating_reduction_p (stmt_vinfo))

>      return NULL;

>

>    oprnd0 = gimple_assign_rhs1 (last_stmt);

> Index: gcc/testsuite/gcc.dg/vect/pr83965-2.c

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

> --- /dev/null   2018-02-19 19:34:42.906488063 +0000

> +++ gcc/testsuite/gcc.dg/vect/pr83965-2.c       2018-02-20 16:28:55.635762095 +0000

> @@ -0,0 +1,16 @@

> +/* { dg-do compile } */

> +/* { dg-additional-options "-Ofast -ftrapv" } */

> +

> +int c;

> +unsigned char d;

> +int e (unsigned char *f)

> +{

> +  int g;

> +  for (int a; a; a++)

> +    {

> +      for (int b = 0; b < 6; b++)

> +       g += __builtin_abs (f[b] - d);

> +      f += c;

> +    }

> +  return g;

> +}
Richard Sandiford Feb. 26, 2018, 2:37 p.m. UTC | #2
Richard Biener <richard.guenther@gmail.com> writes:
> On Tue, Feb 20, 2018 at 5:53 PM, Richard Sandiford

> <richard.sandiford@linaro.org> wrote:

>> This patch prevents pattern-matching of fold-left SLP reduction chains,

>> which the previous patch for 83965 didn't handle properly.  It only

>> stops the last statement in the group from being matched, but that's

>> enough to cause the group to be dissolved later.

>>

>> A better fix would be to put all the information about the reduction

>> on the the first statement in the reduction chain, so that every

>> statement in the group can tell what the group is doing.  That doesn't

>> seem like stage 4 material though.

>>

>> As it stands, things seem to be a bit of a mess.  In

>> vect_force_simple_reduction we attach the reduction type and

>> phi pointer to the last statement in a reduction chain:

>>

>>       reduc_def_info = vinfo_for_stmt (def);

>>       STMT_VINFO_REDUC_TYPE (reduc_def_info) = v_reduc_type;

>>       STMT_VINFO_REDUC_DEF (reduc_def_info) = phi;

>>

>> and mark it as vect_reduction_type in vect_analyze_scalar_cycles_1:

>>

>>                   STMT_VINFO_DEF_TYPE (vinfo_for_stmt (reduc_stmt)) =

>>                                                            vect_reduction_def;

>>

>> This code in vectorizable_reduction gave the impression that

>> everything really is keyed off the last statement:

>>

>>   /* In case of reduction chain we switch to the first stmt in the chain, but

>>      we don't update STMT_INFO, since only the last stmt is marked as reduction

>>      and has reduction properties.  */

>>   if (GROUP_FIRST_ELEMENT (stmt_info)

>>       && GROUP_FIRST_ELEMENT (stmt_info) != stmt)

>>     {

>>       stmt = GROUP_FIRST_ELEMENT (stmt_info);

>>       first_p = false;

>>     }

>>

>> But this code is dead these days.  GROUP_FIRST_ELEMENT is only nonnull

>> for SLP reduction chains, since we dissolve the group if SLP fails.

>> And SLP only analyses the first statement in the group, not the last:

>>

>>   stmt = SLP_TREE_SCALAR_STMTS (node)[0];

>>   stmt_vec_info stmt_info = vinfo_for_stmt (stmt);

>>   [...]

>>   bool res = vect_analyze_stmt (stmt, &dummy, node, node_instance);

>>

>> So from that point of view the DEF_TYPE, REDUC_TYPE and REDUC_DEF

>> are being attached to the wrong statement, since we only analyse

>> the first one.  But it turns out that REDUC_TYPE and REDUC_DEF

>> don't matter for the first statement in the group, since that

>> takes the phi as an input, and when the phi is a direct input,

>> we use *its* REDUC_TYPE and REDUC_DEF instead of the statement's

>> own info.  The DEF_TYPE problem is handled by:

>>

>>       /* Mark the first element of the reduction chain as reduction to properly

>>          transform the node.  In the reduction analysis phase only the last

>>          element of the chain is marked as reduction.  */

>>       if (!STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt)))

>>         STMT_VINFO_DEF_TYPE (vinfo_for_stmt (stmt)) = vect_reduction_def;

>>

>> in vect_analyze_slp_instance (cancelled by:

>>

>>                 STMT_VINFO_DEF_TYPE (vinfo_for_stmt (first_element))

>>                   = vect_internal_def;

>>

>> in vect_analyze_slp on failure), with the operation being repeated

>> in vect_schedule_slp_instance (redundantly AFAICT):

>>

>>   /* Mark the first element of the reduction chain as reduction to properly

>>      transform the node.  In the analysis phase only the last element of the

>>      chain is marked as reduction.  */

>>   if (GROUP_FIRST_ELEMENT (stmt_info) && !STMT_VINFO_GROUPED_ACCESS (stmt_info)

>>       && GROUP_FIRST_ELEMENT (stmt_info) == stmt)

>>     {

>>       STMT_VINFO_DEF_TYPE (stmt_info) = vect_reduction_def;

>>       STMT_VINFO_TYPE (stmt_info) = reduc_vec_info_type;

>>     }

>>

>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu.

>> OK to install?

>

> Ok for stage1.


It's a GCC 8 regression, so OK for stage4?

Richard

> Richard.

>

>> Richard

>>

>>

>> 2018-02-20  Richard Sandiford  <richard.sandiford@linaro.org>

>>

>> gcc/

>>         PR tree-optimization/83965

>>         * tree-vect-patterns.c (vect_reassociating_reduction_p): Assume

>>         that grouped statements are part of a reduction chain.  Return

>>         true if the statement is not marked as a reduction itself but

>>         is part of a group.

>>         (vect_recog_dot_prod_pattern): Don't check whether the statement

>>         is part of a group here.

>>         (vect_recog_sad_pattern): Likewise.

>>         (vect_recog_widen_sum_pattern): Likewise.

>>

>> gcc/testsuite/

>>         PR tree-optimization/83965

>>         * gcc.dg/vect/pr83965-2.c: New test.

>>

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

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

>> --- gcc/tree-vect-patterns.c    2018-02-20 09:40:41.843451227 +0000

>> +++ gcc/tree-vect-patterns.c    2018-02-20 16:28:55.636762056 +0000

>> @@ -222,13 +222,16 @@ vect_recog_temp_ssa_var (tree type, gimp

>>  }

>>

>>  /* Return true if STMT_VINFO describes a reduction for which reassociation

>> -   is allowed.  */

>> +   is allowed.  If STMT_INFO is part of a group, assume that it's part of

>> +   a reduction chain and optimistically assume that all statements

>> +   except the last allow reassociation.  */

>>

>>  static bool

>>  vect_reassociating_reduction_p (stmt_vec_info stmt_vinfo)

>>  {

>>    return (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def

>> -         && STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION);

>> +         ? STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION

>> +         : GROUP_FIRST_ELEMENT (stmt_vinfo) != NULL);

>>  }

>>

>>  /* Function vect_recog_dot_prod_pattern

>> @@ -350,8 +353,7 @@ vect_recog_dot_prod_pattern (vec<gimple

>>      {

>>        gimple *def_stmt;

>>

>> -      if (!vect_reassociating_reduction_p (stmt_vinfo)

>> -         && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

>> +      if (!vect_reassociating_reduction_p (stmt_vinfo))

>>         return NULL;

>>        oprnd0 = gimple_assign_rhs1 (last_stmt);

>>        oprnd1 = gimple_assign_rhs2 (last_stmt);

>> @@ -571,8 +573,7 @@ vect_recog_sad_pattern (vec<gimple *> *s

>>      {

>>        gimple *def_stmt;

>>

>> -      if (!vect_reassociating_reduction_p (stmt_vinfo)

>> -         && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

>> +      if (!vect_reassociating_reduction_p (stmt_vinfo))

>>         return NULL;

>>        plus_oprnd0 = gimple_assign_rhs1 (last_stmt);

>>        plus_oprnd1 = gimple_assign_rhs2 (last_stmt);

>> @@ -1256,8 +1257,7 @@ vect_recog_widen_sum_pattern (vec<gimple

>>    if (gimple_assign_rhs_code (last_stmt) != PLUS_EXPR)

>>      return NULL;

>>

>> -  if (!vect_reassociating_reduction_p (stmt_vinfo)

>> -      && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

>> +  if (!vect_reassociating_reduction_p (stmt_vinfo))

>>      return NULL;

>>

>>    oprnd0 = gimple_assign_rhs1 (last_stmt);

>> Index: gcc/testsuite/gcc.dg/vect/pr83965-2.c

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

>> --- /dev/null   2018-02-19 19:34:42.906488063 +0000

>> +++ gcc/testsuite/gcc.dg/vect/pr83965-2.c       2018-02-20 16:28:55.635762095 +0000

>> @@ -0,0 +1,16 @@

>> +/* { dg-do compile } */

>> +/* { dg-additional-options "-Ofast -ftrapv" } */

>> +

>> +int c;

>> +unsigned char d;

>> +int e (unsigned char *f)

>> +{

>> +  int g;

>> +  for (int a; a; a++)

>> +    {

>> +      for (int b = 0; b < 6; b++)

>> +       g += __builtin_abs (f[b] - d);

>> +      f += c;

>> +    }

>> +  return g;

>> +}
Richard Biener Feb. 26, 2018, 3:01 p.m. UTC | #3
On Mon, Feb 26, 2018 at 3:37 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:

>> On Tue, Feb 20, 2018 at 5:53 PM, Richard Sandiford

>> <richard.sandiford@linaro.org> wrote:

>>> This patch prevents pattern-matching of fold-left SLP reduction chains,

>>> which the previous patch for 83965 didn't handle properly.  It only

>>> stops the last statement in the group from being matched, but that's

>>> enough to cause the group to be dissolved later.

>>>

>>> A better fix would be to put all the information about the reduction

>>> on the the first statement in the reduction chain, so that every

>>> statement in the group can tell what the group is doing.  That doesn't

>>> seem like stage 4 material though.

>>>

>>> As it stands, things seem to be a bit of a mess.  In

>>> vect_force_simple_reduction we attach the reduction type and

>>> phi pointer to the last statement in a reduction chain:

>>>

>>>       reduc_def_info = vinfo_for_stmt (def);

>>>       STMT_VINFO_REDUC_TYPE (reduc_def_info) = v_reduc_type;

>>>       STMT_VINFO_REDUC_DEF (reduc_def_info) = phi;

>>>

>>> and mark it as vect_reduction_type in vect_analyze_scalar_cycles_1:

>>>

>>>                   STMT_VINFO_DEF_TYPE (vinfo_for_stmt (reduc_stmt)) =

>>>                                                            vect_reduction_def;

>>>

>>> This code in vectorizable_reduction gave the impression that

>>> everything really is keyed off the last statement:

>>>

>>>   /* In case of reduction chain we switch to the first stmt in the chain, but

>>>      we don't update STMT_INFO, since only the last stmt is marked as reduction

>>>      and has reduction properties.  */

>>>   if (GROUP_FIRST_ELEMENT (stmt_info)

>>>       && GROUP_FIRST_ELEMENT (stmt_info) != stmt)

>>>     {

>>>       stmt = GROUP_FIRST_ELEMENT (stmt_info);

>>>       first_p = false;

>>>     }

>>>

>>> But this code is dead these days.  GROUP_FIRST_ELEMENT is only nonnull

>>> for SLP reduction chains, since we dissolve the group if SLP fails.

>>> And SLP only analyses the first statement in the group, not the last:

>>>

>>>   stmt = SLP_TREE_SCALAR_STMTS (node)[0];

>>>   stmt_vec_info stmt_info = vinfo_for_stmt (stmt);

>>>   [...]

>>>   bool res = vect_analyze_stmt (stmt, &dummy, node, node_instance);

>>>

>>> So from that point of view the DEF_TYPE, REDUC_TYPE and REDUC_DEF

>>> are being attached to the wrong statement, since we only analyse

>>> the first one.  But it turns out that REDUC_TYPE and REDUC_DEF

>>> don't matter for the first statement in the group, since that

>>> takes the phi as an input, and when the phi is a direct input,

>>> we use *its* REDUC_TYPE and REDUC_DEF instead of the statement's

>>> own info.  The DEF_TYPE problem is handled by:

>>>

>>>       /* Mark the first element of the reduction chain as reduction to properly

>>>          transform the node.  In the reduction analysis phase only the last

>>>          element of the chain is marked as reduction.  */

>>>       if (!STMT_VINFO_GROUPED_ACCESS (vinfo_for_stmt (stmt)))

>>>         STMT_VINFO_DEF_TYPE (vinfo_for_stmt (stmt)) = vect_reduction_def;

>>>

>>> in vect_analyze_slp_instance (cancelled by:

>>>

>>>                 STMT_VINFO_DEF_TYPE (vinfo_for_stmt (first_element))

>>>                   = vect_internal_def;

>>>

>>> in vect_analyze_slp on failure), with the operation being repeated

>>> in vect_schedule_slp_instance (redundantly AFAICT):

>>>

>>>   /* Mark the first element of the reduction chain as reduction to properly

>>>      transform the node.  In the analysis phase only the last element of the

>>>      chain is marked as reduction.  */

>>>   if (GROUP_FIRST_ELEMENT (stmt_info) && !STMT_VINFO_GROUPED_ACCESS (stmt_info)

>>>       && GROUP_FIRST_ELEMENT (stmt_info) == stmt)

>>>     {

>>>       STMT_VINFO_DEF_TYPE (stmt_info) = vect_reduction_def;

>>>       STMT_VINFO_TYPE (stmt_info) = reduc_vec_info_type;

>>>     }

>>>

>>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64-linux-gnu.

>>> OK to install?

>>

>> Ok for stage1.

>

> It's a GCC 8 regression, so OK for stage4?


Oh, ok then.

Richard.

> Richard

>

>> Richard.

>>

>>> Richard

>>>

>>>

>>> 2018-02-20  Richard Sandiford  <richard.sandiford@linaro.org>

>>>

>>> gcc/

>>>         PR tree-optimization/83965

>>>         * tree-vect-patterns.c (vect_reassociating_reduction_p): Assume

>>>         that grouped statements are part of a reduction chain.  Return

>>>         true if the statement is not marked as a reduction itself but

>>>         is part of a group.

>>>         (vect_recog_dot_prod_pattern): Don't check whether the statement

>>>         is part of a group here.

>>>         (vect_recog_sad_pattern): Likewise.

>>>         (vect_recog_widen_sum_pattern): Likewise.

>>>

>>> gcc/testsuite/

>>>         PR tree-optimization/83965

>>>         * gcc.dg/vect/pr83965-2.c: New test.

>>>

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

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

>>> --- gcc/tree-vect-patterns.c    2018-02-20 09:40:41.843451227 +0000

>>> +++ gcc/tree-vect-patterns.c    2018-02-20 16:28:55.636762056 +0000

>>> @@ -222,13 +222,16 @@ vect_recog_temp_ssa_var (tree type, gimp

>>>  }

>>>

>>>  /* Return true if STMT_VINFO describes a reduction for which reassociation

>>> -   is allowed.  */

>>> +   is allowed.  If STMT_INFO is part of a group, assume that it's part of

>>> +   a reduction chain and optimistically assume that all statements

>>> +   except the last allow reassociation.  */

>>>

>>>  static bool

>>>  vect_reassociating_reduction_p (stmt_vec_info stmt_vinfo)

>>>  {

>>>    return (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def

>>> -         && STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION);

>>> +         ? STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION

>>> +         : GROUP_FIRST_ELEMENT (stmt_vinfo) != NULL);

>>>  }

>>>

>>>  /* Function vect_recog_dot_prod_pattern

>>> @@ -350,8 +353,7 @@ vect_recog_dot_prod_pattern (vec<gimple

>>>      {

>>>        gimple *def_stmt;

>>>

>>> -      if (!vect_reassociating_reduction_p (stmt_vinfo)

>>> -         && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

>>> +      if (!vect_reassociating_reduction_p (stmt_vinfo))

>>>         return NULL;

>>>        oprnd0 = gimple_assign_rhs1 (last_stmt);

>>>        oprnd1 = gimple_assign_rhs2 (last_stmt);

>>> @@ -571,8 +573,7 @@ vect_recog_sad_pattern (vec<gimple *> *s

>>>      {

>>>        gimple *def_stmt;

>>>

>>> -      if (!vect_reassociating_reduction_p (stmt_vinfo)

>>> -         && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

>>> +      if (!vect_reassociating_reduction_p (stmt_vinfo))

>>>         return NULL;

>>>        plus_oprnd0 = gimple_assign_rhs1 (last_stmt);

>>>        plus_oprnd1 = gimple_assign_rhs2 (last_stmt);

>>> @@ -1256,8 +1257,7 @@ vect_recog_widen_sum_pattern (vec<gimple

>>>    if (gimple_assign_rhs_code (last_stmt) != PLUS_EXPR)

>>>      return NULL;

>>>

>>> -  if (!vect_reassociating_reduction_p (stmt_vinfo)

>>> -      && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))

>>> +  if (!vect_reassociating_reduction_p (stmt_vinfo))

>>>      return NULL;

>>>

>>>    oprnd0 = gimple_assign_rhs1 (last_stmt);

>>> Index: gcc/testsuite/gcc.dg/vect/pr83965-2.c

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

>>> --- /dev/null   2018-02-19 19:34:42.906488063 +0000

>>> +++ gcc/testsuite/gcc.dg/vect/pr83965-2.c       2018-02-20 16:28:55.635762095 +0000

>>> @@ -0,0 +1,16 @@

>>> +/* { dg-do compile } */

>>> +/* { dg-additional-options "-Ofast -ftrapv" } */

>>> +

>>> +int c;

>>> +unsigned char d;

>>> +int e (unsigned char *f)

>>> +{

>>> +  int g;

>>> +  for (int a; a; a++)

>>> +    {

>>> +      for (int b = 0; b < 6; b++)

>>> +       g += __builtin_abs (f[b] - d);

>>> +      f += c;

>>> +    }

>>> +  return g;

>>> +}
diff mbox series

Patch

Index: gcc/tree-vect-patterns.c
===================================================================
--- gcc/tree-vect-patterns.c	2018-02-20 09:40:41.843451227 +0000
+++ gcc/tree-vect-patterns.c	2018-02-20 16:28:55.636762056 +0000
@@ -222,13 +222,16 @@  vect_recog_temp_ssa_var (tree type, gimp
 }
 
 /* Return true if STMT_VINFO describes a reduction for which reassociation
-   is allowed.  */
+   is allowed.  If STMT_INFO is part of a group, assume that it's part of
+   a reduction chain and optimistically assume that all statements
+   except the last allow reassociation.  */
 
 static bool
 vect_reassociating_reduction_p (stmt_vec_info stmt_vinfo)
 {
   return (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def
-	  && STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION);
+	  ? STMT_VINFO_REDUC_TYPE (stmt_vinfo) != FOLD_LEFT_REDUCTION
+	  : GROUP_FIRST_ELEMENT (stmt_vinfo) != NULL);
 }
 
 /* Function vect_recog_dot_prod_pattern
@@ -350,8 +353,7 @@  vect_recog_dot_prod_pattern (vec<gimple
     {
       gimple *def_stmt;
 
-      if (!vect_reassociating_reduction_p (stmt_vinfo)
-	  && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))
+      if (!vect_reassociating_reduction_p (stmt_vinfo))
 	return NULL;
       oprnd0 = gimple_assign_rhs1 (last_stmt);
       oprnd1 = gimple_assign_rhs2 (last_stmt);
@@ -571,8 +573,7 @@  vect_recog_sad_pattern (vec<gimple *> *s
     {
       gimple *def_stmt;
 
-      if (!vect_reassociating_reduction_p (stmt_vinfo)
-	  && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))
+      if (!vect_reassociating_reduction_p (stmt_vinfo))
 	return NULL;
       plus_oprnd0 = gimple_assign_rhs1 (last_stmt);
       plus_oprnd1 = gimple_assign_rhs2 (last_stmt);
@@ -1256,8 +1257,7 @@  vect_recog_widen_sum_pattern (vec<gimple
   if (gimple_assign_rhs_code (last_stmt) != PLUS_EXPR)
     return NULL;
 
-  if (!vect_reassociating_reduction_p (stmt_vinfo)
-      && ! STMT_VINFO_GROUP_FIRST_ELEMENT (stmt_vinfo))
+  if (!vect_reassociating_reduction_p (stmt_vinfo))
     return NULL;
 
   oprnd0 = gimple_assign_rhs1 (last_stmt);
Index: gcc/testsuite/gcc.dg/vect/pr83965-2.c
===================================================================
--- /dev/null	2018-02-19 19:34:42.906488063 +0000
+++ gcc/testsuite/gcc.dg/vect/pr83965-2.c	2018-02-20 16:28:55.635762095 +0000
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-Ofast -ftrapv" } */
+
+int c;
+unsigned char d;
+int e (unsigned char *f)
+{
+  int g;
+  for (int a; a; a++)
+    {
+      for (int b = 0; b < 6; b++)
+	g += __builtin_abs (f[b] - d);
+      f += c;
+    }
+  return g;
+}