| Message ID | 87zib2pb7g.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 14, 2017 at 2:48 PM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > We weren't checking whether the phi in a conditional reduction was > used by the condition itself (which isn't a case we handle). > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Ok. Richard. > Thanks, > Richard > > > 2017-08-11 Richard Sandiford <richard.sandiford@linaro.org> > > gcc/ > PR tree-optimization/81835 > * tree-vect-loop.c (vect_is_simple_reduction): Simply checks for > the phi SSA_NAME. Check that the condition in a COND_EXPR does > not depend on the phi. > > gcc/testsuite/ > PR tree-optimization/81835 > * gcc.dg/vect/pr81815.c: New test. > > Index: gcc/tree-vect-loop.c > =================================================================== > --- gcc/tree-vect-loop.c 2017-08-04 11:41:05.710206063 +0100 > +++ gcc/tree-vect-loop.c 2017-08-14 13:46:28.336459006 +0100 > @@ -2690,15 +2690,15 @@ vect_is_simple_reduction (loop_vec_info > *double_reduc = false; > *v_reduc_type = TREE_CODE_REDUCTION; > > - name = PHI_RESULT (phi); > + tree phi_name = PHI_RESULT (phi); > /* ??? If there are no uses of the PHI result the inner loop reduction > won't be detected as possibly double-reduction by vectorizable_reduction > because that tries to walk the PHI arg from the preheader edge which > can be constant. See PR60382. */ > - if (has_zero_uses (name)) > + if (has_zero_uses (phi_name)) > return NULL; > nloop_uses = 0; > - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, name) > + FOR_EACH_IMM_USE_FAST (use_p, imm_iter, phi_name) > { > gimple *use_stmt = USE_STMT (use_p); > if (is_gimple_debug (use_stmt)) > @@ -2847,10 +2847,7 @@ vect_is_simple_reduction (loop_vec_info > simply rewriting this into "res += -x[i]". Avoid changing > gimple instruction for the first simple tests and only do this > if we're allowed to change code at all. */ > - if (code == MINUS_EXPR > - && ! ((op1 = gimple_assign_rhs2 (def_stmt)) > - && TREE_CODE (op1) == SSA_NAME > - && SSA_NAME_DEF_STMT (op1) == phi)) > + if (code == MINUS_EXPR && gimple_assign_rhs2 (def_stmt) != phi_name) > code = PLUS_EXPR; > > if (code == COND_EXPR) > @@ -2864,6 +2861,14 @@ vect_is_simple_reduction (loop_vec_info > op4 = TREE_OPERAND (op3, 1); > op3 = TREE_OPERAND (op3, 0); > } > + if (op3 == phi_name || op4 == phi_name) > + { > + if (dump_enabled_p ()) > + report_vect_op (MSG_MISSED_OPTIMIZATION, def_stmt, > + "reduction: condition depends on previous" > + " iteration: "); > + return NULL; > + } > > op1 = gimple_assign_rhs2 (def_stmt); > op2 = gimple_assign_rhs3 (def_stmt); > Index: gcc/testsuite/gcc.dg/vect/pr81815.c > =================================================================== > --- /dev/null 2017-08-14 10:10:17.469973707 +0100 > +++ gcc/testsuite/gcc.dg/vect/pr81815.c 2017-08-14 13:46:28.334496436 +0100 > @@ -0,0 +1,26 @@ > +/* { dg-do run } */ > + > +int __attribute__ ((noinline, noclone)) > +f (int *x, int n) > +{ > + int b = 13; > + for (int i = 0; i < n; ++i) > + { > + int next = x[i]; > + b = b < 100 ? next : 200; > + } > + return b; > +} > + > +static int res[32]; > + > +int > +main (void) > +{ > + for (int i = 0; i < 32; ++i) > + res[i] = i; > + res[15] = 100; > + if (f (res, 32) != 200) > + __builtin_abort (); > + return 0; > +}
Index: gcc/tree-vect-loop.c =================================================================== --- gcc/tree-vect-loop.c 2017-08-04 11:41:05.710206063 +0100 +++ gcc/tree-vect-loop.c 2017-08-14 13:46:28.336459006 +0100 @@ -2690,15 +2690,15 @@ vect_is_simple_reduction (loop_vec_info *double_reduc = false; *v_reduc_type = TREE_CODE_REDUCTION; - name = PHI_RESULT (phi); + tree phi_name = PHI_RESULT (phi); /* ??? If there are no uses of the PHI result the inner loop reduction won't be detected as possibly double-reduction by vectorizable_reduction because that tries to walk the PHI arg from the preheader edge which can be constant. See PR60382. */ - if (has_zero_uses (name)) + if (has_zero_uses (phi_name)) return NULL; nloop_uses = 0; - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, name) + FOR_EACH_IMM_USE_FAST (use_p, imm_iter, phi_name) { gimple *use_stmt = USE_STMT (use_p); if (is_gimple_debug (use_stmt)) @@ -2847,10 +2847,7 @@ vect_is_simple_reduction (loop_vec_info simply rewriting this into "res += -x[i]". Avoid changing gimple instruction for the first simple tests and only do this if we're allowed to change code at all. */ - if (code == MINUS_EXPR - && ! ((op1 = gimple_assign_rhs2 (def_stmt)) - && TREE_CODE (op1) == SSA_NAME - && SSA_NAME_DEF_STMT (op1) == phi)) + if (code == MINUS_EXPR && gimple_assign_rhs2 (def_stmt) != phi_name) code = PLUS_EXPR; if (code == COND_EXPR) @@ -2864,6 +2861,14 @@ vect_is_simple_reduction (loop_vec_info op4 = TREE_OPERAND (op3, 1); op3 = TREE_OPERAND (op3, 0); } + if (op3 == phi_name || op4 == phi_name) + { + if (dump_enabled_p ()) + report_vect_op (MSG_MISSED_OPTIMIZATION, def_stmt, + "reduction: condition depends on previous" + " iteration: "); + return NULL; + } op1 = gimple_assign_rhs2 (def_stmt); op2 = gimple_assign_rhs3 (def_stmt); Index: gcc/testsuite/gcc.dg/vect/pr81815.c =================================================================== --- /dev/null 2017-08-14 10:10:17.469973707 +0100 +++ gcc/testsuite/gcc.dg/vect/pr81815.c 2017-08-14 13:46:28.334496436 +0100 @@ -0,0 +1,26 @@ +/* { dg-do run } */ + +int __attribute__ ((noinline, noclone)) +f (int *x, int n) +{ + int b = 13; + for (int i = 0; i < n; ++i) + { + int next = x[i]; + b = b < 100 ? next : 200; + } + return b; +} + +static int res[32]; + +int +main (void) +{ + for (int i = 0; i < 32; ++i) + res[i] = i; + res[15] = 100; + if (f (res, 32) != 200) + __builtin_abort (); + return 0; +}