Re: Deadly optimization bug (all gcc versions!)

[Bernd Schmidt <bernds@cygnus.co.uk>]

> 
> Well, this is the analysis, however I have no idea how to fix it correctly :-(.
> A local fix in if_then_else_cond() would be to compare the results of
> nonzero_bits(x), reg_nonzero_bits[REGNO(x)] and
> reg_last_set_nonzero_bits[REGNO(x)] and use the biggest one? But this may
> just hide other potential problems with get_last_value()?

I believe the code we need to fix is in get_last_value.
The situation is this (target i686-linux with gcc-2.95):

insn 11     : set (reg 24) [some value]
insn 13 [i1]: set (reg 25) [some value]
insn 16     : set (reg 24) (plus (reg 24) (const_int 1))
insn 18 [i2]: set (reg 27) (lshiftrt (reg 24) (const_int 1))
insn 20 [i3]: set (reg 26) (plus (reg 27) (reg 25))

We already substituted  i2 into i3 and got a (valid) newpat that contains
reg 24.  Then, we try to substitute i1 into the newpat.
subst_low_cuid gets set to INSN_CUID (i1).  reg_last_set[24] is insn 16 at
this point.  The problem is that get_last_value returns the SET_SRC of
insn 11, completely ignoring insn 16.

The patch below seems to fix it.

The whole block of code looks very suspicious, though.  I'm under the
impression it can't ever find a valid set.  If it finds an insn before
subst_low_cuid, shouldn't that one be pointed to by reg_last_set[regno]?
I've tried completely disabling the whole if statement, and FWIW the
optimization does not trigger once when compiling cc1.  If no one can show
that it can have an effect, I suggest ripping it out.
The block of code was introduced by Richard Kenner on Aug 2, 1992 (it's in
gcc-2.2.2-2.3.1.diff.gz).

Bernd

	* combine.c (get_last_value): Don't look for earlier sets if the last
	known set is somewhere in between the insns being combined.

Index: combine.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/combine.c,v
retrieving revision 1.68
diff -u -p -r1.68 combine.c
--- combine.c	1999/08/20 23:05:06	1.68
+++ combine.c	1999/08/24 13:59:03
@@ -10817,6 +10817,11 @@ get_last_value (x)
     {
       rtx insn, set;
 
+      /* We can't do anything if the value is set in between the insns we are
+	 processing.  */
+      if (INSN_CUID (reg_last_set[regno]) <= INSN_CUID (subst_insn))
+	return 0;
+
       /* We can not do anything useful in this case, because there is
 	 an instruction which is not on the insn chain.  */
       if (subst_prev_insn)