> Ah, OK, yea, it makes perfect sense now. Approved.
Thanks.
> If you wanted to get ambitious, rewriting reorg.c to compute and use
> dependency links to drive its candidate selection instead of the insane
> tracking code it uses would be a huge step forward, both in terms of
> cleanliness. It'd also help compile-time performance; we do a lot of
> work to track resources that would be totally unnecessary.
Yes, I agree that it's quite convoluted but, as Steven already said, rewriting
it to use a more modern framework is hard because of SEQUENCEs and, frankly, I
have neither the real incentive nor the time to start such an endeavor.
Instead I can raise the following couple of points that I also ran into with
the private port I'm working on:
1. When fill_simple_delay_slots is scanning backwards to find insns to put
into slots, it calls update_reg_dead_note which collects REG_DEAD notes and
puts them on the insn to be moved. But emit_delay_sequence will later drop
them on the floor. I have one case where preserving such a REG_DEAD note is
correct and makes a difference (slot filled vs empty).
2. In resource.c, when mark_target_live_regs is doing its liveness analysis,
it extracts the insns wrapped in USEs by reorg.c:
/* If this insn is a USE made by update_block, we care about the
underlying insn. */
if (code == INSN && GET_CODE (PATTERN (insn)) == USE
&& INSN_P (XEXP (PATTERN (insn), 0)))
real_insn = XEXP (PATTERN (insn), 0);
and proceeds with the liveness analysis without further ado. Now, the story
is different in find_dead_or_set_registers, which does:
if (GET_CODE (PATTERN (insn)) == USE)
{
/* If INSN is a USE made by update_block, we care about the
underlying insn. Any registers set by the underlying insn
are live since the insn is being done somewhere else. */
if (INSN_P (XEXP (PATTERN (insn), 0)))
mark_set_resources (XEXP (PATTERN (insn), 0), res, 0,
MARK_SRC_DEST_CALL);
and thus pessimizes the analysis by making registers unnecessary live. Again
I have one case where not pessimizing is correct and makes a difference.
Experimental patch attached, clean on the C testsuite for the private port.
What do you think?
--
Eric BotcazouIndex: reorg.c
===================================================================
--- reorg.c (revision 197617)
+++ reorg.c (working copy)
@@ -549,6 +549,11 @@ emit_delay_sequence (rtx insn, rtx list,
remove_note (tem, note);
break;
+ case REG_DEP_TRUE:
+ /* This was a REG_DEAD note that we want to preserve. */
+ PUT_REG_NOTE_KIND (note, REG_DEAD);
+ break;
+
case REG_LABEL_OPERAND:
case REG_LABEL_TARGET:
/* Keep the label reference count up to date. */
@@ -1806,8 +1811,10 @@ update_reg_dead_notes (rtx insn, rtx del
if (reg_referenced_p (XEXP (link, 0), PATTERN (insn)))
{
- /* Move the REG_DEAD note from P to INSN. */
+ /* Move the REG_DEAD note from P to INSN but smuggle it so that
+ emit_delay_sequence doesn't drop it on the floor. */
remove_note (p, link);
+ PUT_REG_NOTE_KIND (link, REG_DEP_TRUE);
XEXP (link, 1) = REG_NOTES (insn);
REG_NOTES (insn) = link;
}
Index: resource.c
===================================================================
--- resource.c (revision 197617)
+++ resource.c (working copy)
@@ -425,6 +425,7 @@ find_dead_or_set_registers (rtx target,
for (insn = target; insn; insn = next)
{
rtx this_jump_insn = insn;
+ rtx real_insn = insn;
next = NEXT_INSN (insn);
@@ -443,7 +444,6 @@ find_dead_or_set_registers (rtx target,
AND_COMPL_HARD_REG_SET (pending_dead_regs, needed.regs);
AND_COMPL_HARD_REG_SET (res->regs, pending_dead_regs);
CLEAR_HARD_REG_SET (pending_dead_regs);
-
continue;
case BARRIER:
@@ -454,14 +454,12 @@ find_dead_or_set_registers (rtx target,
if (GET_CODE (PATTERN (insn)) == USE)
{
/* If INSN is a USE made by update_block, we care about the
- underlying insn. Any registers set by the underlying insn
- are live since the insn is being done somewhere else. */
+ underlying insn. */
if (INSN_P (XEXP (PATTERN (insn), 0)))
- mark_set_resources (XEXP (PATTERN (insn), 0), res, 0,
- MARK_SRC_DEST_CALL);
-
- /* All other USE insns are to be ignored. */
- continue;
+ real_insn = XEXP (PATTERN (insn), 0);
+ else
+ /* All other USE insns are to be ignored. */
+ continue;
}
else if (GET_CODE (PATTERN (insn)) == CLOBBER)
continue;
@@ -582,8 +580,8 @@ find_dead_or_set_registers (rtx target,
}
}
- mark_referenced_resources (insn, &needed, true);
- mark_set_resources (insn, &set, 0, MARK_SRC_DEST_CALL);
+ mark_referenced_resources (real_insn, &needed, true);
+ mark_set_resources (real_insn, &set, 0, MARK_SRC_DEST_CALL);
COPY_HARD_REG_SET (scratch, set.regs);
AND_COMPL_HARD_REG_SET (scratch, needed.regs);
