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

[Geoff Keating <geoffk at geoffk dot org>]

> Mailing-List: contact gcc-regression-help@gcc.gnu.org; run by ezmlm
> list-help: <mailto:gcc-regression-help@gcc.gnu.org>
> list-unsubscribe: <mailto:gcc-regression-unsubscribe@gcc.gnu.org>
> list-post: <mailto:gcc-regression@gcc.gnu.org>
> Date: Mon, 22 Jul 2002 15:39:57 +0200
> From: Jan Hubicka <jh@suse.cz>
> Cc: jh@suse.cz, gcc-regression@gcc.gnu.org, amodra@bigpond.net.au, bo@suse.de,
>    rth@cygnus.com, gcc-patches@gcc.gnu.org
> Content-Disposition: inline
> User-Agent: Mutt/1.3.28i
> 
> > > 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.

The peephole code is supposed to look at the data flow, so it
shouldn't matter how many instructions are between the setter and user.

> 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).

I don't think your patch represents a good design.  If it's important
to keep hard register lifetimes short, then that problem should be
solved in a general way, not by having special code for just one case
in just one of the many routines that insert insns.

> > This code really doesn't feel right to me.  There could be many
> > unexpected consequences.

For instance, you used single_set, but I think that's wrong in this
case, because a CLOBBER might mean the instruction has a conflict with
the insn you're moving over.

I think you also don't check that the basic block actually has more
than one insn.  What happens if first instruction in a program is a
(conditional?) jump?

-- 
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>