PR 27073: invalid gcse manipulation of REG_EQUIV notes

Richard Sandiford richard@codesourcery.com
Mon Apr 10 06:44:00 GMT 2006


Roger Sayle <roger@eyesopen.com> writes:
> On Sun, 9 Apr 2006, Richard Sandiford wrote:
>> I didn't mention any potential performance problems. ;)
>
> My apologies, I'm one of those paranoid folks who immediately equate
> changes in register allocation with potential performance problems;
> you did write:
>
>>> this is bound to be a regression for _some_ testcase, due to
>>> differences in register allocation.

I meant: the bug is bound to be a _wrong code_ regression
from some testcase.  The quote was from a paragraph about
whether the fix should be backported to release branches:

    (2) above means that this is bound to be a regression for _some_
    testcase, due to differences in register allocation.  I just don't
    know which testcase.  I'll leave it to others to decide whether
    that's sufficient grounds for applying it to 4.1. ;)

>> I don't understand.  Why is this any different from what my
>> patch does?
>
> The difference cconcerns this test from later in try_replace_reg:
>
>       if (!success && note == 0 && set != 0
>
> Previously for instructions containing REG_EQUIV notes, note would
> be non-zero, however it now indicates whether we had a REG_EQUAL
> note.  This will now lead to situations where our instruction will
> have both REG_EQUAL and REG_EQUIV notes, as GCSE may now blindly
> add a REG_EQUAL note if the substitution isn't a valid instruction.
> This may or may not confuse later passes, for example the function
> find_reg_equal_equiv_note assumes there's only or other note, and
> reload changes notes from one form to another, which will result
> in multiple notes.

OK, if that's what bothers you, I'll change it.  Patch to come.

> The potential performance impact was that previously we were making
> unsafe unsubstitutions in REG_EQUIV notes, but in some circumstances
> they may have been beneficial.  For example, allowing us to eliminate
> all references to a hard frame pointer.  Whilst I agree that your
> patch is a correctness fix (nothing guarantees the replacement is
> valid throughout the function), there may exists codes that have
> previously benefited from their luck.  If they never spill the
> stack parameter, they'll never encounter problems.  Either way,
> the compiler may now be generating different code on different
> targets.

I think I answered that point yesterday.

Richard



More information about the Gcc-patches mailing list