Re: [PATCH] Fix PR23821

[Jeff Law <law at redhat dot com>]

On 10/01/09 03:54, Richard Guenther wrote:
> On Wed, 30 Sep 2009, Richard Guenther wrote:
>
>    
> > On Wed, 30 Sep 2009, Jeff Law wrote:
> >
> >      
> > > On 09/30/09 09:31, Richard Guenther wrote:
> > >        
> > > > On Wed, 30 Sep 2009, Jeff Law wrote:
> > > >
> > > >
> > > >          
> > > > > On 09/30/09 06:33, Richard Guenther wrote:
> > > > >
> > > > >            
> > > > > > This tries to address PR23821 where propagation of equivalences can
> > > > > > cause SCEV analysis to fail and thus loop optimizations to not trigger.
> > > > > >
> > > > > > Two passes propagate this kind of equivalences, VRP and DOM.  For
> > > > > > VRP this patch simply disables doing so, for DOM we avoid propagating
> > > > > > equivalences into statements that look like induction variable
> > > > > > increments (something that is done in other places of DOM already).
> > > > > >
> > > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu.  I have
> > > > > > tested the effect on SPEC2000 and SPEC2006 and the effects are in
> > > > > > the noise.
> > > > > >
> > > > > > Richard.
> > > > > >
> > > > > > 2009-09-30  Richard Guenther<rguenther@suse.de>
> > > > > >
> > > > > > 	PR tree-optimization/23821
> > > > > > 	* tree-vrp.c (vrp_finalize): Do not perform copy propagation.
> > > > > > 	* tree-ssa-dom.c (cprop_operand): Do not propagate copies into
> > > > > > 	simple IV increments.
> > > > > >
> > > > > >
> > > > > >              
> > > > > The DOM bits are OK.  The VRP bits seem a little heavy handed.  Is there
> > > > > some
> > > > > reason why we can't be a little more selective in substitute_and_fold,
> > > > > using
> > > > > simple_iv_increment_p?
> > > > >
> > > > >            
> > > > Not in the way substitute_and_fold is structured.
> > > >          
> > > substitute_and_fold just walks every statement in every block and does
> > > replacements.     So ISTM we have substitute_and_fold or replace_uses_in check
> > > simple_iv_increment_p and if true, avoid substituting one SSA name with
> > > another in that specific case.
> > >
> > > What am I missing?
> > >        
> > Hm, right.  I was misremembering - doing it in propagate_value
> > is not possible, in replace_uses_in it is.  I'll revisit the patch.
> >      
> OTOH we only want to disable propagation from equivalences.  Patching
> replace_uses_in would also disable propagation from copyprop which
> is not what we want to do.
>    
Sigh.

> So I'd rather go with the original patch?
>    
Yea.  OK

jeff