diff mbox

Make IPA-CP work on aggregates

Message ID 20121107143915.GB24044@virgil.arch.suse.de
State New
Headers show

Commit Message

Martin Jambor Nov. 7, 2012, 2:39 p.m. UTC
On Tue, Nov 06, 2012 at 02:35:30PM +0100, Jakub Jelinek wrote:
> On Tue, Nov 06, 2012 at 12:58:07AM +0100, Martin Jambor wrote:
> > 2012-11-05  Martin Jambor  <mjambor@suse.cz>
> > 
> > 	PR tree-optimization/53787
> > 	* ipa-cp.c (ipcp_value_source): New field offset.
> ...
> 
> Is this supposed to do something about Fortran array descriptors
> where some fields in the descriptors have known constant values in the
> caller?
> 
> Say
> subroutine bar (a, b, n)
>   integer :: a(n), b(n)
>   call foo (a, b)
> contains
> subroutine foo (a, b)
>   integer :: a(:), b(:)
>   a = b
> end subroutine
> end
> -O2 -fno-inline (there could be thousands of better testcases though, this
> one doesn't look at too many fields).
> With your patch
> foo.1899.constprop.0 is created, but I don't see any immediate other
> effects.  Certainly e.g.
>   _2 = a_1(D)->dim[0].stride;
>   if (_2 != 0)
> remains till *.optimized dump, even when in the caller it is set to 1.
> I guess for Fortran being able to optimize on constant (or even better
> constant one) stride would be very worthwhile.
> 

Oh... but that is not a due to a bug in this patch but due to an
unnecessarily strict bail out condition when building the jump
functions (code that went in in July), a thinko really.  The following
patch fixes it.  So far it is untested but I'll give it a go when
another bootstrap finishes.  I'm not sure if it would be OK to commit
it now, given it is stage3, though.  OTOH, I think it would be worth
it.

Thanks,

Martin


2012-11-07  Martin Jambor  <mjambor@suse.cz>

	* ipa-prop.c (determine_known_aggregate_parts): Skip writes to
	different declarations when tracking writes to a declaration.

Comments

Jakub Jelinek Nov. 7, 2012, 2:41 p.m. UTC | #1
On Wed, Nov 07, 2012 at 03:39:15PM +0100, Martin Jambor wrote:
> another bootstrap finishes.  I'm not sure if it would be OK to commit
> it now, given it is stage3, though.  OTOH, I think it would be worth
> it.

I'm ok with getting that in now from RM POV, but not familiar with the
code enough to review it.  So, if somebody acks it (Honza?), it can be
added.

> 2012-11-07  Martin Jambor  <mjambor@suse.cz>
> 
> 	* ipa-prop.c (determine_known_aggregate_parts): Skip writes to
> 	different declarations when tracking writes to a declaration.
> 
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -1318,7 +1318,12 @@ determine_known_aggregate_parts (gimple
>  	    break;
>  	}
>        else if (lhs_base != arg_base)
> -	break;
> +	{
> +	  if (DECL_P (lhs_base))
> +	    continue;
> +	  else
> +	    break;
> +	}
>  
>        if (lhs_offset + lhs_size < arg_offset
>  	  || lhs_offset >= (arg_offset + arg_size))

	Jakub
Jan Hubicka Nov. 7, 2012, 2:55 p.m. UTC | #2
> On Wed, Nov 07, 2012 at 03:39:15PM +0100, Martin Jambor wrote:
> > another bootstrap finishes.  I'm not sure if it would be OK to commit
> > it now, given it is stage3, though.  OTOH, I think it would be worth
> > it.
> 
> I'm ok with getting that in now from RM POV, but not familiar with the
> code enough to review it.  So, if somebody acks it (Honza?), it can be
> added.
> 
> > 2012-11-07  Martin Jambor  <mjambor@suse.cz>
> > 
> > 	* ipa-prop.c (determine_known_aggregate_parts): Skip writes to
> > 	different declarations when tracking writes to a declaration.
> > 
> > Index: src/gcc/ipa-prop.c
> > ===================================================================
> > --- src.orig/gcc/ipa-prop.c
> > +++ src/gcc/ipa-prop.c
> > @@ -1318,7 +1318,12 @@ determine_known_aggregate_parts (gimple
> >  	    break;
> >  	}
> >        else if (lhs_base != arg_base)
> > -	break;
> > +	{
> > +	  if (DECL_P (lhs_base))
> > +	    continue;
> > +	  else
> > +	    break;
> > +	}

OK, so the point of patch is to not stop on writes to decls while looking
for value the field is initialized to?

It looks OK.
Thanks,
Honza
> >  
> >        if (lhs_offset + lhs_size < arg_offset
> >  	  || lhs_offset >= (arg_offset + arg_size))
> 
> 	Jakub
diff mbox

Patch

Index: src/gcc/ipa-prop.c
===================================================================
--- src.orig/gcc/ipa-prop.c
+++ src/gcc/ipa-prop.c
@@ -1318,7 +1318,12 @@  determine_known_aggregate_parts (gimple
 	    break;
 	}
       else if (lhs_base != arg_base)
-	break;
+	{
+	  if (DECL_P (lhs_base))
+	    continue;
+	  else
+	    break;
+	}
 
       if (lhs_offset + lhs_size < arg_offset
 	  || lhs_offset >= (arg_offset + arg_size))