| Message ID | CAFiYyc0jmCTdKvKOZK-vZQ6c=2kE_VODDwk06XH1-tS1AO367w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Richard, Here is updated patch which includes (1) a test on exit proposed by you and (2) another test from PR68021 which is caught by new check on counted loop. Outer-loop unswitching is not performed for both new tests. Bootstrapping and regression testing did not show any new failures. Is it OK for trunk. ChangeLog: 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> PR tree-optimization/68021 PR tree-optimization/68906 * tree-ssa-loop-unswitch.c : Include couple header files. (tree_unswitch_outer_loop): Add check that an exit is not inside inner loop, use number_of_latch_executions to detect non-iterated loops. gcc/testsuite/ChangeLog * gcc.dg/torture/pr68021.c: New test. * gcc.dg/torture/pr68906.c: Likewise. 2015-12-16 15:51 GMT+03:00 Richard Biener <richard.guenther@gmail.com>: > On Wed, Dec 16, 2015 at 1:14 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: >> Hi All, >> >> Here is simple patch which cures the issue with outer-loop unswitching >> - added invocation of number_of_latch_executions() to reject >> unswitching for non-iterated loops. >> >> Bootstrapping and regression testing did not show any new failures. >> Is it OK for trunk? > > No, that looks like just papering over the issue. > > The issue (with the 2nd testcase at least) is that single_exit () accepts > an exit from the inner loop. > > Index: gcc/tree-ssa-loop-unswitch.c > =================================================================== > --- gcc/tree-ssa-loop-unswitch.c (revision 231686) > +++ gcc/tree-ssa-loop-unswitch.c (working copy) > @@ -431,7 +431,7 @@ tree_unswitch_outer_loop (struct loop *l > return false; > /* Accept loops with single exit only. */ > exit = single_exit (loop); > - if (!exit) > + if (!exit || exit->src->loop_father != loop) > return false; > /* Check that phi argument of exit edge is not defined inside loop. */ > if (!check_exit_phi (loop)) > > fixes the runtime testcase for me (not suitable for the testsuite due > to the infinite > looping though). > > Can you please bootstrap/test the above with your testcase? The above patch is > ok if it passes testing (no time myself right now) > > Thanks, > Richard. > >> ChangeLog: >> >> 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> >> >> PR tree-optimization/68906 >> * tree-ssa-loop-unswitch.c : Include couple header files. >> (tree_unswitch_outer_loop): Use number_of_latch_executions >> to reject non-iterated loops. >> >> gcc/testsuite/ChangeLog >> * gcc.dg/torture/pr68906.c: New test.
On Wed, Dec 16, 2015 at 3:36 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: > Richard, > > Here is updated patch which includes (1) a test on exit proposed by > you and (2) another test from PR68021 which is caught by new check on > counted loop. Outer-loop unswitching is not performed for both new > tests. As said I don't think /* If the loop is not expected to iterate, there is no need for unswitching. */ - iterations = estimated_loop_iterations_int (loop); - if (iterations >= 0 && iterations <= 1) + niters = number_of_latch_executions (loop); + if (!niters || chrec_contains_undetermined (niters)) { is good. We do want to retain the estimated_loop_iterations check as it takes into account profile data while yours lets through all counted loops. Also I don't see why SCEV needs to be able to analyze the IV to check for validity. Can you please split the patch into the part I suggested (which is ok) and the rest? Thanks, Richard. > > Bootstrapping and regression testing did not show any new failures. > > Is it OK for trunk. > > ChangeLog: > 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> > > PR tree-optimization/68021 > PR tree-optimization/68906 > * tree-ssa-loop-unswitch.c : Include couple header files. > (tree_unswitch_outer_loop): Add check that an exit is not inside inner > loop, use number_of_latch_executions to detect non-iterated loops. > > gcc/testsuite/ChangeLog > * gcc.dg/torture/pr68021.c: New test. > * gcc.dg/torture/pr68906.c: Likewise. > > 2015-12-16 15:51 GMT+03:00 Richard Biener <richard.guenther@gmail.com>: >> On Wed, Dec 16, 2015 at 1:14 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: >>> Hi All, >>> >>> Here is simple patch which cures the issue with outer-loop unswitching >>> - added invocation of number_of_latch_executions() to reject >>> unswitching for non-iterated loops. >>> >>> Bootstrapping and regression testing did not show any new failures. >>> Is it OK for trunk? >> >> No, that looks like just papering over the issue. >> >> The issue (with the 2nd testcase at least) is that single_exit () accepts >> an exit from the inner loop. >> >> Index: gcc/tree-ssa-loop-unswitch.c >> =================================================================== >> --- gcc/tree-ssa-loop-unswitch.c (revision 231686) >> +++ gcc/tree-ssa-loop-unswitch.c (working copy) >> @@ -431,7 +431,7 @@ tree_unswitch_outer_loop (struct loop *l >> return false; >> /* Accept loops with single exit only. */ >> exit = single_exit (loop); >> - if (!exit) >> + if (!exit || exit->src->loop_father != loop) >> return false; >> /* Check that phi argument of exit edge is not defined inside loop. */ >> if (!check_exit_phi (loop)) >> >> fixes the runtime testcase for me (not suitable for the testsuite due >> to the infinite >> looping though). >> >> Can you please bootstrap/test the above with your testcase? The above patch is >> ok if it passes testing (no time myself right now) >> >> Thanks, >> Richard. >> >>> ChangeLog: >>> >>> 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> >>> >>> PR tree-optimization/68906 >>> * tree-ssa-loop-unswitch.c : Include couple header files. >>> (tree_unswitch_outer_loop): Use number_of_latch_executions >>> to reject non-iterated loops. >>> >>> gcc/testsuite/ChangeLog >>> * gcc.dg/torture/pr68906.c: New test.
Richard, Here is modified patch which checks only that exit block belongs to loop. Bootstrapping and regression testing were successful. Is it OK for trunk? ChangeLog: 2014-12-17 Yuri Rumyantsev <ysrumyan@gmail.com> PR tree-optimization/68906 * tree-ssa-loop-unswitch.c (tree_unswitch_outer_loop): Add a check that an exit block belongs to LOOP. gcc/testsuite/ChangeLog * gcc.dg/torture/pr68906.c: New test. . 2015-12-16 17:49 GMT+03:00 Richard Biener <richard.guenther@gmail.com>: > On Wed, Dec 16, 2015 at 3:36 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: >> Richard, >> >> Here is updated patch which includes (1) a test on exit proposed by >> you and (2) another test from PR68021 which is caught by new check on >> counted loop. Outer-loop unswitching is not performed for both new >> tests. > > As said I don't think > > /* If the loop is not expected to iterate, there is no need > for unswitching. */ > - iterations = estimated_loop_iterations_int (loop); > - if (iterations >= 0 && iterations <= 1) > + niters = number_of_latch_executions (loop); > + if (!niters || chrec_contains_undetermined (niters)) > { > > is good. We do want to retain the estimated_loop_iterations check > as it takes into account profile data while yours lets through all > counted loops. > > Also I don't see why SCEV needs to be able to analyze the IV to check > for validity. > > Can you please split the patch into the part I suggested (which is ok) > and the rest? > > Thanks, > Richard. > >> >> Bootstrapping and regression testing did not show any new failures. >> >> Is it OK for trunk. >> >> ChangeLog: >> 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> >> >> PR tree-optimization/68021 >> PR tree-optimization/68906 >> * tree-ssa-loop-unswitch.c : Include couple header files. >> (tree_unswitch_outer_loop): Add check that an exit is not inside inner >> loop, use number_of_latch_executions to detect non-iterated loops. >> >> gcc/testsuite/ChangeLog >> * gcc.dg/torture/pr68021.c: New test. >> * gcc.dg/torture/pr68906.c: Likewise. >> >> 2015-12-16 15:51 GMT+03:00 Richard Biener <richard.guenther@gmail.com>: >>> On Wed, Dec 16, 2015 at 1:14 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: >>>> Hi All, >>>> >>>> Here is simple patch which cures the issue with outer-loop unswitching >>>> - added invocation of number_of_latch_executions() to reject >>>> unswitching for non-iterated loops. >>>> >>>> Bootstrapping and regression testing did not show any new failures. >>>> Is it OK for trunk? >>> >>> No, that looks like just papering over the issue. >>> >>> The issue (with the 2nd testcase at least) is that single_exit () accepts >>> an exit from the inner loop. >>> >>> Index: gcc/tree-ssa-loop-unswitch.c >>> =================================================================== >>> --- gcc/tree-ssa-loop-unswitch.c (revision 231686) >>> +++ gcc/tree-ssa-loop-unswitch.c (working copy) >>> @@ -431,7 +431,7 @@ tree_unswitch_outer_loop (struct loop *l >>> return false; >>> /* Accept loops with single exit only. */ >>> exit = single_exit (loop); >>> - if (!exit) >>> + if (!exit || exit->src->loop_father != loop) >>> return false; >>> /* Check that phi argument of exit edge is not defined inside loop. */ >>> if (!check_exit_phi (loop)) >>> >>> fixes the runtime testcase for me (not suitable for the testsuite due >>> to the infinite >>> looping though). >>> >>> Can you please bootstrap/test the above with your testcase? The above patch is >>> ok if it passes testing (no time myself right now) >>> >>> Thanks, >>> Richard. >>> >>>> ChangeLog: >>>> >>>> 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> >>>> >>>> PR tree-optimization/68906 >>>> * tree-ssa-loop-unswitch.c : Include couple header files. >>>> (tree_unswitch_outer_loop): Use number_of_latch_executions >>>> to reject non-iterated loops. >>>> >>>> gcc/testsuite/ChangeLog >>>> * gcc.dg/torture/pr68906.c: New test.
On Thu, Dec 17, 2015 at 10:21 AM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: > Richard, > > Here is modified patch which checks only that exit block belongs to loop. > > Bootstrapping and regression testing were successful. > Is it OK for trunk? Ok with the spurious @@ -441,7 +441,7 @@ iterations = estimated_loop_iterations_int (loop); if (iterations >= 0 && iterations <= 1) { - if (dump_file && (dump_flags & TDF_DETAILS)) + if (dump_enabled_p ()) fprintf (dump_file, ";; Not unswitching, loop is not expected" " to iterate\n"); return false; hunk removed. Thanks, Richard. > ChangeLog: > 2014-12-17 Yuri Rumyantsev <ysrumyan@gmail.com> > > PR tree-optimization/68906 > * tree-ssa-loop-unswitch.c (tree_unswitch_outer_loop): Add a check > that an exit block belongs to LOOP. > > gcc/testsuite/ChangeLog > * gcc.dg/torture/pr68906.c: New test. > . > > 2015-12-16 17:49 GMT+03:00 Richard Biener <richard.guenther@gmail.com>: >> On Wed, Dec 16, 2015 at 3:36 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: >>> Richard, >>> >>> Here is updated patch which includes (1) a test on exit proposed by >>> you and (2) another test from PR68021 which is caught by new check on >>> counted loop. Outer-loop unswitching is not performed for both new >>> tests. >> >> As said I don't think >> >> /* If the loop is not expected to iterate, there is no need >> for unswitching. */ >> - iterations = estimated_loop_iterations_int (loop); >> - if (iterations >= 0 && iterations <= 1) >> + niters = number_of_latch_executions (loop); >> + if (!niters || chrec_contains_undetermined (niters)) >> { >> >> is good. We do want to retain the estimated_loop_iterations check >> as it takes into account profile data while yours lets through all >> counted loops. >> >> Also I don't see why SCEV needs to be able to analyze the IV to check >> for validity. >> >> Can you please split the patch into the part I suggested (which is ok) >> and the rest? >> >> Thanks, >> Richard. >> >>> >>> Bootstrapping and regression testing did not show any new failures. >>> >>> Is it OK for trunk. >>> >>> ChangeLog: >>> 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> >>> >>> PR tree-optimization/68021 >>> PR tree-optimization/68906 >>> * tree-ssa-loop-unswitch.c : Include couple header files. >>> (tree_unswitch_outer_loop): Add check that an exit is not inside inner >>> loop, use number_of_latch_executions to detect non-iterated loops. >>> >>> gcc/testsuite/ChangeLog >>> * gcc.dg/torture/pr68021.c: New test. >>> * gcc.dg/torture/pr68906.c: Likewise. >>> >>> 2015-12-16 15:51 GMT+03:00 Richard Biener <richard.guenther@gmail.com>: >>>> On Wed, Dec 16, 2015 at 1:14 PM, Yuri Rumyantsev <ysrumyan@gmail.com> wrote: >>>>> Hi All, >>>>> >>>>> Here is simple patch which cures the issue with outer-loop unswitching >>>>> - added invocation of number_of_latch_executions() to reject >>>>> unswitching for non-iterated loops. >>>>> >>>>> Bootstrapping and regression testing did not show any new failures. >>>>> Is it OK for trunk? >>>> >>>> No, that looks like just papering over the issue. >>>> >>>> The issue (with the 2nd testcase at least) is that single_exit () accepts >>>> an exit from the inner loop. >>>> >>>> Index: gcc/tree-ssa-loop-unswitch.c >>>> =================================================================== >>>> --- gcc/tree-ssa-loop-unswitch.c (revision 231686) >>>> +++ gcc/tree-ssa-loop-unswitch.c (working copy) >>>> @@ -431,7 +431,7 @@ tree_unswitch_outer_loop (struct loop *l >>>> return false; >>>> /* Accept loops with single exit only. */ >>>> exit = single_exit (loop); >>>> - if (!exit) >>>> + if (!exit || exit->src->loop_father != loop) >>>> return false; >>>> /* Check that phi argument of exit edge is not defined inside loop. */ >>>> if (!check_exit_phi (loop)) >>>> >>>> fixes the runtime testcase for me (not suitable for the testsuite due >>>> to the infinite >>>> looping though). >>>> >>>> Can you please bootstrap/test the above with your testcase? The above patch is >>>> ok if it passes testing (no time myself right now) >>>> >>>> Thanks, >>>> Richard. >>>> >>>>> ChangeLog: >>>>> >>>>> 2014-12-16 Yuri Rumyantsev <ysrumyan@gmail.com> >>>>> >>>>> PR tree-optimization/68906 >>>>> * tree-ssa-loop-unswitch.c : Include couple header files. >>>>> (tree_unswitch_outer_loop): Use number_of_latch_executions >>>>> to reject non-iterated loops. >>>>> >>>>> gcc/testsuite/ChangeLog >>>>> * gcc.dg/torture/pr68906.c: New test.
Index: gcc/tree-ssa-loop-unswitch.c =================================================================== --- gcc/tree-ssa-loop-unswitch.c (revision 231686) +++ gcc/tree-ssa-loop-unswitch.c (working copy) @@ -431,7 +431,7 @@ tree_unswitch_outer_loop (struct loop *l return false; /* Accept loops with single exit only. */ exit = single_exit (loop); - if (!exit) + if (!exit || exit->src->loop_father != loop) return false; /* Check that phi argument of exit edge is not defined inside loop. */ if (!check_exit_phi (loop))