Re: 4 GCC regressions, 2 new, with your patch on 2002-07-18T09:56:17Z.

[Jan Hubicka <jh at suse dot cz>]

> > Date: Mon, 22 Jul 2002 03:05:58 +0200
> > From: Jan Hubicka <jh@suse.cz>
> 
> > > > With your recent patch, GCC has some regression test failures, which
> > > > used to pass.  There are 2 new failures, and 2
> > > > failures that existed before and after that patch; 0 failures
> > > > have been fixed.
> > > > 
> > > > The new failures are:
> > > > powerpc-eabisim gcc.sum gcc.dg/20010822-1.c
> > > > native gcc.sum gcc.dg/20010822-1.c
> > > 
> > > I guess I would have to conclude that these are mines however strange it
> > > looks.  I will check what is going on.
> > 
> > Hi,
> > this is another interesting problem. What happends is that code hoisting
> > (insert_insn_end_bb) inserts load in between flags set and conditional
> > jump instruction. This is valid, since load does not clobber flags, but
> > not very sane as we lose optimizaiton opurtunities.
> 
> We really shouldn't be losing any optimisation opportunities by doing
> this; if we do, it probably means that some piece of code is looking
> at the RTL sequence instead of using def/use chains or whatever.  What
> goes wrong?

We do lose since often peepholes are asking for flags to do something
smart (like using xor for loading zero). Whenever we place unnecesary
code in between flags setter and user we lose.
I think we simply should not extend liveness of hard registers when it
is easy to go without (ie we do not lose the optimization).
> 
> This code really doesn't feel right to me.  There could be many
> unexpected consequences.
> 
> > However, the code gets wrong in ifcvt as ifcvt insert new code before
> > the flags set and uses uninitialized register.
> 
> Why does the code in ifcvt think this is valid?

The code is wrong as well I believe.  It should verify whether the code
in between earliest and jump change the operands used by to be inserted
instruction. This is bit tricky to do, I plan to do in separate patch.
> 
> > This patch fixes the gcse part by avoiding code hoisting to insert
> > instructions as such a crazy places.  I believe this is good thing to
> > do, but still I would like to know what to do with ifcvt.
> 
> I think it would be best to fix ifcvt.

We need both IMO.

Honza
> 
> > Regtested bootstrapped mainline.  I am now rerunning bootstrap with -Os.
> > OK for mainline?
> 
> I don't think this code is a good idea.
> 
> > Mon Jul 22 03:02:47 CEST 2002  Jan Hubicka  <jh@suse.cz>
> > 	* gcse.c (insert_insn_end_bb): Do not insert in between flags setter
> > 	and user.
> -- 
> - Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>