Re: [PATCH] Fix PR60911

[Richard Biener <rguenther at suse dot de>]

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