[Patch,AVR]: Fix PR49903

Hans-Peter Nilsson hp@bitrange.com
Sat Aug 13 13:26:00 GMT 2011


On Sat, 13 Aug 2011, Georg-Johann Lay wrote:
> Hans-Peter Nilsson schrieb:
> > A glance at AVR makes me think this should already be handled by
> > the NOTICE_UPDATE_CC machinery.  Any analysis why this doesn't
> > happen?  With the same test-case (at -Os) I don't see redundant
> > compares for cris-elf, for example.
>
> One reason is that branch insns set cc0 to "clobber" where it is
> actually "none".  I could not depict the rationale for this from
> the avr BE, presumably it's because of text peepholes that change
> branches or jump-over-one-insn skip optimizations.

Or gas relaxation of branches ...nope, not in current binutils.
(And with gcc keeping track of instruction lengths, that's an
obsolete feature for code generated by gcc.)

> Second reason is that avr has no GT/GTU and therefore reorg transforms

Sidenote: please don't refer to it as reorg.  Reorg is the
delay-slot handler beast living in reorg.c and resource.c (but
whose days are numbered, thankfully).  ITYM machine-dependent
reorg; md_reorg, avr_reorg.

>
>    cc0 = compare (Reg, Num)
>    if (cc0 > 0)
>      goto L2
>
> to
>
>    cc0 = compare (Reg, Num-1)
>    if (cc0 >= 0)
>      goto L2
>
> so that the comparisons are no more the same.

Sorry, I missed that (those).  It just looked like you added a
lot of code for something that already should be handled.

> Of cource, reorg could ommit the last optimization if there is
> a similar comparison beforehand.  But the hard part is not
> doing/skipping the optimization, the annoyance is detecting the
> right 4-insn instruction sequences.
>
> I also thought about extending genrecog et al. which currently
> can handle 3 types of things (RECOG, SPLIT, PEEPHOLE2) to a
> fourth one like INSN_SEQ so that one could write down the
> sequence as RTL instead of as brain-dead C (brain-dead in the
> way it must be written down, not in what it does)
> and use insn-recog, insn-extract etc. to analyse such sequences.

I'm not sure what you mean here, except it sounds like yet more
code.  If you meant to keep the compare and branch together,
then there's an option to keep it: define cbranchM as a
define_insn, not as define_expand.

brgds, H-P



More information about the Gcc-patches mailing list