Patchwork Make IPA-CP work on aggregates

login
register
mail settings
Submitter Martin Jambor
Date Nov. 8, 2012, 2:52 p.m.
Message ID <20121108145229.GD24044@virgil.arch.suse.de>
Download mbox | patch
Permalink /patch/197846/
State New
Headers show

Comments

Martin Jambor - Nov. 8, 2012, 2:52 p.m.
Hi,

On Wed, Nov 07, 2012 at 03:55:16PM +0100, Jan Hubicka wrote:
> > 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.

Yes, when we are looking for writes to a decl and come accross a write
to another decl, we just skip it instead of bailing out completely.
The reason my original implementation missed it is that I relied on
stmt_may_clobber_ref_p_1 to determine it could not clobber the decl
but apprently it does not... perhaps that is a bug?

Anyway, I added a testcase to the above and this is what I am about to
commit.

Thanks a lot,

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.

	* gfortran.dg/ipcp-array-1.f90: New test.
Jan Hubicka - Nov. 8, 2012, 4:41 p.m.
> Hi,
> 
> On Wed, Nov 07, 2012 at 03:55:16PM +0100, Jan Hubicka wrote:
> > > 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.
> 
> Yes, when we are looking for writes to a decl and come accross a write
> to another decl, we just skip it instead of bailing out completely.
> The reason my original implementation missed it is that I relied on
> stmt_may_clobber_ref_p_1 to determine it could not clobber the decl
> but apprently it does not... perhaps that is a bug?
> 
> Anyway, I added a testcase to the above and this is what I am about to
> commit.
> 
> Thanks a lot,
> 
> 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.
> 
> 	* gfortran.dg/ipcp-array-1.f90: New test.

Thanks, it looks good.
I think the code should also be extended to handle var=const_var
i.e. when you arrive to var_decl with const_value_known_p or const_decl.

I suppose this will need to look into constructors then...

Honza

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))
Index: src/gcc/testsuite/gfortran.dg/ipcp-array-1.f90
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gfortran.dg/ipcp-array-1.f90
@@ -0,0 +1,19 @@ 
+! { dg-do compile }
+! { dg-options "-O2 -fdump-ipa-cp-details -fno-inline -fdump-tree-optimized" }
+
+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
+
+! { dg-final { scan-ipa-dump "Creating a specialized node of foo" "cp" } }
+! { dg-final { scan-ipa-dump-times "Aggregate replacements\[^=\]*=\[^=\]*=\[^=\]*=\[^=\]*=\[^=\]*=\[^=\]*=\[^=\]*=\[^=\]*=" 2 "cp" } }
+! { dg-final { cleanup-ipa-dump "cp" } }
+! { dg-final { scan-tree-dump-not "stride;" "optimized" } }
+! { dg-final { scan-tree-dump-not "lbound;" "optimized" } }
+! { dg-final { cleanup-tree-dump "optimized" } }