Re: regression tester mail...

Re: regression tester mail...

[Jan Hubicka]

> > 
> > mips-elf gcc.sum gcc.c-torture/execute/921029-1.c
> > 
> > is this:
> > 
> > +Sun Jul 21 00:54:54 CEST 2002  Jan Hubicka  <jh@suse.cz>
> > +
> > +       * gcse.c: Include cselib.h
> > +       (constptop_register): Break out from ...
> > +       (cprop_insn): ... here; kill basic_block argument.
> > +       (do_local_cprop, local_cprop_pass): New functions.
> > +       (one_cprop_pass): Call local_cprop_pass.
> > +
> Hi,
> this bug is not handled by the other three patches I've sent and
> exposses another latent problem, this time in cprop_jump.
> 
> The problem is sequence like:
> (set (reg A) (xor (reg A) (reg B)))
> (set (pc) (if_then_else (eq (reg A) (0))) ... )
> 
> Now the cprop_jump proceeds by substituing the first instruction into
> the conditional and it notices reg B to be 0.  The result of
> simplification is (of course) the ellimination of xor and exactly same
> conditional as previously.  Then it validates the replacement of the
> conditional for it's equivalent and is happy.
> 
> This uncovers thinko - when the setcc instruction modifies it's operand,
> it is of course invalid to recompute it afterwards again and expect the
> same result.  The attached patch fixes the problem and also adds extra
> check to avoid substituting value for the same and infinite loop.
> 
> Checked to fix the mips problem and regtested/bootstrapped i386.
> In case this patch is OK, can you please apply it as well?
> 
> Honza
> 
> Fri Jul 26 14:34:49 CEST 2002  Jan Hubicka  <jh@suse.cz>
> 	* gcse.c (cprop_jump): Avoid false positives; verify that the
> 	transformation is valid.

Uhm, there is another bug in the code.  The copy propagation is doing
the work as for the jump.  In the between of setcc and jump there may be
other instructions copying registers around, so we need to verify that
the substitution suggested by copy propagation is valid at the setcc
execution point as well.

Note that the rtx_equal_p test is now probably redundant.  I can't think
of situation where simplification will result in exactly same expression
as previously and the other checks will pass (an don't have any testcase
of course), but it is probably better to be safe then sorry.

Restarting bootstrapping/regtesting with the updated patch.

Honza

Index: gcse.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/gcse.c,v
retrieving revision 1.215
diff -c -3 -p -r1.215 gcse.c
*** gcse.c	25 Jul 2002 18:57:30 -0000	1.215
--- gcse.c	26 Jul 2002 12:54:26 -0000
*************** cprop_jump (bb, setcc, jump, from, src)
*** 4093,4099 ****
  
    /* First substitute in the INSN condition as the SET_SRC of the JUMP,
       then substitute that given values in this expanded JUMP.  */
!   if (setcc != NULL)
      {
        rtx setcc_set = single_set (setcc);
        new_set = simplify_replace_rtx (SET_SRC (set),
--- 4093,4101 ----
  
    /* First substitute in the INSN condition as the SET_SRC of the JUMP,
       then substitute that given values in this expanded JUMP.  */
!   if (setcc != NULL
!       && !modified_between_p (from, setcc, jump)
!       && !modified_between_p (src, setcc, jump))
      {
        rtx setcc_set = single_set (setcc);
        new_set = simplify_replace_rtx (SET_SRC (set),
*************** cprop_jump (bb, setcc, jump, from, src)
*** 4107,4113 ****
  
    /* If no simplification can be made, then try the next
       register.  */
!   if (rtx_equal_p (new, new_set))
      return 0;
  
    /* If this is now a no-op delete it, otherwise this must be a valid insn.  */
--- 4109,4115 ----
  
    /* If no simplification can be made, then try the next
       register.  */
!   if (rtx_equal_p (new, new_set) || rtx_equal_p (new, SET_SRC (set)))
      return 0;
  
    /* If this is now a no-op delete it, otherwise this must be a valid insn.  */
*************** cprop_jump (bb, setcc, jump, from, src)
*** 4115,4120 ****
--- 4117,4127 ----
      delete_insn (jump);
    else
      {
+       /* Ensure the value computed inside the jump insn to be equivalent
+          to one computed by setcc.  */
+       if (setcc 
+ 	  && modified_in_p (new, setcc))
+ 	return 0;
        if (! validate_change (jump, &SET_SRC (set), new, 0))
  	return 0;