| Message ID | alpine.LNX.2.00.1012222039540.5746@monoid.intra.ispras.ru |
|---|---|
| State | New |
| Headers | show |
On 22.12.2010 20:57, Alexander Monakov wrote: > Hi, > > This patch drops bits of support for moving up unconditional jumps. The rest > of the scheduler does not handle it anyway, and most of the time dependencies > would prohibit such motion (in the testcase it's possible because other insns > are nops). While I'm happy with this patch, I would like to note that I'm rather tired of fixing various corner cases that are only seen in the scheduler when running it with -O0. I do believe that we need to force cleanup_cfg and DCE when not optimizing and stop caring. > > I've verified that the patch does not affect code generation on several cc1 .i > files at -O3 (targeting ia64). Bootstrapped on x86-64-linux, ia64-linux > (testsuite running). OK for trunk? I would also suggest to go further and check codegen differences on SPEC2k. Andrey
On 12/22/2010 04:02 PM, Andrey Belevantsev wrote: > On 22.12.2010 20:57, Alexander Monakov wrote: >> Hi, >> >> This patch drops bits of support for moving up unconditional jumps. >> The rest >> of the scheduler does not handle it anyway, and most of the time >> dependencies >> would prohibit such motion (in the testcase it's possible because >> other insns >> are nops). > While I'm happy with this patch, I would like to note that I'm rather > tired of fixing various corner cases that are only seen in the > scheduler when running it with -O0. I do believe that we need to > force cleanup_cfg and DCE when not optimizing and stop caring. > I don't think it is a good solution, Andrey. It is practically the same solution as preventing insn scheduling for -O0. While the passes are separate and have options to switch them on/off, the pass should work independently of the other passes work. >> >> I've verified that the patch does not affect code generation on >> several cc1 .i >> files at -O3 (targeting ia64). Bootstrapped on x86-64-linux, ia64-linux >> (testsuite running). OK for trunk? > I would also suggest to go further and check codegen differences on > SPEC2k. > Yes, it would be interesting to see differences.
On 12/22/2010 12:57 PM, Alexander Monakov wrote: > Hi, > > This patch drops bits of support for moving up unconditional jumps. The rest > of the scheduler does not handle it anyway, and most of the time dependencies > would prohibit such motion (in the testcase it's possible because other insns > are nops). > > I've verified that the patch does not affect code generation on several cc1 .i > files at -O3 (targeting ia64). Bootstrapped on x86-64-linux, ia64-linux > (testsuite running). OK for trunk? > Ok. Thanks for fixing the bug. > 2010-12-22 Alexander Monakov<amonakov@ispras.ru> > > PR rtl-optimization/47036 > * sel-sched-ir.c (fallthru_bb_of_jump): Remove special support for > unconditional jumps. > * sel-sched.c (moveup_expr): Ditto. > > testsuite: > * g++.dg/opt/pr47036.C: New.
On 23.12.2010 19:56, Vladimir Makarov wrote: > On 12/22/2010 04:02 PM, Andrey Belevantsev wrote: >> On 22.12.2010 20:57, Alexander Monakov wrote: >>> Hi, >>> >>> This patch drops bits of support for moving up unconditional jumps. The >>> rest >>> of the scheduler does not handle it anyway, and most of the time >>> dependencies >>> would prohibit such motion (in the testcase it's possible because other >>> insns >>> are nops). >> While I'm happy with this patch, I would like to note that I'm rather >> tired of fixing various corner cases that are only seen in the scheduler >> when running it with -O0. I do believe that we need to force cleanup_cfg >> and DCE when not optimizing and stop caring. >> > I don't think it is a good solution, Andrey. It is practically the same > solution as preventing insn scheduling for -O0. While the passes are > separate and have options to switch them on/off, the pass should work > independently of the other passes work. I guess you and Alexander are right, and I'm just tired and need a vacation :) Happy holidays, Vlad, and thanks for bearing with our patches, we kept you busy this year. Andrey
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c index 468dfd7..de40ba0 100644 --- a/gcc/sel-sched-ir.c +++ b/gcc/sel-sched-ir.c @@ -4441,9 +4441,6 @@ fallthru_bb_of_jump (rtx jump) if (!JUMP_P (jump)) return NULL; - if (any_uncondjump_p (jump)) - return single_succ (BLOCK_FOR_INSN (jump)); - if (!any_condjump_p (jump)) return NULL; diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c index 3b5603c..b5c9e57 100644 --- a/gcc/sel-sched.c +++ b/gcc/sel-sched.c @@ -2171,10 +2171,8 @@ moveup_expr (expr_t expr, insn_t through_insn, bool inside_insn_group, || ! in_current_region_p (fallthru_bb)) return MOVEUP_EXPR_NULL; - /* And it should be mutually exclusive with through_insn, or - be an unconditional jump. */ - if (! any_uncondjump_p (insn) - && ! sched_insns_conditions_mutex_p (insn, through_insn) + /* And it should be mutually exclusive with through_insn. */ + if (! sched_insns_conditions_mutex_p (insn, through_insn) && ! DEBUG_INSN_P (through_insn)) return MOVEUP_EXPR_NULL; } diff --git a/gcc/testsuite/g++.dg/opt/pr47036.C b/gcc/testsuite/g++.dg/opt/pr47036.C index e69de29..d6d5adc 100644 --- a/gcc/testsuite/g++.dg/opt/pr47036.C +++ b/gcc/testsuite/g++.dg/opt/pr47036.C @@ -0,0 +1,10 @@ +// { dg-do compile { target powerpc*-*-* ia64-*-* x86_64-*-* } } +// { dg-options "-fschedule-insns -fselective-scheduling -fno-dce" } + + +void foo () +{ + for (;;) + for (;;({break;})); +} +