This is a severe, but very rare, bug in combine, that one of the recent
changes have exposed. I don't know which.
Basically, combine will not try combining across call instructions, but
on the other hand it will not invalidate equivalences with registers
that are explicitly set in the call instruction. This patch teaches it
about this case.
Also, record_value_for_reg had a comment saying
"If INSN is zero, don't update reg_stat[].last_set; this is
only permitted with VALUE also zero and is used to invalidate the
register"
However, this feature was never used, and as implemented it was not
sufficient to fix the bug. So, I made combine set "last_set_invalid"
too when INSN == VALUE == NULL_RTX.
Combine already has a loop that tries to invalidate call-clobbered
register. The logic is the same as calling record_value_for_reg
*before* my bug fix. However I did not change this because I was not
sure if it was necessary -- and anyway, call-clobbered hard regs should
only be present in combine if the user is using the "register ... asm"
extension. In this case, the user cannot rely much on their semantics
across function calls if they specify call-clobbered registers.
This patch was bootstrapped on powerpc-apple-darwin8.3.0, and H-P
Nilsson tested on cris-axis-elf that it causes no assembly code
generation difference on the testsuite (except for fixing the bug!) and
CSiBE.
Ok for mainline?
2005-01-07 Paolo Bonzini <[EMAIL PROTECTED]>
* combine.c (record_value_for_reg): Invalidate registers
if INSN is null.
(record_dead_and_set_regs_1): Likewise, by passing them to
record_value_for_reg.
(record_dead_and_set_regs): Invalidate stores made by a
call. We do not combine across a call, but we still
relied on equivalences.
Index: combine.c
===================================================================
--- combine.c (revision 109374)
+++ combine.c (working copy)
@@ -10823,7 +10826,7 @@ record_value_for_reg (rtx reg, rtx insn,
for (i = regno; i < endregno; i++)
{
reg_stat[i].last_set_label = label_tick;
- if (value && reg_stat[i].last_set_table_tick == label_tick)
+ if (!insn || (value && reg_stat[i].last_set_table_tick == label_tick))
reg_stat[i].last_set_invalid = 1;
else
reg_stat[i].last_set_invalid = 0;
@@ -10872,6 +10874,13 @@ record_dead_and_set_regs_1 (rtx dest, rt
if (GET_CODE (dest) == SUBREG)
dest = SUBREG_REG (dest);
+ if (!record_dead_insn)
+ {
+ if (REG_P (dest))
+ record_value_for_reg (dest, NULL_RTX, NULL_RTX);
+ return;
+ }
+
if (REG_P (dest))
{
/* If we are setting the whole register, we know its value. Otherwise
@@ -10944,15 +10953,14 @@ record_dead_and_set_regs (rtx insn)
last_call_cuid = mem_last_set = INSN_CUID (insn);
- /* Don't bother recording what this insn does. It might set the
- return value register, but we can't combine into a call
- pattern anyway, so there's no point trying (and it may cause
- a crash, if e.g. we wind up asking for last_set_value of a
- SUBREG of the return value register). */
- return;
+ /* We can't combine into a call pattern. Remember, though, that
+ the return value register is set at this CUID. We could
+ still replace a register with the return value from the
+ wrong subroutine call! */
+ note_stores (PATTERN (insn), record_dead_and_set_regs_1, NULL_RTX);
}
-
- note_stores (PATTERN (insn), record_dead_and_set_regs_1, insn);
+ else
+ note_stores (PATTERN (insn), record_dead_and_set_regs_1, insn);
}
/* If a SUBREG has the promoted bit set, it is in fact a property of the
