From 09a7858b2c53eccf28f780f5f3e4f2764f440eb1 Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Fri, 19 Jan 2018 11:57:47 +0000 Subject: [PATCH] Check whether any statements need masking (PR 83922) This PR is an odd case in which, due to the low optimisation level, we enter vectorisation with: outer1: x_1 = PHI ; ... inner: x_2 = 0; ... outer2: x_3 = PHI ; These statements are tentatively treated as a double reduction by vect_force_simple_reduction, but in the end only x_3 and x_2 are marked as relevant. vect_analyze_loop_operations skips over x_3, leaving the vectorizable_reduction check to a presumed future test of x_1, which in this case never happens. We therefore end up vectorising x_2 only (complete with peeling for niters!) and leave the scalar x_3 in place. This caused a segfault in the support for fully-masked loops, since there were no statements that needed masking. Fixed by checking for that. But I think this is also a flaw in vect_analyze_loop_operations. Outer loop vectorisation reduces the number of times that the inner loop is executed, so it wouldn't necessarily be valid to leave the scalar x_3 in place for all vectorisable x_2. There's already code to forbid that when x_1 isn't present: /* FORNOW: we currently don't support the case that these phis are not used in the outerloop (unless it is double reduction, i.e., this phi is vect_reduction_def), cause this case requires to actually do something here. */ I think we need to do the same if x_1 is present but not relevant. 2018-01-19 Richard Sandiford gcc/ PR tree-optimization/83922 * tree-vect-loop.c (vect_verify_full_masking): Return false if there are no statements that need masking. (vect_active_double_reduction_p): New function. (vect_analyze_loop_operations): Use it when handling phis that are not in the loop header. gcc/testsuite/ PR tree-optimization/83922 * gcc.dg/pr83922.c: New test. From-SVN: r256885 --- gcc/ChangeLog | 9 +++++++++ gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/gcc.dg/pr83922.c | 21 ++++++++++++++++++++ gcc/tree-vect-loop.c | 36 ++++++++++++++++++++++++++++++++-- 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr83922.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a86ea914ca5..c11f8c728b0 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2018-01-19 Richard Sandiford + + PR tree-optimization/83922 + * tree-vect-loop.c (vect_verify_full_masking): Return false if + there are no statements that need masking. + (vect_active_double_reduction_p): New function. + (vect_analyze_loop_operations): Use it when handling phis that + are not in the loop header. + 2018-01-19 Richard Sandiford PR tree-optimization/83914 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 44080e0b54a..f0485d310e8 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-01-19 Richard Sandiford + + PR tree-optimization/83922 + * gcc.dg/pr83922.c: New test. + 2018-01-19 Richard Sandiford PR tree-optimization/83914 diff --git a/gcc/testsuite/gcc.dg/pr83922.c b/gcc/testsuite/gcc.dg/pr83922.c new file mode 100644 index 00000000000..8f99f7724bf --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr83922.c @@ -0,0 +1,21 @@ +/* { dg-options "-O -ftree-vectorize" } */ + +int j4; + +void +k1 (int ak) +{ + while (ak < 1) + { + int ur; + + for (ur = 0; ur < 2; ++ur) + { + ++j4; + if (j4 != 0) + j4 = 0; + } + + ++ak; + } +} diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c index 8b2ecf84e3f..f75a48313a2 100644 --- a/gcc/tree-vect-loop.c +++ b/gcc/tree-vect-loop.c @@ -1294,6 +1294,12 @@ vect_verify_full_masking (loop_vec_info loop_vinfo) struct loop *loop = LOOP_VINFO_LOOP (loop_vinfo); unsigned int min_ni_width; + /* Use a normal loop if there are no statements that need masking. + This only happens in rare degenerate cases: it means that the loop + has no loads, no stores, and no live-out values. */ + if (LOOP_VINFO_MASKS (loop_vinfo).is_empty ()) + return false; + /* Get the maximum number of iterations that is representable in the counter type. */ tree ni_type = TREE_TYPE (LOOP_VINFO_NITERSM1 (loop_vinfo)); @@ -1739,6 +1745,33 @@ vect_update_vf_for_slp (loop_vec_info loop_vinfo) } } +/* Return true if STMT_INFO describes a double reduction phi and if + the other phi in the reduction is also relevant for vectorization. + This rejects cases such as: + + outer1: + x_1 = PHI ; + ... + + inner: + x_2 = ...; + ... + + outer2: + x_3 = PHI ; + + if nothing in x_2 or elsewhere makes x_1 relevant. */ + +static bool +vect_active_double_reduction_p (stmt_vec_info stmt_info) +{ + if (STMT_VINFO_DEF_TYPE (stmt_info) != vect_double_reduction_def) + return false; + + gimple *other_phi = STMT_VINFO_REDUC_DEF (stmt_info); + return STMT_VINFO_RELEVANT_P (vinfo_for_stmt (other_phi)); +} + /* Function vect_analyze_loop_operations. Scan the loop stmts and make sure they are all vectorizable. */ @@ -1786,8 +1819,7 @@ vect_analyze_loop_operations (loop_vec_info loop_vinfo) i.e., this phi is vect_reduction_def), cause this case requires to actually do something here. */ if (STMT_VINFO_LIVE_P (stmt_info) - && STMT_VINFO_DEF_TYPE (stmt_info) - != vect_double_reduction_def) + && !vect_active_double_reduction_p (stmt_info)) { if (dump_enabled_p ()) dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, -- 2.30.2