| Message ID | 53732FE2.9010305@ispras.ru |
|---|---|
| State | New |
| Headers | show |
On Wed, May 14, 2014 at 10:57 AM, Andrey Belevantsev <abel@ispras.ru> wrote: > This ICE comes from the ix86_dependencies_evaluation_hook code assumption > that any scheduling region will be connected. This assumption is not > correct in case of the outer loops pipelining of the selective scheduler as > explained in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60901#c3. > Trying to add dependencies between insns from the different scheduling > regions results in a segfault within the dependency analyzer code. > > The fix is to adjust the code to account for the situation when basic > block's predecessors do not belong to the same scheduling region. > > Bootstrapped and tested on x86-64, OK for trunk? Branches? The fix is low > risk as the additional test should always be true for the regular scheduler. I don't know all scheduler details, so your opinion counts there. Let's put this fix to mainline first and after a week without problems, backport it to all release branches. > gcc/ > 2014-05-14 Andrey Belevantsev <abel@ispras.ru> > > PR rtl-optimization/60901 > > * config/i386/i386.c (ix86_dependencies_evaluation_hook): Check that > bb predecessor belongs to the same scheduling region. Adjust comment. > > testsuite/ > 2014-05-14 Andrey Belevantsev <abel@ispras.ru> > > PR rtl-optimization/60901 > * gcc.dg/pr60901.c: New test. +/* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */ +/* { dg-options "-O -fselective-scheduling -fschedule-insns -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops -fno-tree-dominator-opts" } */ + As this is clearly a target bug, let's put the test in gcc.target/i386 directory. You can remove target selector, and the test will run for 64bit and 32bit targets automatically. Thanks, Uros.
On 14.05.2014 13:09, Uros Bizjak wrote: > On Wed, May 14, 2014 at 10:57 AM, Andrey Belevantsev <abel@ispras.ru> wrote: > >> This ICE comes from the ix86_dependencies_evaluation_hook code assumption >> that any scheduling region will be connected. This assumption is not >> correct in case of the outer loops pipelining of the selective scheduler as >> explained in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60901#c3. >> Trying to add dependencies between insns from the different scheduling >> regions results in a segfault within the dependency analyzer code. >> >> The fix is to adjust the code to account for the situation when basic >> block's predecessors do not belong to the same scheduling region. >> >> Bootstrapped and tested on x86-64, OK for trunk? Branches? The fix is low >> risk as the additional test should always be true for the regular scheduler. > > I don't know all scheduler details, so your opinion counts there. > Let's put this fix to mainline first and after a week without > problems, backport it to all release branches. > >> gcc/ >> 2014-05-14 Andrey Belevantsev <abel@ispras.ru> >> >> PR rtl-optimization/60901 >> >> * config/i386/i386.c (ix86_dependencies_evaluation_hook): Check that >> bb predecessor belongs to the same scheduling region. Adjust comment. >> >> testsuite/ >> 2014-05-14 Andrey Belevantsev <abel@ispras.ru> >> >> PR rtl-optimization/60901 >> * gcc.dg/pr60901.c: New test. > > +/* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */ > +/* { dg-options "-O -fselective-scheduling -fschedule-insns > -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops > -fno-tree-dominator-opts" } */ > + > > As this is clearly a target bug, let's put the test in gcc.target/i386 > directory. You can remove target selector, and the test will run for > 64bit and 32bit targets automatically. Thanks for noticing, I forgot about this. Updated the test and committed in 210414. Andrey
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 99f0657..0274288 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -26254,13 +26254,17 @@ ix86_dependencies_evaluation_hook (rtx head, rtx tail) { edge e; edge_iterator ei; - /* Assume that region is SCC, i.e. all immediate predecessors - of non-head block are in the same region. */ + + /* Regions are SCCs with the exception of selective + scheduling with pipelining of outer blocks enabled. + So also check that immediate predecessors of a non-head + block are in the same region. */ FOR_EACH_EDGE (e, ei, bb->preds) { /* Avoid creating of loop-carried dependencies through - using topological odering in region. */ - if (BLOCK_TO_BB (bb->index) > BLOCK_TO_BB (e->src->index)) + using topological ordering in the region. */ + if (rgn == CONTAINING_RGN (e->src->index) + && BLOCK_TO_BB (bb->index) > BLOCK_TO_BB (e->src->index)) add_dependee_for_func_arg (first_arg, e->src); } } diff --git a/gcc/testsuite/gcc.dg/pr60901.c b/gcc/testsuite/gcc.dg/pr60901.c new file mode 100644 index 0000000..1350f16 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr60901.c @@ -0,0 +1,18 @@ +/* { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } */ +/* { dg-options "-O -fselective-scheduling -fschedule-insns -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops -fno-tree-dominator-opts" } */ + +extern int n; +extern void bar (void); +extern int baz (int); + +void +foo (void) +{ + int i, j; + for (j = 0; j < n; j++) + { + for (i = 1; i < j; i++) + bar (); + baz (0); + } +}