Patchwork Fix PR tree-optimization/52870 (another crash in SLP pattern detection)

login
register
mail settings
Submitter Ulrich Weigand
Date April 6, 2012, 6:30 p.m.
Message ID <201204061830.q36IUnYd002379@d06av02.portsmouth.uk.ibm.com>
Download mbox | patch
Permalink /patch/151242/
State New
Headers show

Comments

Ulrich Weigand - April 6, 2012, 6:30 p.m.
Hello,

PR 52870 is another crash in vectorizer pattern detection that was uncovered
by Ira's patch to enable patterns for SLP as well.

In this case, the problem is that vect_recog_widen_mult_pattern detects a
statement as part of a pattern, but that statement is actually outside of
the basic block SLP is currently working on.  This means the statement
has no stmt_vinfo allocated, and thus SLP crashes later on when operating
on the statement.

Note that in theory this could already have happened before that latest
patch, if vect_recog_widen_mult_pattern would have detected a statement
outside the current *loop*.  That is apparently much rarer, though.

The fix is to verify that the statement is actually part of the current
basic block or loop, as appropriate.  That same check is already performed
in various other pattern detection routines.

Tested on i686-linux and x86_64 with no regressions.

OK for mainline?

Bye,
Ulrich



ChangeLog:

	gcc/
	PR tree-optimization/52870
	* tree-vect-patterns.c (vect_recog_widen_mult_pattern): Verify that
	presumed pattern statement is within the same loop or basic block.

	gcc/testsuite/
	PR tree-optimization/52870
	* gcc.dg/vect/pr52870.c: New test.
Richard Guenther - April 9, 2012, 10:52 a.m.
On Fri, Apr 6, 2012 at 8:30 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> PR 52870 is another crash in vectorizer pattern detection that was uncovered
> by Ira's patch to enable patterns for SLP as well.
>
> In this case, the problem is that vect_recog_widen_mult_pattern detects a
> statement as part of a pattern, but that statement is actually outside of
> the basic block SLP is currently working on.  This means the statement
> has no stmt_vinfo allocated, and thus SLP crashes later on when operating
> on the statement.
>
> Note that in theory this could already have happened before that latest
> patch, if vect_recog_widen_mult_pattern would have detected a statement
> outside the current *loop*.  That is apparently much rarer, though.
>
> The fix is to verify that the statement is actually part of the current
> basic block or loop, as appropriate.  That same check is already performed
> in various other pattern detection routines.
>
> Tested on i686-linux and x86_64 with no regressions.
>
> OK for mainline?

Ok.

Thanks,
Richard.

> Bye,
> Ulrich
>
>
>
> ChangeLog:
>
>        gcc/
>        PR tree-optimization/52870
>        * tree-vect-patterns.c (vect_recog_widen_mult_pattern): Verify that
>        presumed pattern statement is within the same loop or basic block.
>
>        gcc/testsuite/
>        PR tree-optimization/52870
>        * gcc.dg/vect/pr52870.c: New test.
>
>
> === added file 'gcc/testsuite/gcc.dg/vect/pr52870.c'
> --- gcc/testsuite/gcc.dg/vect/pr52870.c 1970-01-01 00:00:00 +0000
> +++ gcc/testsuite/gcc.dg/vect/pr52870.c 2012-04-04 16:18:08 +0000
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -ftree-vectorize" } */
> +
> +long
> +test (int *x)
> +{
> +  unsigned long sx, xprec;
> +
> +  sx = *x >= 0 ? *x : -*x;
> +
> +  xprec = sx * 64;
> +
> +  if (sx < 16384)
> +    foo (sx);
> +
> +  return xprec;
> +}
>
> === modified file 'gcc/tree-vect-patterns.c'
> --- gcc/tree-vect-patterns.c    2012-02-17 16:18:02 +0000
> +++ gcc/tree-vect-patterns.c    2012-04-04 16:18:08 +0000
> @@ -564,6 +564,16 @@
>   VEC (tree, heap) *dummy_vec;
>   bool op1_ok;
>   bool promotion;
> +  loop_vec_info loop_vinfo;
> +  struct loop *loop = NULL;
> +  bb_vec_info bb_vinfo;
> +  stmt_vec_info stmt_vinfo;
> +
> +  stmt_vinfo = vinfo_for_stmt (last_stmt);
> +  loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo);
> +  bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo);
> +  if (loop_vinfo)
> +    loop = LOOP_VINFO_LOOP (loop_vinfo);
>
>   if (!is_gimple_assign (last_stmt))
>     return NULL;
> @@ -635,6 +645,11 @@
>           || gimple_assign_rhs_code (use_stmt) != NOP_EXPR)
>         return NULL;
>
> +      if (!gimple_bb (use_stmt)
> +         || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (use_stmt)))
> +         || (!loop && gimple_bb (use_stmt) != BB_VINFO_BB (bb_vinfo)))
> +       return NULL;
> +
>       use_lhs = gimple_assign_lhs (use_stmt);
>       use_type = TREE_TYPE (use_lhs);
>       if (!INTEGRAL_TYPE_P (use_type)
>
> --
>  Dr. Ulrich Weigand
>  GNU Toolchain for Linux on System z and Cell BE
>  Ulrich.Weigand@de.ibm.com
>

Patch

=== added file 'gcc/testsuite/gcc.dg/vect/pr52870.c'
--- gcc/testsuite/gcc.dg/vect/pr52870.c	1970-01-01 00:00:00 +0000
+++ gcc/testsuite/gcc.dg/vect/pr52870.c	2012-04-04 16:18:08 +0000
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1 -ftree-vectorize" } */
+
+long
+test (int *x)
+{
+  unsigned long sx, xprec;
+
+  sx = *x >= 0 ? *x : -*x;
+
+  xprec = sx * 64;
+
+  if (sx < 16384)
+    foo (sx);
+
+  return xprec;
+}

=== modified file 'gcc/tree-vect-patterns.c'
--- gcc/tree-vect-patterns.c	2012-02-17 16:18:02 +0000
+++ gcc/tree-vect-patterns.c	2012-04-04 16:18:08 +0000
@@ -564,6 +564,16 @@ 
   VEC (tree, heap) *dummy_vec;
   bool op1_ok;
   bool promotion;
+  loop_vec_info loop_vinfo;
+  struct loop *loop = NULL;
+  bb_vec_info bb_vinfo;
+  stmt_vec_info stmt_vinfo;
+
+  stmt_vinfo = vinfo_for_stmt (last_stmt);
+  loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_vinfo);
+  bb_vinfo = STMT_VINFO_BB_VINFO (stmt_vinfo);
+  if (loop_vinfo)
+    loop = LOOP_VINFO_LOOP (loop_vinfo);
 
   if (!is_gimple_assign (last_stmt))
     return NULL;
@@ -635,6 +645,11 @@ 
           || gimple_assign_rhs_code (use_stmt) != NOP_EXPR)
         return NULL;
 
+      if (!gimple_bb (use_stmt)
+	  || (loop && !flow_bb_inside_loop_p (loop, gimple_bb (use_stmt)))
+	  || (!loop && gimple_bb (use_stmt) != BB_VINFO_BB (bb_vinfo)))
+	return NULL;
+
       use_lhs = gimple_assign_lhs (use_stmt);
       use_type = TREE_TYPE (use_lhs);
       if (!INTEGRAL_TYPE_P (use_type)