| Message ID | f134d419-d7d8-b7e4-0e4c-0a4cccfedcea@ispras.ru |
|---|---|
| State | New |
| Headers | show |
On Mon, 14 Mar 2016, Andrey Belevantsev wrote: > We fail to find the proper seqno for the fresh bookkeeping copy in this PR. > The problem is that in get_seqno_by_preds we are iterating over bb from the > given insn backwards up to the first bb insn. We skip the initial insn when > iterating over bb, yet we should take seqno from it. > > The code in question originally didn't include bb head when iterating, and was > patched to do so in 2011. The patch was wrong and instead of including bb > head managed to exclude the original insn itself. By reading the original and > patched code I've convinced myself that the right fix will be to do what the > patch intended and include both the initial insn and the bb head in the > iteration. > > Ok for trunk? > > gcc/ > > 2016-03-14 Andrey Belevantsev <abel@ispras.ru> > > PR rtl-optimization/69032 > * sel-sched-ir.c (get_seqno_by_preds): Include both tmp and head when > looping backwards over basic block insns. "both 'insn' and 'head'" (not tmp). > diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c > index ec59280..c1a9e55 100644 > --- a/gcc/sel-sched-ir.c > +++ b/gcc/sel-sched-ir.c > @@ -4103,11 +4103,14 @@ get_seqno_by_preds (rtx_insn *insn) > insn_t *preds; > int n, i, seqno; > > - while (tmp != head) > + /* Loop backwards from insn to head including both. */ "from INSN to HEAD" (uppercase). The following structure is equivalent, but would look a bit more canonical: for (rtx_insn *tmp = insn; ; tmp = PREV_INSN (tmp)) { if (INSN_P (tmp)) return INSN_SEQNO (tmp); if (tmp == head) break; } > + while (1) > { > - tmp = PREV_INSN (tmp); > if (INSN_P (tmp)) > return INSN_SEQNO (tmp); > + if (tmp == head) > + break; > + tmp = PREV_INSN (tmp); > } > > cfg_preds (bb, &preds, &n); OK with formatting nits fixed ('while'/'for' spelling change at your choice). Thanks. Alexander
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c index ec59280..c1a9e55 100644 --- a/gcc/sel-sched-ir.c +++ b/gcc/sel-sched-ir.c @@ -4103,11 +4103,14 @@ get_seqno_by_preds (rtx_insn *insn) insn_t *preds; int n, i, seqno; - while (tmp != head) + /* Loop backwards from insn to head including both. */ + while (1) { - tmp = PREV_INSN (tmp); if (INSN_P (tmp)) return INSN_SEQNO (tmp); + if (tmp == head) + break; + tmp = PREV_INSN (tmp); } cfg_preds (bb, &preds, &n); diff --git a/gcc/testsuite/gcc.dg/pr69032.c b/gcc/testsuite/gcc.dg/pr69032.c new file mode 100644 index 0000000..e0925cd --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr69032.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */ +/* { dg-options "-O2 -fsched-pressure -fsel-sched-pipelining -fselective-scheduling" } */ + +void foo (long long i) +{ + while (i != -1) + { + ++i; + __asm__ (""); + } +}
Hello, We fail to find the proper seqno for the fresh bookkeeping copy in this PR. The problem is that in get_seqno_by_preds we are iterating over bb from the given insn backwards up to the first bb insn. We skip the initial insn when iterating over bb, yet we should take seqno from it. The code in question originally didn't include bb head when iterating, and was patched to do so in 2011. The patch was wrong and instead of including bb head managed to exclude the original insn itself. By reading the original and patched code I've convinced myself that the right fix will be to do what the patch intended and include both the initial insn and the bb head in the iteration. Ok for trunk? gcc/ 2016-03-14 Andrey Belevantsev <abel@ispras.ru> PR rtl-optimization/69032 * sel-sched-ir.c (get_seqno_by_preds): Include both tmp and head when looping backwards over basic block insns. testsuite/ 2016-03-14 Andrey Belevantsev <abel@ispras.ru> PR rtl-optimization/69032 * gcc.dg/pr69032.c: New test. Best, Andrey