| Message ID | e444d4ed-0522-3bf7-5d68-81d398a8d736@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 9, 2016 at 11:13 PM, Pat Haugen <pthaugen@linux.vnet.ibm.com> wrote: > The following fixes a problem introduced by my earlier loop unroller patch, https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01612.html. In instances where the niter expr is not reliable we need to still emit an initial peel copy of the loop. > > Bootstrap/regtest on powerpc64le with no new regressions. Ok for trunk? Ok. Richard. > -Pat > > > 2016-11-09 Pat Haugen <pthaugen@us.ibm.com> > > PR rtl-optimization/78241 > * loop-unroll.c (unroll_loop_runtime_iterations): Don't adjust 'niter', but > emit initial peel copy if niter expr is not reliable. > > > testsuite/ChangeLog: > 2016-11-09 Pat Haugen <pthaugen@us.ibm.com> > > * gcc.dg/pr78241.c: New test. > >
On Wed, Nov 9, 2016 at 2:13 PM, Pat Haugen <pthaugen@linux.vnet.ibm.com> wrote: > The following fixes a problem introduced by my earlier loop unroller patch, https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01612.html. In instances where the niter expr is not reliable we need to still emit an initial peel copy of the loop. > > Bootstrap/regtest on powerpc64le with no new regressions. Ok for trunk? This fixes the performance regression I reported with the original patch at https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01224.html . Thanks, Andrew > > -Pat > > > 2016-11-09 Pat Haugen <pthaugen@us.ibm.com> > > PR rtl-optimization/78241 > * loop-unroll.c (unroll_loop_runtime_iterations): Don't adjust 'niter', but > emit initial peel copy if niter expr is not reliable. > > > testsuite/ChangeLog: > 2016-11-09 Pat Haugen <pthaugen@us.ibm.com> > > * gcc.dg/pr78241.c: New test. > >
On 11/10/2016 06:54 PM, Andrew Pinski wrote: > On Wed, Nov 9, 2016 at 2:13 PM, Pat Haugen <pthaugen@linux.vnet.ibm.com> wrote: >> > The following fixes a problem introduced by my earlier loop unroller patch, https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01612.html. In instances where the niter expr is not reliable we need to still emit an initial peel copy of the loop. >> > >> > Bootstrap/regtest on powerpc64le with no new regressions. Ok for trunk? > This fixes the performance regression I reported with the original > patch at https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01224.html . That's good to hear your performance problem is gone. I'm thinking it's more of a side effect of this patch, which causes differences in basic block frequencies (which then affects decisions based on those frequencies). I'm currently looking at pr78116, that noted a couple degradations after my unroller patches, which I'm also guessing are due to BB frequency changes. We have 2 open pr's on phases that currently mess up BB frequencies, vectorizer (pr77536) and loop unroller (pr68212). -Pat
Index: gcc/loop-unroll.c =================================================================== --- gcc/loop-unroll.c (revision 241821) +++ gcc/loop-unroll.c (working copy) @@ -918,9 +918,10 @@ unroll_loop_runtime_iterations (struct l if (tmp != niter) emit_move_insn (niter, tmp); - /* For loops that exit at end, add one to niter to account for first pass - through loop body before reaching exit test. */ - if (exit_at_end) + /* For loops that exit at end and whose number of iterations is reliable, + add one to niter to account for first pass through loop body before + reaching exit test. */ + if (exit_at_end && !desc->noloop_assumptions) { niter = expand_simple_binop (desc->mode, PLUS, niter, const1_rtx, @@ -946,7 +947,7 @@ unroll_loop_runtime_iterations (struct l auto_sbitmap wont_exit (max_unroll + 2); - if (extra_zero_check) + if (extra_zero_check || desc->noloop_assumptions) { /* Peel the first copy of loop body. Leave the exit test if the number of iterations is not reliable. Also record the place of the extra zero Index: gcc/testsuite/gcc.dg/pr78241.c =================================================================== --- gcc/testsuite/gcc.dg/pr78241.c (revision 0) +++ gcc/testsuite/gcc.dg/pr78241.c (working copy) @@ -0,0 +1,20 @@ +/* { dg-do run } */ +/* { dg-options "-Og -funroll-loops" } */ + +static __attribute__((noinline, noclone)) unsigned +foo (unsigned x) +{ + do + x++; + while (x <= 15); + return x; +} + +int main () +{ + unsigned x = foo (-2); + if (x != (unsigned)-1) + __builtin_abort(); + return 0; +} +
The following fixes a problem introduced by my earlier loop unroller patch, https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01612.html. In instances where the niter expr is not reliable we need to still emit an initial peel copy of the loop. Bootstrap/regtest on powerpc64le with no new regressions. Ok for trunk? -Pat 2016-11-09 Pat Haugen <pthaugen@us.ibm.com> PR rtl-optimization/78241 * loop-unroll.c (unroll_loop_runtime_iterations): Don't adjust 'niter', but emit initial peel copy if niter expr is not reliable. testsuite/ChangeLog: 2016-11-09 Pat Haugen <pthaugen@us.ibm.com> * gcc.dg/pr78241.c: New test.