This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: 6 GCC regressions, 2 new, with your patch on 2002-07-20T23:26:42Z.


> 
> Hi Jan,
> > There are 2 new failures, and 4 failures that existed before and after
> > that patch; 0 failures have been fixed.
> >
> > The new failures are:
> > mips-elf gcc.sum gcc.c-torture/execute/921029-1.c
> > native gcc.sum gcc.dg/20010912-1.c
> 
> These look like they're yours.  I'm guessing its a bug in cselib or
> your new cprop code.  In 20010912-1.c, the routine foo is miscompiled:
> 
> >> int foo (int x, char *y)
> >> {
> >>   int a;
> >>   char *b = y;
> >>   a = bar (x, &y);
> >>   if (a)
> >>     {
> >>       y = b;
>          ^
> This assignment is incorrectly removed.  The call to "bar" changes
> the value of y, which isn't correctly noticed by cprop...

This is really interesting case.  The beggining of function contains:
(insn 4 3 6 0 (nil) (set (reg:SI 62)
        (mem/f:SI (plus:SI (reg/f:SI 16 argp)
                (const_int 4 [0x4])) [4 y+0 S4 A32])) 45 {*movsi_1}
(nil)
    (expr_list:REG_EQUIV (mem/f:SI (plus:SI (reg/f:SI 16 argp)
                (const_int 4 [0x4])) [4 y+0 S4 A32])
        (nil)))

And later also:

(insn 12 6 21 0 0x401a3a80 (set (reg/v/f:SI 64)
        (mem/f:SI (plus:SI (reg/f:SI 16 argp)
                (const_int 4 [0x4])) [4 S4 A8])) 45 {*movsi_1} (nil)
    (nil))

cselib notices the equivalence and cprop actually does the replacement
of reg 64 by reg 62.  But the REG_EQUIV note is not really valid, as the
value of memory copy y is not valid during the function (overwriten by
bar call).  Just before register allocation the load of register 62
appears to be moved just before the assignment. I am not sure who does
the movement but it is valid given the information about equivalency
with memory.

cse.c seems to contain code preventing such a substitution as gcse does,
so I am attaching simple patch that does the same for cprop and I will
install it as obvious as I am not quite sure whether I will be able to
read my email tomorrow.

Sun Jul 21 21:35:20 CEST 2002  Jan Hubicka  <jh@suse.cz>
	* gcse.c (do_local_cprop): Do not extend lifetimes of registers set by
	do_local_cprop.

Index: gcse.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/gcse.c,v
retrieving revision 1.210
diff -c -3 -p -r1.210 gcse.c
*** gcse.c	20 Jul 2002 22:56:05 -0000	1.210
--- gcse.c	21 Jul 2002 19:34:59 -0000
*************** do_local_cprop (x, insn, alter_jumps)
*** 4312,4320 ****
        for (l = val->locs; l; l = l->next)
  	{
  	  rtx this_rtx = l->loc;
  	  if (CONSTANT_P (this_rtx))
  	    newcnst = this_rtx;
! 	  if (REG_P (this_rtx) && REGNO (this_rtx) >= FIRST_PSEUDO_REGISTER)
  	    newreg = this_rtx;
  	}
        if (newcnst && constprop_register (insn, x, newcnst, alter_jumps))
--- 4312,4329 ----
        for (l = val->locs; l; l = l->next)
  	{
  	  rtx this_rtx = l->loc;
+ 	  rtx note;
+ 
  	  if (CONSTANT_P (this_rtx))
  	    newcnst = this_rtx;
! 	  if (REG_P (this_rtx) && REGNO (this_rtx) >= FIRST_PSEUDO_REGISTER
! 	      /* Don't copy propagate if it has attached REG_EQUIV note.
! 		 At this point this only function parameters should have
! 		 REG_EQUIV notes and if the argument slot is used somewhere
! 		 explicitly, it means address of parameter has been taken,
! 		 so we should not extend the lifetime of the pseudo.  */
! 	      && (!(note = find_reg_note (l->setting_insn, REG_EQUIV, NULL_RTX))
! 		  || GET_CODE (XEXP (note, 0)) != MEM))
  	    newreg = this_rtx;
  	}
        if (newcnst && constprop_register (insn, x, newcnst, alter_jumps))


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]