Patchwork [trunk,PR57358] Avoid IPA-CP analysis if attribute optimize precludes it

login
register
mail settings
Submitter Martin Jambor
Date June 12, 2013, 12:33 p.m.
Message ID <20130612123300.GE26236@virgil.suse>
Download mbox | patch
Permalink /patch/250753/
State New
Headers show

Comments

Martin Jambor - June 12, 2013, 12:33 p.m.
Hi,

this is how I would like to fix the ICE when analyzing a function with
attribute optimize on trunk.  Inlining, the other user of the
analysis, is already smart enough not to analyze such functions, so
this teaches IPA-CP to do the same thing.  Consequently, functions
witrh attribute optimize(O0) or even optimize(fno-ipa-cp) will be
completely ignored by IPA-CP and friends, making it completely opaque
black box during the analysis.

It is a tiny bit more efficient than the simple fix for the branch
(but changes compiler behavior).  I assume it can also come handy
during some debugging/analysis.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin


2013-06-11  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/57358
	* ipa-prop.c (ipa_func_spec_opts_forbid_analysis_p): New function.
	(ipa_compute_jump_functions_for_edge): Bail out if it returns true.
	(ipa_analyze_params_uses): Generate pessimistic info when true.

testsuite
	* gcc.dg/ipa/pr57358.c: New test.
Jan Hubicka - June 20, 2013, 11:32 a.m.
> 
> 2013-06-11  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/57358
> 	* ipa-prop.c (ipa_func_spec_opts_forbid_analysis_p): New function.
> 	(ipa_compute_jump_functions_for_edge): Bail out if it returns true.
> 	(ipa_analyze_params_uses): Generate pessimistic info when true.
> 
> testsuite
> 	* gcc.dg/ipa/pr57358.c: New test.
> 
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -78,6 +78,21 @@ struct ipa_cst_ref_desc
>  
>  static alloc_pool ipa_refdesc_pool;
>  
> +/* Return true if DECL_FUNCTION_SPECIFIC_OPTIMIZATION of the decl associated
> +   with NODE should prevent us from analyzing it for the purposes of IPA-CP.  */
> +
> +static bool
> +ipa_func_spec_opts_forbid_analysis_p (struct cgraph_node *node)
> +{
> +  tree fs_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (node->symbol.decl);
> +  struct cl_optimization *os;
> +
> +  if (!fs_opts)
> +    return false;
> +  os = TREE_OPTIMIZATION (fs_opts);
> +  return !os->x_optimize || !os->x_flag_ipa_cp;
> +}

Hmm, somewhat ugly, but I suppose it is as ugly as it needs to be.
I wonder about the other IPA passes. Do we want to always ignore them?
How the ICE happens? Is it because we don't do the analysis?

Honza
Martin Jambor - June 20, 2013, 12:37 p.m.
Hi,

On Thu, Jun 20, 2013 at 01:32:38PM +0200, Jan Hubicka wrote:
> > 
> > 2013-06-11  Martin Jambor  <mjambor@suse.cz>
> > 
> > 	PR tree-optimization/57358
> > 	* ipa-prop.c (ipa_func_spec_opts_forbid_analysis_p): New function.
> > 	(ipa_compute_jump_functions_for_edge): Bail out if it returns true.
> > 	(ipa_analyze_params_uses): Generate pessimistic info when true.
> > 
> > testsuite
> > 	* gcc.dg/ipa/pr57358.c: New test.
> > 
> > Index: src/gcc/ipa-prop.c
> > ===================================================================
> > --- src.orig/gcc/ipa-prop.c
> > +++ src/gcc/ipa-prop.c
> > @@ -78,6 +78,21 @@ struct ipa_cst_ref_desc
> >  
> >  static alloc_pool ipa_refdesc_pool;
> >  
> > +/* Return true if DECL_FUNCTION_SPECIFIC_OPTIMIZATION of the decl associated
> > +   with NODE should prevent us from analyzing it for the purposes of IPA-CP.  */
> > +
> > +static bool
> > +ipa_func_spec_opts_forbid_analysis_p (struct cgraph_node *node)
> > +{
> > +  tree fs_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (node->symbol.decl);
> > +  struct cl_optimization *os;
> > +
> > +  if (!fs_opts)
> > +    return false;
> > +  os = TREE_OPTIMIZATION (fs_opts);
> > +  return !os->x_optimize || !os->x_flag_ipa_cp;
> > +}
> 
> Hmm, somewhat ugly, but I suppose it is as ugly as it needs to be.
> I wonder about the other IPA passes. Do we want to always ignore them?

Well, I suppose that if we wanted to truly support the optimize
attribute in its full strength and generality (or at least make an
effort to do so), all IPA passes would need to check for their flag(s)
their.  In fact, this should be even more ugly if done really
properly.  -fno-indirect-inlinig should be checked in
ipa_analyze_call_uses, for example, -fno-devirtualize at various other
places...  And of course, we should probably never just bail out but
mark the flags in the IPA structures instead because they are used by
both IPA-CP and inlining and perhaps disabling the features in one
should not disable it another...

...so I just decided this has never worked and I was not going to
spend too much time fixing it.  I hesitated about the
os->x_flag_ipa_cp test, but eventually thought it might be useful for
us when debugging so I kept it.  Still, publicly I'd continue saying
that IPA passes cannot be adjusted by the optimize attribute.

> How the ICE happens? Is it because we don't do the analysis?

No, we do, and we ask alias oracle about stuff even though there are
no VDEFs and VUSEs which segfaults.  IPA passes just have to be
careful not to ICE on -O0 functions.

Thanks,

Martin

Patch

Index: src/gcc/ipa-prop.c
===================================================================
--- src.orig/gcc/ipa-prop.c
+++ src/gcc/ipa-prop.c
@@ -78,6 +78,21 @@  struct ipa_cst_ref_desc
 
 static alloc_pool ipa_refdesc_pool;
 
+/* Return true if DECL_FUNCTION_SPECIFIC_OPTIMIZATION of the decl associated
+   with NODE should prevent us from analyzing it for the purposes of IPA-CP.  */
+
+static bool
+ipa_func_spec_opts_forbid_analysis_p (struct cgraph_node *node)
+{
+  tree fs_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (node->symbol.decl);
+  struct cl_optimization *os;
+
+  if (!fs_opts)
+    return false;
+  os = TREE_OPTIMIZATION (fs_opts);
+  return !os->x_optimize || !os->x_flag_ipa_cp;
+}
+
 /* Return index of the formal whose tree is PTREE in function which corresponds
    to INFO.  */
 
@@ -1446,6 +1461,9 @@  ipa_compute_jump_functions_for_edge (str
     return;
   vec_safe_grow_cleared (args->jump_functions, arg_num);
 
+  if (ipa_func_spec_opts_forbid_analysis_p (cs->caller))
+    return;
+
   for (n = 0; n < arg_num; n++)
     {
       struct ipa_jump_func *jfunc = ipa_get_ith_jump_func (args, n);
@@ -1936,6 +1954,17 @@  ipa_analyze_params_uses (struct cgraph_n
   if (ipa_get_param_count (info) == 0 || info->uses_analysis_done)
     return;
 
+  info->uses_analysis_done = 1;
+  if (ipa_func_spec_opts_forbid_analysis_p (node))
+    {
+      for (i = 0; i < ipa_get_param_count (info); i++)
+	{
+	  ipa_set_param_used (info, i, true);
+	  ipa_set_controlled_uses (info, i, IPA_UNDESCRIBED_USE);
+	}
+      return;
+    }
+
   for (i = 0; i < ipa_get_param_count (info); i++)
     {
       tree parm = ipa_get_param (info, i);
@@ -1992,8 +2021,6 @@  ipa_analyze_params_uses (struct cgraph_n
 				       visit_ref_for_mod_analysis,
 				       visit_ref_for_mod_analysis);
     }
-
-  info->uses_analysis_done = 1;
 }
 
 /* Free stuff in PARMS_AINFO, assume there are PARAM_COUNT parameters.  */
Index: src/gcc/testsuite/gcc.dg/ipa/pr57358.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/ipa/pr57358.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+struct t { void (*func)(void*); };
+void test_func(struct t* a) __attribute__((optimize("O0")));
+void test_func(struct t* a)
+{
+  a->func(0);
+}