On Wed, 2019-01-23 at 16:52 +0300, Alexander Monakov wrote: > On Wed, 23 Jan 2019, Andrey Belevantsev wrote: > > > For that, I'm not sure. Your patch will leave the tablejump > > unscheduled at > > all, i.e. any fields like INSN_TICK would be unfilled and thus the > > later > > passes like bundling on ia64 will not work. Maybe we can just stop > > tablejumps from moving within its block, Alexander? > > On the Bugzilla there's an alternative patch by Segher that adjusts > the assert > to accept this situation (there's still a barrier insn after the > tablejump insn, > it's just after a jump_table_data insn rather than immediately > following the > jump). That should be a better approach, and David said he was > testing it. > > That said, I'm really concerned that on this testcase we should not > be moving > the tablejump *at all*: we are moving it up past a 'use ax' insn (the > use is > for the function's return value). So after the move the use is in an > unreachable > block, which makes no sense. > > So my concern is that just fixing the assert changes the issue from > the ICE to a > (latent) wrong-code issue. > > There should have been an anti-dependence between the use and the > jump. I'll try > to investigate. > > Alexander
Thanks everyone for their clarifications (this is somewhat outside my normal comfort zone of diagnostics/frontend stuff...) Here's Segher's patch to sched-ebb.c, along with a couple of compile-only testcases I put together (which triggered ICEs on x86_64 and powerpc without the sched-ebb.c fix). Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, and this fixes the ICE in the the powerpc testcase. That said, I share your concern that this might be papering over a latent wrong-code issue. Dave gcc/ChangeLog: PR rtl-optimization/88347 PR rtl-optimization/88423 * sched-ebb.c (begin_move_insn): Update assertion to handle table jumps. gcc/testsuite/ChangeLog: PR rtl-optimization/88347 PR rtl-optimization/88423 * gcc.c-torture/compile/pr88347.c: New test. * gcc.c-torture/compile/pr88423.c: New test. --- gcc/sched-ebb.c | 6 +++++- gcc/testsuite/gcc.c-torture/compile/pr88347.c | 4 ++++ gcc/testsuite/gcc.c-torture/compile/pr88423.c | 5 +++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr88347.c create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr88423.c diff --git a/gcc/sched-ebb.c b/gcc/sched-ebb.c index d459e09..76a72b0 100644 --- a/gcc/sched-ebb.c +++ b/gcc/sched-ebb.c @@ -172,7 +172,11 @@ begin_move_insn (rtx_insn *insn, rtx_insn *last) if (e) gcc_checking_assert (NOTE_P (x) || LABEL_P (x)); else - gcc_checking_assert (BARRIER_P (x)); + { + if (LABEL_P (x) && JUMP_TABLE_DATA_P (NEXT_INSN (x))) + x = NEXT_INSN (NEXT_INSN (x)); + gcc_checking_assert (BARRIER_P (x)); + } } if (e) diff --git a/gcc/testsuite/gcc.c-torture/compile/pr88347.c b/gcc/testsuite/gcc.c-torture/compile/pr88347.c new file mode 100644 index 0000000..4e9b438 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr88347.c @@ -0,0 +1,4 @@ +/* { dg-do compile { target { powerpc-*-* powerpcle-*-* } } } */ +/* { dg-options "-mcpu=603e -fsched-stalled-insns -fsched2-use-superblocks -fschedule-insns2 -fno-dce -fno-guess-branch-probability --param max-cse-insns=4" } */ + +#include "../../gcc.target/powerpc/ppc-switch-2.c" diff --git a/gcc/testsuite/gcc.c-torture/compile/pr88423.c b/gcc/testsuite/gcc.c-torture/compile/pr88423.c new file mode 100644 index 0000000..4948817 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr88423.c @@ -0,0 +1,5 @@ +/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */ +/* { dg-options "-march=skylake -fPIC -fsched2-use-superblocks -fno-if-conversion" } */ +/* { dg-require-effective-target fpic } */ + +#include "../../gcc.dg/20030309-1.c" -- 1.8.5.3