Re: [patch, avr] Take 2: Fix PR64331: insn output and insn length computation rely on REG_DEAD notes.

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

On Thu, Feb 26, 2015 at 8:35 PM, Georg-Johann Lay wrote:
> Take #2 introduces a new, avr-specific rtl pass whose sole purpose is to
> rectify notes.  The pass is scheduled right before cfg does down (right
> before .*free_cfg) so that cfg and hence df machinery is available.
>
> Regression tests look fine and for the test case the patches produce correct
> code and correct insn length.

Sorry for party-pooping, but it seems to me that the real bug is that
the target is lying to reload.

IIUC the AVR port selects the insn alternative after register
allocation (after reload). It bases its selection on REG_DEAD notes.
In PR64331 an alternative is used that clobbers r20 that has a
REG_DEAD note, but r20 is not actually dead because hardreg-cprop has
propagated it forward without adjusting the note.

The "normal" way of things is that the insn alternative is selected in
reload (or in LRA) and that the clobbers are added as necessary. In
PR64331, an alternative for insn r17 would be selected that has a
CLOBBER for r20, prevent hardreg-cprop from propagating r20.

Selecting insns based on REG-notes is dangerous business. Lying to
reload and to post-RA passes is a mortal sin, the compiler will punish
you. There is no guarantee that nothing will change between your new
pass to recompute notes, and the final pass that emits the insns.

It's not my port, for sure, but I would look for a real fix instead:
Don't select insns to output based on unreliable information like
REG-notes.

Ciao!
Steven