public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH PR96698] aarch64: ICE during GIMPLE pass:vect
@ 2020-08-20  2:44 yangyang (ET)
  2020-08-24  8:38 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: yangyang (ET) @ 2020-08-20  2:44 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

Hi, 

	This is a simple fix for PR96698.

	For the test case, there are two PHIs in the inner loop in pass_vect

		<bb 5> [local count: 719407024]:
		# b_26 = PHI <0(4), b_15(10)>
		# c_27 = PHI <0(4), b_26(10)>

	c_27 = PHI <0(4), b_26(10)> is detected to be a vectorizable nested cycle by vect_is_simple_reduction incorrectly, and # b_26 = PHI <0(4), b_15(10)> is marked as the reduc_def of it, resulting in the crash.

	This patch adjusts the order of judgements in vect_is_simple_reduction to avoid the incorrect detection.

	Added one testcase for this. Bootstrap and tested on both aarch64 and x86 Linux platform, no new regression witnessed.

	Ok for trunk?`

Thanks, 
Yang Yang


+2020-08-20  Yang Yang  <yangyang305@huawei.com>
+
+       PR tree-optimization/96698
+       * tree-vect-loop.c (vect_is_simple_reduction): Moves the analysis
+       of phi nodes above the analysis of nested cycle to avoid the
+       incorrect detection.
+

+2020-08-20  Yang Yang  <yangyang305@huawei.com>
+
+       PR tree-optimization/96698
+       * gcc.dg/pr96698.c: New test.
+


[-- Attachment #2: pr96698-v1.patch --]
[-- Type: application/octet-stream, Size: 3754 bytes --]

From d2ede22f123ab1e5cf303f86682cca41fd79eaa8 Mon Sep 17 00:00:00 2001
From: Yang Yang <yangyang305@huawei.com>
Date: Wed, 19 Aug 2020 19:49:09 +0800
Subject: [PATCH] vect: Fix an ICE in vect_is_simple_reduction

In vect_is_simple_reduction, an incorrect detection is returned when the
DEF_STMT is a phi node itself. This patch moves the analysis of phi nodes
above the analysis of nested cycle to avoid the incorrect detection.

2020-08-20  Yang Yang <yangyang305@huawei.com>

gcc/ChangeLog:

	PR tree-optimization/96698
	* tree-vect-loop.c (vect_is_simple_reduction): Moves the analysis
	of phi nodes above the analysis of nested cycle to avoid the
	incorrect detection.

gcc/testsuite/ChangeLog:

	PR tree-optimization/96698
	* gcc.dg/pr96698.c: New test.
---
 gcc/testsuite/gcc.dg/pr96698.c | 15 ++++++++++++
 gcc/tree-vect-loop.c           | 44 +++++++++++++++++-----------------
 2 files changed, 37 insertions(+), 22 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr96698.c

diff --git a/gcc/testsuite/gcc.dg/pr96698.c b/gcc/testsuite/gcc.dg/pr96698.c
new file mode 100644
index 00000000000..5e8416e36f8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr96698.c
@@ -0,0 +1,15 @@
+/* PR tree-optimization/96698 */
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+void test(int a, int* i) {
+  for (; a < 5; ++a) {
+    int b = 0;
+    int c = 0;
+    for (; b != -11; b--)
+      for (int d = 0; d ==0; d++) {
+        *i += c & a;
+        c = b;
+      }
+  }
+}
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index dba230f6320..52847540162 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -3348,28 +3348,6 @@ vect_is_simple_reduction (loop_vec_info loop_info, stmt_vec_info phi_info,
 	}
     }
 
-  /* If we are vectorizing an inner reduction we are executing that
-     in the original order only in case we are not dealing with a
-     double reduction.  */
-  if (nested_in_vect_loop && !inner_loop_of_double_reduc)
-    {
-      if (dump_enabled_p ())
-	report_vect_op (MSG_NOTE, def_stmt_info->stmt,
-			"detected nested cycle: ");
-      return def_stmt_info;
-    }
-
-  /* If this isn't a nested cycle or if the nested cycle reduction value
-     is used ouside of the inner loop we cannot handle uses of the reduction
-     value.  */
-  if (nlatch_def_loop_uses > 1 || nphi_def_loop_uses > 1)
-    {
-      if (dump_enabled_p ())
-	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
-			 "reduction used in loop.\n");
-      return NULL;
-    }
-
   /* If DEF_STMT is a phi node itself, we expect it to have a single argument
      defined in the inner loop.  */
   if (gphi *def_stmt = dyn_cast <gphi *> (def_stmt_info->stmt))
@@ -3405,6 +3383,28 @@ vect_is_simple_reduction (loop_vec_info loop_info, stmt_vec_info phi_info,
       return NULL;
     }
 
+  /* If we are vectorizing an inner reduction we are executing that
+     in the original order only in case we are not dealing with a
+     double reduction.  */
+  if (nested_in_vect_loop && !inner_loop_of_double_reduc)
+    {
+      if (dump_enabled_p ())
+	report_vect_op (MSG_NOTE, def_stmt_info->stmt,
+			"detected nested cycle: ");
+      return def_stmt_info;
+    }
+
+  /* If this isn't a nested cycle or if the nested cycle reduction value
+     is used ouside of the inner loop we cannot handle uses of the reduction
+     value.  */
+  if (nlatch_def_loop_uses > 1 || nphi_def_loop_uses > 1)
+    {
+      if (dump_enabled_p ())
+	dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+			"reduction used in loop.\n");
+      return NULL;
+    }
+
   /* Look for the expression computing latch_def from then loop PHI result.  */
   auto_vec<std::pair<ssa_op_iter, use_operand_p> > path;
   enum tree_code code;
-- 
2.19.1


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

* Re: [PATCH PR96698] aarch64: ICE during GIMPLE pass:vect
  2020-08-20  2:44 [PATCH PR96698] aarch64: ICE during GIMPLE pass:vect yangyang (ET)
@ 2020-08-24  8:38 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2020-08-24  8:38 UTC (permalink / raw)
  To: yangyang (ET); +Cc: gcc-patches

On Thu, Aug 20, 2020 at 4:45 AM yangyang (ET) <yangyang305@huawei.com> wrote:
>
> Hi,
>
>         This is a simple fix for PR96698.
>
>         For the test case, there are two PHIs in the inner loop in pass_vect
>
>                 <bb 5> [local count: 719407024]:
>                 # b_26 = PHI <0(4), b_15(10)>
>                 # c_27 = PHI <0(4), b_26(10)>
>
>         c_27 = PHI <0(4), b_26(10)> is detected to be a vectorizable nested cycle by vect_is_simple_reduction incorrectly, and # b_26 = PHI <0(4), b_15(10)> is marked as the reduc_def of it, resulting in the crash.
>
>         This patch adjusts the order of judgements in vect_is_simple_reduction to avoid the incorrect detection.
>
>         Added one testcase for this. Bootstrap and tested on both aarch64 and x86 Linux platform, no new regression witnessed.
>
>         Ok for trunk?`

It's still a nested cycle so I think the fix is not correct.  I'll
have a look myself later this week.

Richard.

> Thanks,
> Yang Yang
>
>
> +2020-08-20  Yang Yang  <yangyang305@huawei.com>
> +
> +       PR tree-optimization/96698
> +       * tree-vect-loop.c (vect_is_simple_reduction): Moves the analysis
> +       of phi nodes above the analysis of nested cycle to avoid the
> +       incorrect detection.
> +
>
> +2020-08-20  Yang Yang  <yangyang305@huawei.com>
> +
> +       PR tree-optimization/96698
> +       * gcc.dg/pr96698.c: New test.
> +
>

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

end of thread, other threads:[~2020-08-24  8:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20  2:44 [PATCH PR96698] aarch64: ICE during GIMPLE pass:vect yangyang (ET)
2020-08-24  8:38 ` 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).