* Avoid matching the same pattern statement twice
@ 2018-07-03 8:01 Richard Sandiford
2018-07-03 9:36 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Richard Sandiford @ 2018-07-03 8:01 UTC (permalink / raw)
To: gcc-patches
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.
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;
+ }
+}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Avoid matching the same pattern statement twice
2018-07-03 8:01 Avoid matching the same pattern statement twice Richard Sandiford
@ 2018-07-03 9:36 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2018-07-03 9:36 UTC (permalink / raw)
To: GCC Patches, richard.sandiford
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;
> + }
> +}
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-07-03 9:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-03 8:01 Avoid matching the same pattern statement twice Richard Sandiford
2018-07-03 9:36 ` 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).