Re: GCC 3.3

[Mark Mitchell <mark at codesourcery dot com>]

On Tue, 2003-04-29 at 07:02, Jan Hubicka wrote:
> > Mark Mitchell wrote:
> > 
> > >If you know of a regression for which you think we should hold the
> > >release:
> > 
> > There is a serious regression on s390 (Linux kernel silently
> > miscompiled).  See optimization/10536.
> > 
> > This is a regression over 3.2, introduced by this patch:
> > http://gcc.gnu.org/ml/gcc-patches/2003-03/msg01070.html
> > 
> > It can be fixed either by reverting that patch, or else by this fix:
> > http://gcc.gnu.org/ml/gcc-patches/2003-04/msg01318.html
> > (which has not yet been approved).
> 
> Mark,
> reverting of the patch is safe as the patch is purely compilation time
> optimization.  I also tested the proposer proper fix relatively
> troroughly in SuSE's internal compiler and it appears to cause no
> problems as well. 

I don't understand your new patch:

! 	  /* Do not delete instructions initializing operands needed by dead
! 	     instruction with side effects.  */
! 	  || (!counts[REGNO (x)] && incr > 0
! 	      && insn_live_p (insn, NULL)))
 
First, INSN is live -- not dead, given that insn_live_p holds.

Second, no deletion is taking place here.
  
Third, checking incr > 0 is pointless, since all that happens if the
condition is true is:

  counts[REGNO (x)] += incr;

Fourth, why check !counts[REGNO (x)]?

I think you want something more like:

  /* If this instruction is live, then even modifications to the  
destination register matter. */
  || insn_live_p (insn, NULL)

But then, the comment above the entire function needs to change, since
it explicitly says that modifications to the destination register don't
matter.

In light of this, please revert your original patch on the 3.3 branch,
but proceed with fixing it on the mainline.  We can consider this
optimization for 3.3.1 if it is stable by then.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark at codesourcery dot com