public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR60911
@ 2014-04-24 11:42 Richard Biener
  2014-04-24 14:37 ` Jan Hubicka
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2014-04-24 11:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka


Simple IPA passes are supposed to see function bodies with IPA transforms 
applied - this is what the code in execute_one_pass tries to ensure.
But that doesn't work anymore with on-demand function-body loading.
The following addresses this in the least intrusive way - inlining
do_per_function (apply_ipa_transforms) and adjusting it accordingly.

This IMHO is definitely the solution for the 4.9 branch (and for
the time being on trunk).

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Ok for trunk and branch?

Thanks,
Richard.

2014-04-24  Richard Biener  <rguenther@suse.de>

	PR ipa/60911
	* passes.c (apply_ipa_transforms): Inline into only caller ...
	(execute_one_pass): ... here.  Properly bring in function
	bodies for nodes we want to apply IPA transforms to.

	* gcc.dg/lto/pr60911_0.c: New testcase.

Index: gcc/passes.c
===================================================================
--- gcc/passes.c	(revision 209742)
+++ gcc/passes.c	(working copy)
@@ -2109,20 +2109,6 @@ execute_all_ipa_transforms (void)
     }
 }
 
-/* Callback for do_per_function to apply all IPA transforms.  */
-
-static void
-apply_ipa_transforms (void *data)
-{
-  struct cgraph_node *node = cgraph_get_node (current_function_decl);
-  if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
-    {
-      *(bool *)data = true;
-      execute_all_ipa_transforms ();
-      rebuild_cgraph_edges ();
-    }
-}
-
 /* Check if PASS is explicitly disabled or enabled and return
    the gate status.  FUNC is the function to be processed, and
    GATE_STATUS is the gate status determined by pass manager by
@@ -2194,8 +2180,26 @@ execute_one_pass (opt_pass *pass)
      Apply all trnasforms first.  */
   if (pass->type == SIMPLE_IPA_PASS)
     {
+      struct cgraph_node *node;
       bool applied = false;
-      do_per_function (apply_ipa_transforms, (void *)&applied);
+      FOR_EACH_DEFINED_FUNCTION (node)
+	if (node->analyzed
+	    && cgraph_function_with_gimple_body_p (node)
+	    && (!node->clone_of || node->decl != node->clone_of->decl))
+	  {
+	    if (!node->global.inlined_to
+		&& node->ipa_transforms_to_apply.exists ())
+	      {
+		cgraph_get_body (node);
+		push_cfun (DECL_STRUCT_FUNCTION (node->decl));
+		execute_all_ipa_transforms ();
+		rebuild_cgraph_edges ();
+		free_dominance_info (CDI_DOMINATORS);
+		free_dominance_info (CDI_POST_DOMINATORS);
+		pop_cfun ();
+		applied = true;
+	      }
+	  }
       if (applied)
         symtab_remove_unreachable_nodes (true, dump_file);
       /* Restore current_pass.  */
Index: gcc/testsuite/gcc.dg/lto/pr60911_0.c
===================================================================
--- gcc/testsuite/gcc.dg/lto/pr60911_0.c	(revision 0)
+++ gcc/testsuite/gcc.dg/lto/pr60911_0.c	(working copy)
@@ -0,0 +1,21 @@
+// { dg-lto-do run }
+// { dg-lto-options { { -O2 -flto -fipa-pta } } }
+
+int __attribute__ ((__noinline__)) f (unsigned *p, int *x)
+{
+  int y = *p++ & 0xfff;
+  *x++ = y;
+  *x = *p;
+  return y;
+}
+
+int
+main ()
+{
+  unsigned u[2] = { 0x3aad, 0x5ad1 };
+  int x[2] = { 17689, 23456 };
+
+  if (f (u, x) != 0xaad || x[0] != 0xaad || x[1] != 0x5ad1)
+    __builtin_abort ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix PR60911
  2014-04-24 11:42 [PATCH] Fix PR60911 Richard Biener
@ 2014-04-24 14:37 ` Jan Hubicka
  2014-04-25  7:43   ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Hubicka @ 2014-04-24 14:37 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jan Hubicka

> 
> Simple IPA passes are supposed to see function bodies with IPA transforms 
> applied - this is what the code in execute_one_pass tries to ensure.
> But that doesn't work anymore with on-demand function-body loading.
> The following addresses this in the least intrusive way - inlining
> do_per_function (apply_ipa_transforms) and adjusting it accordingly.
> 
> This IMHO is definitely the solution for the 4.9 branch (and for
> the time being on trunk).
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Ok for trunk and branch?

I think it is fine for both 4.9 and mainline. I will try to make better version
for mainline as explained in PR hortly.

Can you, please, double check that it won't load all bodies prior late optimization
by default? 
Looking at gate of pass_omp_simd_clone, perhaps it actually kills late loading
of bodies and perhaps we need to mark in cgraph node whether the given node
needs clonning and page the gate return false if partition has no such unit?
bool
pass_omp_simd_clone::gate (function *)
{
  return ((flag_openmp || flag_openmp_simd
           || flag_cilkplus
           || (in_lto_p && !flag_wpa))
          && (targetm.simd_clone.compute_vecsize_and_simdlen != NULL));
}

I did not see there the in_lto_p previously.

Honza
> 
> Thanks,
> Richard.
> 
> 2014-04-24  Richard Biener  <rguenther@suse.de>
> 
> 	PR ipa/60911
> 	* passes.c (apply_ipa_transforms): Inline into only caller ...
> 	(execute_one_pass): ... here.  Properly bring in function
> 	bodies for nodes we want to apply IPA transforms to.
> 
> 	* gcc.dg/lto/pr60911_0.c: New testcase.
> 
> Index: gcc/passes.c
> ===================================================================
> --- gcc/passes.c	(revision 209742)
> +++ gcc/passes.c	(working copy)
> @@ -2109,20 +2109,6 @@ execute_all_ipa_transforms (void)
>      }
>  }
>  
> -/* Callback for do_per_function to apply all IPA transforms.  */
> -
> -static void
> -apply_ipa_transforms (void *data)
> -{
> -  struct cgraph_node *node = cgraph_get_node (current_function_decl);
> -  if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
> -    {
> -      *(bool *)data = true;
> -      execute_all_ipa_transforms ();
> -      rebuild_cgraph_edges ();
> -    }
> -}
> -
>  /* Check if PASS is explicitly disabled or enabled and return
>     the gate status.  FUNC is the function to be processed, and
>     GATE_STATUS is the gate status determined by pass manager by
> @@ -2194,8 +2180,26 @@ execute_one_pass (opt_pass *pass)
>       Apply all trnasforms first.  */
>    if (pass->type == SIMPLE_IPA_PASS)
>      {
> +      struct cgraph_node *node;
>        bool applied = false;
> -      do_per_function (apply_ipa_transforms, (void *)&applied);
> +      FOR_EACH_DEFINED_FUNCTION (node)
> +	if (node->analyzed
> +	    && cgraph_function_with_gimple_body_p (node)
> +	    && (!node->clone_of || node->decl != node->clone_of->decl))
> +	  {
> +	    if (!node->global.inlined_to
> +		&& node->ipa_transforms_to_apply.exists ())
> +	      {
> +		cgraph_get_body (node);
> +		push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> +		execute_all_ipa_transforms ();
> +		rebuild_cgraph_edges ();
> +		free_dominance_info (CDI_DOMINATORS);
> +		free_dominance_info (CDI_POST_DOMINATORS);
> +		pop_cfun ();
> +		applied = true;
> +	      }
> +	  }
>        if (applied)
>          symtab_remove_unreachable_nodes (true, dump_file);
>        /* Restore current_pass.  */
> Index: gcc/testsuite/gcc.dg/lto/pr60911_0.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/lto/pr60911_0.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/lto/pr60911_0.c	(working copy)
> @@ -0,0 +1,21 @@
> +// { dg-lto-do run }
> +// { dg-lto-options { { -O2 -flto -fipa-pta } } }
> +
> +int __attribute__ ((__noinline__)) f (unsigned *p, int *x)
> +{
> +  int y = *p++ & 0xfff;
> +  *x++ = y;
> +  *x = *p;
> +  return y;
> +}
> +
> +int
> +main ()
> +{
> +  unsigned u[2] = { 0x3aad, 0x5ad1 };
> +  int x[2] = { 17689, 23456 };
> +
> +  if (f (u, x) != 0xaad || x[0] != 0xaad || x[1] != 0x5ad1)
> +    __builtin_abort ();
> +  return 0;
> +}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix PR60911
  2014-04-24 14:37 ` Jan Hubicka
@ 2014-04-25  7:43   ` Richard Biener
  2014-04-25 17:20     ` Jan Hubicka
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2014-04-25  7:43 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

On Thu, 24 Apr 2014, Jan Hubicka wrote:

> > 
> > Simple IPA passes are supposed to see function bodies with IPA transforms 
> > applied - this is what the code in execute_one_pass tries to ensure.
> > But that doesn't work anymore with on-demand function-body loading.
> > The following addresses this in the least intrusive way - inlining
> > do_per_function (apply_ipa_transforms) and adjusting it accordingly.
> > 
> > This IMHO is definitely the solution for the 4.9 branch (and for
> > the time being on trunk).
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > 
> > Ok for trunk and branch?
> 
> I think it is fine for both 4.9 and mainline. I will try to make better version
> for mainline as explained in PR hortly.
> 
> Can you, please, double check that it won't load all bodies prior late 
> optimization by default? Looking at gate of pass_omp_simd_clone, perhaps 

Well, first of all it will only load bodies with IPA transforms to apply
(yeah, that includes inlining, right?).  Then it's only executed if
a small IPA pass actually executes, but ...

> it actually kills late loading of bodies and perhaps we need to mark in 
> cgraph node whether the given node needs clonning and page the gate 
> return false if partition has no such unit? bool 
> pass_omp_simd_clone::gate (function *) {
>   return ((flag_openmp || flag_openmp_simd
>            || flag_cilkplus
>            || (in_lto_p && !flag_wpa))
>           && (targetm.simd_clone.compute_vecsize_and_simdlen != NULL));
> }
> 
> I did not see there the in_lto_p previously.

... this is IIRC because you can't rely on -fopenmp/-fopenmp-simd/-fcilk+
to be present on the LTO commandline.

Richard.

> Honza
> > 
> > Thanks,
> > Richard.
> > 
> > 2014-04-24  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR ipa/60911
> > 	* passes.c (apply_ipa_transforms): Inline into only caller ...
> > 	(execute_one_pass): ... here.  Properly bring in function
> > 	bodies for nodes we want to apply IPA transforms to.
> > 
> > 	* gcc.dg/lto/pr60911_0.c: New testcase.
> > 
> > Index: gcc/passes.c
> > ===================================================================
> > --- gcc/passes.c	(revision 209742)
> > +++ gcc/passes.c	(working copy)
> > @@ -2109,20 +2109,6 @@ execute_all_ipa_transforms (void)
> >      }
> >  }
> >  
> > -/* Callback for do_per_function to apply all IPA transforms.  */
> > -
> > -static void
> > -apply_ipa_transforms (void *data)
> > -{
> > -  struct cgraph_node *node = cgraph_get_node (current_function_decl);
> > -  if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
> > -    {
> > -      *(bool *)data = true;
> > -      execute_all_ipa_transforms ();
> > -      rebuild_cgraph_edges ();
> > -    }
> > -}
> > -
> >  /* Check if PASS is explicitly disabled or enabled and return
> >     the gate status.  FUNC is the function to be processed, and
> >     GATE_STATUS is the gate status determined by pass manager by
> > @@ -2194,8 +2180,26 @@ execute_one_pass (opt_pass *pass)
> >       Apply all trnasforms first.  */
> >    if (pass->type == SIMPLE_IPA_PASS)
> >      {
> > +      struct cgraph_node *node;
> >        bool applied = false;
> > -      do_per_function (apply_ipa_transforms, (void *)&applied);
> > +      FOR_EACH_DEFINED_FUNCTION (node)
> > +	if (node->analyzed
> > +	    && cgraph_function_with_gimple_body_p (node)
> > +	    && (!node->clone_of || node->decl != node->clone_of->decl))
> > +	  {
> > +	    if (!node->global.inlined_to
> > +		&& node->ipa_transforms_to_apply.exists ())
> > +	      {
> > +		cgraph_get_body (node);
> > +		push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> > +		execute_all_ipa_transforms ();
> > +		rebuild_cgraph_edges ();
> > +		free_dominance_info (CDI_DOMINATORS);
> > +		free_dominance_info (CDI_POST_DOMINATORS);
> > +		pop_cfun ();
> > +		applied = true;
> > +	      }
> > +	  }
> >        if (applied)
> >          symtab_remove_unreachable_nodes (true, dump_file);
> >        /* Restore current_pass.  */
> > Index: gcc/testsuite/gcc.dg/lto/pr60911_0.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/lto/pr60911_0.c	(revision 0)
> > +++ gcc/testsuite/gcc.dg/lto/pr60911_0.c	(working copy)
> > @@ -0,0 +1,21 @@
> > +// { dg-lto-do run }
> > +// { dg-lto-options { { -O2 -flto -fipa-pta } } }
> > +
> > +int __attribute__ ((__noinline__)) f (unsigned *p, int *x)
> > +{
> > +  int y = *p++ & 0xfff;
> > +  *x++ = y;
> > +  *x = *p;
> > +  return y;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  unsigned u[2] = { 0x3aad, 0x5ad1 };
> > +  int x[2] = { 17689, 23456 };
> > +
> > +  if (f (u, x) != 0xaad || x[0] != 0xaad || x[1] != 0x5ad1)
> > +    __builtin_abort ();
> > +  return 0;
> > +}
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix PR60911
  2014-04-25  7:43   ` Richard Biener
@ 2014-04-25 17:20     ` Jan Hubicka
  2014-04-28  8:20       ` Richard Biener
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Hubicka @ 2014-04-25 17:20 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

> On Thu, 24 Apr 2014, Jan Hubicka wrote:
> 
> > > 
> > > Simple IPA passes are supposed to see function bodies with IPA transforms 
> > > applied - this is what the code in execute_one_pass tries to ensure.
> > > But that doesn't work anymore with on-demand function-body loading.
> > > The following addresses this in the least intrusive way - inlining
> > > do_per_function (apply_ipa_transforms) and adjusting it accordingly.
> > > 
> > > This IMHO is definitely the solution for the 4.9 branch (and for
> > > the time being on trunk).
> > > 
> > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > 
> > > Ok for trunk and branch?
> > 
> > I think it is fine for both 4.9 and mainline. I will try to make better version
> > for mainline as explained in PR hortly.
> > 
> > Can you, please, double check that it won't load all bodies prior late 
> > optimization by default? Looking at gate of pass_omp_simd_clone, perhaps 
> 
> Well, first of all it will only load bodies with IPA transforms to apply
> (yeah, that includes inlining, right?).  Then it's only executed if
> a small IPA pass actually executes, but ...

All functions have transforms to apply. They are added to all bodies of
functions that are around when pass is run. Not only to those pass wants to do
something. This is how first cut got implemented in 2004 and it is still the
same. We may want to change this, since the per-function transform lists tends
to be memory consuming these days. The reason why the lists are per-function
was that I expected IPA passes to introduce new functions in the middle of
optimization processing (it now happens for sample with ctor merging) and those
sould not be processed by transforms of IPA passes that never see them.
> 
> > it actually kills late loading of bodies and perhaps we need to mark in 
> > cgraph node whether the given node needs clonning and page the gate 
> > return false if partition has no such unit? bool 
> > pass_omp_simd_clone::gate (function *) {
> >   return ((flag_openmp || flag_openmp_simd
> >            || flag_cilkplus
> >            || (in_lto_p && !flag_wpa))
> >           && (targetm.simd_clone.compute_vecsize_and_simdlen != NULL));
> > }
> > 
> > I did not see there the in_lto_p previously.
> 
> ... this is IIRC because you can't rely on -fopenmp/-fopenmp-simd/-fcilk+
> to be present on the LTO commandline.

Yep, together with your change it will kill lazy loading, because the pass is
practicaly unconditoinal with LTO and all functions will have transforms on them.
Adding simple flag if function needs openmp_simd processing would solve it.

Honza
> 
> Richard.
> 
> > Honza
> > > 
> > > Thanks,
> > > Richard.
> > > 
> > > 2014-04-24  Richard Biener  <rguenther@suse.de>
> > > 
> > > 	PR ipa/60911
> > > 	* passes.c (apply_ipa_transforms): Inline into only caller ...
> > > 	(execute_one_pass): ... here.  Properly bring in function
> > > 	bodies for nodes we want to apply IPA transforms to.
> > > 
> > > 	* gcc.dg/lto/pr60911_0.c: New testcase.
> > > 
> > > Index: gcc/passes.c
> > > ===================================================================
> > > --- gcc/passes.c	(revision 209742)
> > > +++ gcc/passes.c	(working copy)
> > > @@ -2109,20 +2109,6 @@ execute_all_ipa_transforms (void)
> > >      }
> > >  }
> > >  
> > > -/* Callback for do_per_function to apply all IPA transforms.  */
> > > -
> > > -static void
> > > -apply_ipa_transforms (void *data)
> > > -{
> > > -  struct cgraph_node *node = cgraph_get_node (current_function_decl);
> > > -  if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
> > > -    {
> > > -      *(bool *)data = true;
> > > -      execute_all_ipa_transforms ();
> > > -      rebuild_cgraph_edges ();
> > > -    }
> > > -}
> > > -
> > >  /* Check if PASS is explicitly disabled or enabled and return
> > >     the gate status.  FUNC is the function to be processed, and
> > >     GATE_STATUS is the gate status determined by pass manager by
> > > @@ -2194,8 +2180,26 @@ execute_one_pass (opt_pass *pass)
> > >       Apply all trnasforms first.  */
> > >    if (pass->type == SIMPLE_IPA_PASS)
> > >      {
> > > +      struct cgraph_node *node;
> > >        bool applied = false;
> > > -      do_per_function (apply_ipa_transforms, (void *)&applied);
> > > +      FOR_EACH_DEFINED_FUNCTION (node)
> > > +	if (node->analyzed
> > > +	    && cgraph_function_with_gimple_body_p (node)
> > > +	    && (!node->clone_of || node->decl != node->clone_of->decl))
> > > +	  {
> > > +	    if (!node->global.inlined_to
> > > +		&& node->ipa_transforms_to_apply.exists ())
> > > +	      {
> > > +		cgraph_get_body (node);
> > > +		push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> > > +		execute_all_ipa_transforms ();
> > > +		rebuild_cgraph_edges ();
> > > +		free_dominance_info (CDI_DOMINATORS);
> > > +		free_dominance_info (CDI_POST_DOMINATORS);
> > > +		pop_cfun ();
> > > +		applied = true;
> > > +	      }
> > > +	  }
> > >        if (applied)
> > >          symtab_remove_unreachable_nodes (true, dump_file);
> > >        /* Restore current_pass.  */
> > > Index: gcc/testsuite/gcc.dg/lto/pr60911_0.c
> > > ===================================================================
> > > --- gcc/testsuite/gcc.dg/lto/pr60911_0.c	(revision 0)
> > > +++ gcc/testsuite/gcc.dg/lto/pr60911_0.c	(working copy)
> > > @@ -0,0 +1,21 @@
> > > +// { dg-lto-do run }
> > > +// { dg-lto-options { { -O2 -flto -fipa-pta } } }
> > > +
> > > +int __attribute__ ((__noinline__)) f (unsigned *p, int *x)
> > > +{
> > > +  int y = *p++ & 0xfff;
> > > +  *x++ = y;
> > > +  *x = *p;
> > > +  return y;
> > > +}
> > > +
> > > +int
> > > +main ()
> > > +{
> > > +  unsigned u[2] = { 0x3aad, 0x5ad1 };
> > > +  int x[2] = { 17689, 23456 };
> > > +
> > > +  if (f (u, x) != 0xaad || x[0] != 0xaad || x[1] != 0x5ad1)
> > > +    __builtin_abort ();
> > > +  return 0;
> > > +}
> > 
> > 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix PR60911
  2014-04-25 17:20     ` Jan Hubicka
@ 2014-04-28  8:20       ` Richard Biener
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2014-04-28  8:20 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

On Fri, 25 Apr 2014, Jan Hubicka wrote:

> > On Thu, 24 Apr 2014, Jan Hubicka wrote:
> > 
> > > > 
> > > > Simple IPA passes are supposed to see function bodies with IPA transforms 
> > > > applied - this is what the code in execute_one_pass tries to ensure.
> > > > But that doesn't work anymore with on-demand function-body loading.
> > > > The following addresses this in the least intrusive way - inlining
> > > > do_per_function (apply_ipa_transforms) and adjusting it accordingly.
> > > > 
> > > > This IMHO is definitely the solution for the 4.9 branch (and for
> > > > the time being on trunk).
> > > > 
> > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > 
> > > > Ok for trunk and branch?
> > > 
> > > I think it is fine for both 4.9 and mainline. I will try to make better version
> > > for mainline as explained in PR hortly.
> > > 
> > > Can you, please, double check that it won't load all bodies prior late 
> > > optimization by default? Looking at gate of pass_omp_simd_clone, perhaps 
> > 
> > Well, first of all it will only load bodies with IPA transforms to apply
> > (yeah, that includes inlining, right?).  Then it's only executed if
> > a small IPA pass actually executes, but ...
> 
> All functions have transforms to apply. They are added to all bodies of
> functions that are around when pass is run. Not only to those pass wants to do
> something. This is how first cut got implemented in 2004 and it is still the
> same. We may want to change this, since the per-function transform lists tends
> to be memory consuming these days. The reason why the lists are per-function
> was that I expected IPA passes to introduce new functions in the middle of
> optimization processing (it now happens for sample with ctor merging) and those
> sould not be processed by transforms of IPA passes that never see them.
> > 
> > > it actually kills late loading of bodies and perhaps we need to mark in 
> > > cgraph node whether the given node needs clonning and page the gate 
> > > return false if partition has no such unit? bool 
> > > pass_omp_simd_clone::gate (function *) {
> > >   return ((flag_openmp || flag_openmp_simd
> > >            || flag_cilkplus
> > >            || (in_lto_p && !flag_wpa))
> > >           && (targetm.simd_clone.compute_vecsize_and_simdlen != NULL));
> > > }
> > > 
> > > I did not see there the in_lto_p previously.
> > 
> > ... this is IIRC because you can't rely on -fopenmp/-fopenmp-simd/-fcilk+
> > to be present on the LTO commandline.
> 
> Yep, together with your change it will kill lazy loading, because the pass is
> practicaly unconditoinal with LTO and all functions will have transforms on them.
> Adding simple flag if function needs openmp_simd processing would solve it.

Not really - you'd also have to defer body loading to the simple IPA
passes (well, and IPA PTA really needs all bodies anyway).  Here
OMP SIMD isn't really an "IPA" pass (other than it creating new
cgraph nodes).

Richard.

> Honza
> > 
> > Richard.
> > 
> > > Honza
> > > > 
> > > > Thanks,
> > > > Richard.
> > > > 
> > > > 2014-04-24  Richard Biener  <rguenther@suse.de>
> > > > 
> > > > 	PR ipa/60911
> > > > 	* passes.c (apply_ipa_transforms): Inline into only caller ...
> > > > 	(execute_one_pass): ... here.  Properly bring in function
> > > > 	bodies for nodes we want to apply IPA transforms to.
> > > > 
> > > > 	* gcc.dg/lto/pr60911_0.c: New testcase.
> > > > 
> > > > Index: gcc/passes.c
> > > > ===================================================================
> > > > --- gcc/passes.c	(revision 209742)
> > > > +++ gcc/passes.c	(working copy)
> > > > @@ -2109,20 +2109,6 @@ execute_all_ipa_transforms (void)
> > > >      }
> > > >  }
> > > >  
> > > > -/* Callback for do_per_function to apply all IPA transforms.  */
> > > > -
> > > > -static void
> > > > -apply_ipa_transforms (void *data)
> > > > -{
> > > > -  struct cgraph_node *node = cgraph_get_node (current_function_decl);
> > > > -  if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
> > > > -    {
> > > > -      *(bool *)data = true;
> > > > -      execute_all_ipa_transforms ();
> > > > -      rebuild_cgraph_edges ();
> > > > -    }
> > > > -}
> > > > -
> > > >  /* Check if PASS is explicitly disabled or enabled and return
> > > >     the gate status.  FUNC is the function to be processed, and
> > > >     GATE_STATUS is the gate status determined by pass manager by
> > > > @@ -2194,8 +2180,26 @@ execute_one_pass (opt_pass *pass)
> > > >       Apply all trnasforms first.  */
> > > >    if (pass->type == SIMPLE_IPA_PASS)
> > > >      {
> > > > +      struct cgraph_node *node;
> > > >        bool applied = false;
> > > > -      do_per_function (apply_ipa_transforms, (void *)&applied);
> > > > +      FOR_EACH_DEFINED_FUNCTION (node)
> > > > +	if (node->analyzed
> > > > +	    && cgraph_function_with_gimple_body_p (node)
> > > > +	    && (!node->clone_of || node->decl != node->clone_of->decl))
> > > > +	  {
> > > > +	    if (!node->global.inlined_to
> > > > +		&& node->ipa_transforms_to_apply.exists ())
> > > > +	      {
> > > > +		cgraph_get_body (node);
> > > > +		push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> > > > +		execute_all_ipa_transforms ();
> > > > +		rebuild_cgraph_edges ();
> > > > +		free_dominance_info (CDI_DOMINATORS);
> > > > +		free_dominance_info (CDI_POST_DOMINATORS);
> > > > +		pop_cfun ();
> > > > +		applied = true;
> > > > +	      }
> > > > +	  }
> > > >        if (applied)
> > > >          symtab_remove_unreachable_nodes (true, dump_file);
> > > >        /* Restore current_pass.  */
> > > > Index: gcc/testsuite/gcc.dg/lto/pr60911_0.c
> > > > ===================================================================
> > > > --- gcc/testsuite/gcc.dg/lto/pr60911_0.c	(revision 0)
> > > > +++ gcc/testsuite/gcc.dg/lto/pr60911_0.c	(working copy)
> > > > @@ -0,0 +1,21 @@
> > > > +// { dg-lto-do run }
> > > > +// { dg-lto-options { { -O2 -flto -fipa-pta } } }
> > > > +
> > > > +int __attribute__ ((__noinline__)) f (unsigned *p, int *x)
> > > > +{
> > > > +  int y = *p++ & 0xfff;
> > > > +  *x++ = y;
> > > > +  *x = *p;
> > > > +  return y;
> > > > +}
> > > > +
> > > > +int
> > > > +main ()
> > > > +{
> > > > +  unsigned u[2] = { 0x3aad, 0x5ad1 };
> > > > +  int x[2] = { 17689, 23456 };
> > > > +
> > > > +  if (f (u, x) != 0xaad || x[0] != 0xaad || x[1] != 0x5ad1)
> > > > +    __builtin_abort ();
> > > > +  return 0;
> > > > +}
> > > 
> > > 
> > 
> > -- 
> > Richard Biener <rguenther@suse.de>
> > SUSE / SUSE Labs
> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
> > GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-04-28  8:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-24 11:42 [PATCH] Fix PR60911 Richard Biener
2014-04-24 14:37 ` Jan Hubicka
2014-04-25  7:43   ` Richard Biener
2014-04-25 17:20     ` Jan Hubicka
2014-04-28  8:20       ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).