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 <a...@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