http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46614
--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2010-11-23
14:47:43 UTC ---
So, the bug is that we have two kinds of INSNs in
deps->last_pending_memory_flush
1) jumps added there via:
deps->last_pending_memory_flush
= alloc_INSN_LIST (insn, deps->last_pending_memory_flush);
in deps_analyze_insn
2) any instructions added there via:
deps->last_pending_memory_flush = alloc_INSN_LIST (insn, NULL_RTX);
in flush_pending_lists
This list is used to add dependencies in flush_pending_lists (correct),
sched_analyze_1 (for writes, also correct, and one correct for write spot in
sched_analyze_insn) and then in sched_analyze_2 and sched_analyze_insn for
reads/DEBUG_INSNs, where it differentiates between 1) and 2) above using JUMP_P
test on the insn in the last_pending_memory_flush list. E.g. in
sched_analyze_2
it adds deps always for the (assumed) category 2) and only adds something for
category 1) if it the memory can trap. The problem is that JUMP_P is not a
good
test between 1) and 2), as flush_pending_lists can be also called on JUMP_Ps:
/* Flush pending lists on jumps, but not on speculative checks. */
if (JUMP_P (insn) && !(sel_sched_p ()
&& sel_insn_is_speculation_check (insn)))
flush_pending_lists (deps, insn, true, true);
and (what happens here):
if (!deps->readonly
&& JUMP_P (insn)
&& !(sel_sched_p ()
&& sel_insn_is_speculation_check (insn)))
{
/* Keep the list a reasonable size. */
if (deps->pending_flush_length++ > MAX_PENDING_LIST_LENGTH)
flush_pending_lists (deps, insn, true, true);
else
deps->last_pending_memory_flush
= alloc_INSN_LIST (insn, deps->last_pending_memory_flush);
}
What happens on the testcase is that pending_flush_length gets too large and
so we flush_pending_lists and flushes everything, adding needed dependencies
and sets deps->last_pending_memory_flush to contain just that jump.
A few insns later we process a read and go through last_pending_memory_flush,
see it only contains a JUMP_P and that the memory can't trap and don't add any
dependency. But in this case, because the jump was added as result of
flush_pending_lists and thus represents all the memory writes before it as
well,
it is wrong, we need to add a dependency.
I guess we need to keep the information whether a jump was added as part of
flush_pending_lists or for 1) somewhere, wonder if mode of the INSN_LIST
wouldn't be usable in this case for it.
