Avoid matching the same pattern statement twice

Message ID 87h8lg92bt.fsf@arm.com
State New
Headers show
Series
  • Avoid matching the same pattern statement twice
Related show

Commit Message

Richard Sandiford July 3, 2018, 7:59 a.m.
r262275 allowed pattern matching on pattern statements.  Testing for
SVE on more benchmarks showed a case where this interacted badly
with 14/n.

The new over-widening detection could narrow a COND_EXPR A to another
COND_EXPR B, which mixed_size_cond could then match.  This was working
as expected.  However, we left B (now dead) in the pattern definition
sequence with a non-null PATTERN_DEF_SEQ.  mask_conversion also
matched B, and unlike most recognisers, didn't clear PATTERN_DEF_SEQ
before adding statements to it.  This meant that the statements
created by mixed_size_cond appeared in two supposedy separate
sequences, causing much confusion.

This patch removes pattern statements that are replaced by further
pattern statements.  As a belt-and-braces fix, it also nullifies
PATTERN_DEF_SEQ on failure, in the same way Richard B. did recently
for RELATED_STMT.

I have patches to clean up the PATTERN_DEF_SEQ handling, but they
only apply after the complete PR85694 sequence, whereas this needs
to go in before 14/n.

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

Richard


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

gcc/
	* tree-vect-patterns.c (vect_mark_pattern_stmts): Remove pattern
	statements that have been replaced by further pattern statements.
	(vect_pattern_recog_1): Clear STMT_VINFO_PATTERN_DEF_SEQ on failure.

gcc/testsuite/
	* gcc.dg/vect/vect-mixed-size-cond-1.c: New test.

Comments

Richard Biener July 3, 2018, 9:35 a.m. | #1
On Tue, Jul 3, 2018 at 10:02 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>

> r262275 allowed pattern matching on pattern statements.  Testing for

> SVE on more benchmarks showed a case where this interacted badly

> with 14/n.

>

> The new over-widening detection could narrow a COND_EXPR A to another

> COND_EXPR B, which mixed_size_cond could then match.  This was working

> as expected.  However, we left B (now dead) in the pattern definition

> sequence with a non-null PATTERN_DEF_SEQ.  mask_conversion also

> matched B, and unlike most recognisers, didn't clear PATTERN_DEF_SEQ

> before adding statements to it.  This meant that the statements

> created by mixed_size_cond appeared in two supposedy separate

> sequences, causing much confusion.

>

> This patch removes pattern statements that are replaced by further

> pattern statements.  As a belt-and-braces fix, it also nullifies

> PATTERN_DEF_SEQ on failure, in the same way Richard B. did recently

> for RELATED_STMT.

>

> I have patches to clean up the PATTERN_DEF_SEQ handling, but they

> only apply after the complete PR85694 sequence, whereas this needs

> to go in before 14/n.

>

> Tested on aarch64-linux-gnu, arm-linux-gnueabihf and x86_64-linux-gnu.

> OK to install?


OK.

Richard.

> Richard

>

>

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

>

> gcc/

>         * tree-vect-patterns.c (vect_mark_pattern_stmts): Remove pattern

>         statements that have been replaced by further pattern statements.

>         (vect_pattern_recog_1): Clear STMT_VINFO_PATTERN_DEF_SEQ on failure.

>

> gcc/testsuite/

>         * gcc.dg/vect/vect-mixed-size-cond-1.c: New test.

>

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

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

> --- gcc/tree-vect-patterns.c    2018-07-02 14:34:45.857732632 +0100

> +++ gcc/tree-vect-patterns.c    2018-07-03 08:56:56.610251460 +0100

> @@ -4295,6 +4295,9 @@ vect_mark_pattern_stmts (gimple *orig_st

>        gimple_stmt_iterator gsi = gsi_for_stmt (orig_stmt, orig_def_seq);

>        gsi_insert_seq_before_without_update (&gsi, def_seq, GSI_SAME_STMT);

>        gsi_insert_before_without_update (&gsi, pattern_stmt, GSI_SAME_STMT);

> +

> +      /* Remove the pattern statement that this new pattern replaces.  */

> +      gsi_remove (&gsi, false);

>      }

>    else

>      vect_set_pattern_stmt (pattern_stmt, orig_stmt_info, pattern_vectype);

> @@ -4358,6 +4361,8 @@ vect_pattern_recog_1 (vect_recog_func *r

>           if (!is_pattern_stmt_p (stmt_info))

>             STMT_VINFO_RELATED_STMT (stmt_info) = NULL;

>         }

> +      /* Clear any half-formed pattern definition sequence.  */

> +      STMT_VINFO_PATTERN_DEF_SEQ (stmt_info) = NULL;

>        return;

>      }

>

> Index: gcc/testsuite/gcc.dg/vect/vect-mixed-size-cond-1.c

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

> --- /dev/null   2018-06-13 14:36:57.192460992 +0100

> +++ gcc/testsuite/gcc.dg/vect/vect-mixed-size-cond-1.c  2018-07-03 08:56:56.610251460 +0100

> @@ -0,0 +1,14 @@

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

> +

> +int

> +f (unsigned char *restrict x, short *restrict y)

> +{

> +  for (int i = 0; i < 100; ++i)

> +    {

> +      unsigned short a = (x[i] + 11) >> 1;

> +      unsigned short b = (x[i] + 42) >> 2;

> +      unsigned short cmp = y[i] == 0 ? a : b;

> +      int res = cmp + 1;

> +      x[i] = res;

> +    }

> +}

Patch

Index: gcc/tree-vect-patterns.c
===================================================================
--- gcc/tree-vect-patterns.c	2018-07-02 14:34:45.857732632 +0100
+++ gcc/tree-vect-patterns.c	2018-07-03 08:56:56.610251460 +0100
@@ -4295,6 +4295,9 @@  vect_mark_pattern_stmts (gimple *orig_st
       gimple_stmt_iterator gsi = gsi_for_stmt (orig_stmt, orig_def_seq);
       gsi_insert_seq_before_without_update (&gsi, def_seq, GSI_SAME_STMT);
       gsi_insert_before_without_update (&gsi, pattern_stmt, GSI_SAME_STMT);
+
+      /* Remove the pattern statement that this new pattern replaces.  */
+      gsi_remove (&gsi, false);
     }
   else
     vect_set_pattern_stmt (pattern_stmt, orig_stmt_info, pattern_vectype);
@@ -4358,6 +4361,8 @@  vect_pattern_recog_1 (vect_recog_func *r
 	  if (!is_pattern_stmt_p (stmt_info))
 	    STMT_VINFO_RELATED_STMT (stmt_info) = NULL;
 	}
+      /* Clear any half-formed pattern definition sequence.  */
+      STMT_VINFO_PATTERN_DEF_SEQ (stmt_info) = NULL;
       return;
     }
 
Index: gcc/testsuite/gcc.dg/vect/vect-mixed-size-cond-1.c
===================================================================
--- /dev/null	2018-06-13 14:36:57.192460992 +0100
+++ gcc/testsuite/gcc.dg/vect/vect-mixed-size-cond-1.c	2018-07-03 08:56:56.610251460 +0100
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+
+int
+f (unsigned char *restrict x, short *restrict y)
+{
+  for (int i = 0; i < 100; ++i)
+    {
+      unsigned short a = (x[i] + 11) >> 1;
+      unsigned short b = (x[i] + 42) >> 2;
+      unsigned short cmp = y[i] == 0 ? a : b;
+      int res = cmp + 1;
+      x[i] = res;
+    }
+}