PR 27073: invalid gcse manipulation of REG_EQUIV notes
Sun Apr 9 09:55:00 GMT 2006
Roger Sayle <firstname.lastname@example.org> writes:
> On Sat, 8 Apr 2006, Richard Sandiford wrote:
>> PR rtl-optimization/27073
>> * gcse.c (try_replace_reg): Just propagate into REG_EQUAL notes,
>> not REG_EQUIVs.
> I agree with your analysis. This is OK for mainline.
> On the off chance that the potential performance problems you describe
> might materialize, there are several fallback alternatives to this PR.
I'm a little confused. I didn't mention any potential performance
problems. ;) That's because, as I understand it, there should be none.
The patch simply stops gcse applying a local equivalence to a REG_EQUIV
note. When we run gcse, the only REG_EQUIV notes should be for pseudos
that are equivalent to a stack parameter. (Indeed, several comments in
gcse.c already say this.) These REG_EQUIV notes start off containing
the expression we want them to contain.
And I imagine that's why the bug has lain unnoticed for so long.
It's not often that something in the parameter set-up sequence
can actually be propagated into the REG_EQUIV notes of later
parameter set-up instructions. Once you find such a situation,
it's very easy indeed to trip over the wrong-code problem I described.
> One alternative fix might be to disable just the problematic bit of
> try_replace_reg, i.e. to change:
>> /* If there is already a NOTE, update the expression in it with our
>> replacement. */
>> if (note != 0)
>> XEXP (note, 0) = simplify_replace_rtx (XEXP (note, 0), from, to);
> into some thing like:
> /* If there is already a REG_EQUAL note, update ... */
> if (note != 0 && REG_NOTE_KIND (note) == REG_EQUAL)
I don't understand. Why is this any different from what my
I got the impression from this and earlier that you thought I was
disabling try_replace_reg for insn _bodies_ if the insn had a
REG_EQUIV note. Not so. We still replace the insn body, just not
the REG_EQUIV note. (This is safe because gcse.c doesn't delete any
insns that set up FROM.)
> And another alternative, might be to convert this note from a
> REG_EQUIV into a REG_EQUAL [if the result isn't identical to the
> SET_SRC, and delete it otherwise].
I think that's a bad idea. We'd then create unnecessary spill slots
for stack parameters.
More information about the Gcc-patches