2015-03-02 15:32 GMT+03:00 Georg-Johann Lay <a...@gjlay.de>: > Am 02/28/2015 um 09:02 AM schrieb Denis Chertykov: > >> 2015-02-27 1:45 GMT+03:00 Steven Bosscher <stevenb....@gmail.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. >> >> >> Steven rights. >> We will have an endless fight with this problem. > > > Just consider the patch as prerequisite for further changes atop of it as > outlined in > > https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01745.html > > The current patches are operating correctly and do nothing wrong. Complete > rewriting of reg_unused_after will be much more work and more error prone > (e.g. unrecognizable insn, optimization flaws). > > >> Better to completely drop `reg_unused_after'. (I know that it used >> around 40 times in port) >> >> What do you think Georg ? > > > I'd prefer to fix it, c.f. link above.
Ok. Please fix it. Denis. PS: As I remember: I have copyed `reg_unused_after' from sparc port. It was around 10 years ago. `reg_unused_after' was removed from sparc a few years ago. So, only avr and sh ports use `reg_unused_after'. It's dangerous because GCC core developers don't bothered about two small ports. (Ports with relatively small user base) At least we can remove `reg_unused_after' in any time. (It's relatively easy.)
