public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR81815: Invalid conditional reduction
@ 2017-08-14 14:22 Richard Sandiford
  2017-08-15 12:47 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Sandiford @ 2017-08-14 14:22 UTC (permalink / raw)
  To: gcc-patches

We weren't checking whether the phi in a conditional reduction was
used by the condition itself (which isn't a case we handle).

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

Thanks,
Richard


2017-08-11  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	PR tree-optimization/81835
	* tree-vect-loop.c (vect_is_simple_reduction): Simply checks for
	the phi SSA_NAME.  Check that the condition in a COND_EXPR does
	not depend on the phi.

gcc/testsuite/
	PR tree-optimization/81835
	* gcc.dg/vect/pr81815.c: New test.

Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	2017-08-04 11:41:05.710206063 +0100
+++ gcc/tree-vect-loop.c	2017-08-14 13:46:28.336459006 +0100
@@ -2690,15 +2690,15 @@ vect_is_simple_reduction (loop_vec_info
   *double_reduc = false;
   *v_reduc_type = TREE_CODE_REDUCTION;
 
-  name = PHI_RESULT (phi);
+  tree phi_name = PHI_RESULT (phi);
   /* ???  If there are no uses of the PHI result the inner loop reduction
      won't be detected as possibly double-reduction by vectorizable_reduction
      because that tries to walk the PHI arg from the preheader edge which
      can be constant.  See PR60382.  */
-  if (has_zero_uses (name))
+  if (has_zero_uses (phi_name))
     return NULL;
   nloop_uses = 0;
-  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, name)
+  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, phi_name)
     {
       gimple *use_stmt = USE_STMT (use_p);
       if (is_gimple_debug (use_stmt))
@@ -2847,10 +2847,7 @@ vect_is_simple_reduction (loop_vec_info
      simply rewriting this into "res += -x[i]".  Avoid changing
      gimple instruction for the first simple tests and only do this
      if we're allowed to change code at all.  */
-  if (code == MINUS_EXPR
-      && ! ((op1 = gimple_assign_rhs2 (def_stmt))
-	    && TREE_CODE (op1) == SSA_NAME
-	    && SSA_NAME_DEF_STMT (op1) == phi))
+  if (code == MINUS_EXPR && gimple_assign_rhs2 (def_stmt) != phi_name)
     code = PLUS_EXPR;
 
   if (code == COND_EXPR)
@@ -2864,6 +2861,14 @@ vect_is_simple_reduction (loop_vec_info
           op4 = TREE_OPERAND (op3, 1);
           op3 = TREE_OPERAND (op3, 0);
         }
+      if (op3 == phi_name || op4 == phi_name)
+	{
+	  if (dump_enabled_p ())
+	    report_vect_op (MSG_MISSED_OPTIMIZATION, def_stmt,
+			    "reduction: condition depends on previous"
+			    " iteration: ");
+	  return NULL;
+	}
 
       op1 = gimple_assign_rhs2 (def_stmt);
       op2 = gimple_assign_rhs3 (def_stmt);
Index: gcc/testsuite/gcc.dg/vect/pr81815.c
===================================================================
--- /dev/null	2017-08-14 10:10:17.469973707 +0100
+++ gcc/testsuite/gcc.dg/vect/pr81815.c	2017-08-14 13:46:28.334496436 +0100
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+
+int __attribute__ ((noinline, noclone))
+f (int *x, int n)
+{
+  int b = 13;
+  for (int i = 0; i < n; ++i)
+    {
+      int next = x[i];
+      b = b < 100 ? next : 200;
+    }
+  return b;
+}
+
+static int res[32];
+
+int
+main (void)
+{
+  for (int i = 0; i < 32; ++i)
+    res[i] = i;
+  res[15] = 100;
+  if (f (res, 32) != 200)
+    __builtin_abort ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: PR81815: Invalid conditional reduction
  2017-08-14 14:22 PR81815: Invalid conditional reduction Richard Sandiford
@ 2017-08-15 12:47 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2017-08-15 12:47 UTC (permalink / raw)
  To: GCC Patches, Richard Sandiford

On Mon, Aug 14, 2017 at 2:48 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> We weren't checking whether the phi in a conditional reduction was
> used by the condition itself (which isn't a case we handle).
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Ok.

Richard.

> Thanks,
> Richard
>
>
> 2017-08-11  Richard Sandiford  <richard.sandiford@linaro.org>
>
> gcc/
>         PR tree-optimization/81835
>         * tree-vect-loop.c (vect_is_simple_reduction): Simply checks for
>         the phi SSA_NAME.  Check that the condition in a COND_EXPR does
>         not depend on the phi.
>
> gcc/testsuite/
>         PR tree-optimization/81835
>         * gcc.dg/vect/pr81815.c: New test.
>
> Index: gcc/tree-vect-loop.c
> ===================================================================
> --- gcc/tree-vect-loop.c        2017-08-04 11:41:05.710206063 +0100
> +++ gcc/tree-vect-loop.c        2017-08-14 13:46:28.336459006 +0100
> @@ -2690,15 +2690,15 @@ vect_is_simple_reduction (loop_vec_info
>    *double_reduc = false;
>    *v_reduc_type = TREE_CODE_REDUCTION;
>
> -  name = PHI_RESULT (phi);
> +  tree phi_name = PHI_RESULT (phi);
>    /* ???  If there are no uses of the PHI result the inner loop reduction
>       won't be detected as possibly double-reduction by vectorizable_reduction
>       because that tries to walk the PHI arg from the preheader edge which
>       can be constant.  See PR60382.  */
> -  if (has_zero_uses (name))
> +  if (has_zero_uses (phi_name))
>      return NULL;
>    nloop_uses = 0;
> -  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, name)
> +  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, phi_name)
>      {
>        gimple *use_stmt = USE_STMT (use_p);
>        if (is_gimple_debug (use_stmt))
> @@ -2847,10 +2847,7 @@ vect_is_simple_reduction (loop_vec_info
>       simply rewriting this into "res += -x[i]".  Avoid changing
>       gimple instruction for the first simple tests and only do this
>       if we're allowed to change code at all.  */
> -  if (code == MINUS_EXPR
> -      && ! ((op1 = gimple_assign_rhs2 (def_stmt))
> -           && TREE_CODE (op1) == SSA_NAME
> -           && SSA_NAME_DEF_STMT (op1) == phi))
> +  if (code == MINUS_EXPR && gimple_assign_rhs2 (def_stmt) != phi_name)
>      code = PLUS_EXPR;
>
>    if (code == COND_EXPR)
> @@ -2864,6 +2861,14 @@ vect_is_simple_reduction (loop_vec_info
>            op4 = TREE_OPERAND (op3, 1);
>            op3 = TREE_OPERAND (op3, 0);
>          }
> +      if (op3 == phi_name || op4 == phi_name)
> +       {
> +         if (dump_enabled_p ())
> +           report_vect_op (MSG_MISSED_OPTIMIZATION, def_stmt,
> +                           "reduction: condition depends on previous"
> +                           " iteration: ");
> +         return NULL;
> +       }
>
>        op1 = gimple_assign_rhs2 (def_stmt);
>        op2 = gimple_assign_rhs3 (def_stmt);
> Index: gcc/testsuite/gcc.dg/vect/pr81815.c
> ===================================================================
> --- /dev/null   2017-08-14 10:10:17.469973707 +0100
> +++ gcc/testsuite/gcc.dg/vect/pr81815.c 2017-08-14 13:46:28.334496436 +0100
> @@ -0,0 +1,26 @@
> +/* { dg-do run } */
> +
> +int __attribute__ ((noinline, noclone))
> +f (int *x, int n)
> +{
> +  int b = 13;
> +  for (int i = 0; i < n; ++i)
> +    {
> +      int next = x[i];
> +      b = b < 100 ? next : 200;
> +    }
> +  return b;
> +}
> +
> +static int res[32];
> +
> +int
> +main (void)
> +{
> +  for (int i = 0; i < 32; ++i)
> +    res[i] = i;
> +  res[15] = 100;
> +  if (f (res, 32) != 200)
> +    __builtin_abort ();
> +  return 0;
> +}

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-08-15 11:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-14 14:22 PR81815: Invalid conditional reduction Richard Sandiford
2017-08-15 12:47 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).