Re: PR 27073: invalid gcse manipulation of REG_EQUIV notes

[Richard Sandiford <richard at codesourcery dot com>]

Roger Sayle <roger@eyesopen.com> 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.

Thanks.

> 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
patch does?

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.

Richard