diff mbox

Pass manager: add support for termination of pass list

Message ID 5638C127.5060508@suse.cz
State New
Headers show

Commit Message

Martin Liška Nov. 3, 2015, 2:13 p.m. UTC 
On 11/03/2015 02:46 PM, Richard Biener wrote:
> On Fri, Oct 30, 2015 at 1:53 PM, Martin Liška <mliska@suse.cz> wrote:
>> On 10/30/2015 01:13 PM, Richard Biener wrote:
>>> So I suggest to do the push/pop of cfun there.
>>> do_per_function_toporder can be made static btw.
>>>
>>> Richard.
>>
>> Right, I've done that and it works (bootstrap has been currently running),
>> feasible for HSA branch too.
>>
>> tree-pass.h:
>>
>> /* Declare for plugins.  */
>> extern void do_per_function_toporder (void (*) (function *, void *), void *);
>>
>> Attaching the patch that I'm going to test.
> 
> Err.
> 
> +      cgraph_node::get (current_function_decl)->release_body ();
> +
> +      current_function_decl = NULL;
> +      set_cfun (NULL);
> 
> I'd have expected
> 
>       tree fn = cfun->decl;
>       pop_cfun ();
>       gcc_assert (!cfun);
>       cgraph_node::get (fn)->release_body ();
> 
> here.

Yeah, that works, but we have to add following hunk:



If you are fine with that, looks we've fixed all issues related to the change, right?
Updated version of the is attached.

Martin

> 
>> Martin
>>

Comments

Richard Biener Nov. 3, 2015, 2:44 p.m. UTC | #1 
On Tue, Nov 3, 2015 at 3:13 PM, Martin Liška <mliska@suse.cz> wrote:
> On 11/03/2015 02:46 PM, Richard Biener wrote:
>> On Fri, Oct 30, 2015 at 1:53 PM, Martin Liška <mliska@suse.cz> wrote:
>>> On 10/30/2015 01:13 PM, Richard Biener wrote:
>>>> So I suggest to do the push/pop of cfun there.
>>>> do_per_function_toporder can be made static btw.
>>>>
>>>> Richard.
>>>
>>> Right, I've done that and it works (bootstrap has been currently running),
>>> feasible for HSA branch too.
>>>
>>> tree-pass.h:
>>>
>>> /* Declare for plugins.  */
>>> extern void do_per_function_toporder (void (*) (function *, void *), void *);
>>>
>>> Attaching the patch that I'm going to test.
>>
>> Err.
>>
>> +      cgraph_node::get (current_function_decl)->release_body ();
>> +
>> +      current_function_decl = NULL;
>> +      set_cfun (NULL);
>>
>> I'd have expected
>>
>>       tree fn = cfun->decl;
>>       pop_cfun ();
>>       gcc_assert (!cfun);
>>       cgraph_node::get (fn)->release_body ();
>>
>> here.
>
> Yeah, that works, but we have to add following hunk:
>
> diff --git a/gcc/function.c b/gcc/function.c
> index aaf49a4..4718fe1 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -4756,6 +4756,13 @@ push_cfun (struct function *new_cfun)
>  void
>  pop_cfun (void)
>  {
> +  if (cfun_stack.is_empty ())
> +    {
> +      set_cfun (NULL);
> +      current_function_decl = NULL_TREE;
> +      return;
> +    }
> +
>    struct function *new_cfun = cfun_stack.pop ();
>    /* When in_dummy_function, we do have a cfun but current_function_decl is
>       NULL.  We also allow pushing NULL cfun and subsequently changing

Why again?  cfun should be set via push_cfun here so what's having cfun == NULL
at the pop_cfun point?  Or rather, what code used set_cfun () instead
of push_cfun ()?

>
> If you are fine with that, looks we've fixed all issues related to the change, right?
> Updated version of the is attached.
>
> Martin
>
>>
>>> Martin
>>>
>
diff mbox

Patch

From 0921507773eedadca1216a9edca954af240b7a49 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 22 Oct 2015 12:46:16 +0200
Subject: [PATCH] Pass manager: add support for termination of pass list

gcc/ChangeLog:

2015-10-30  Martin Liska  <mliska@suse.cz>

	* function.c (pop_cfun): Set cfun and current_function_decl to
	NULL if the cfun_stack is empty.
	* passes.c (do_per_function_toporder): Push to cfun before
	calling the pass manager.
	(execute_one_pass): Handle TODO_discard_function.
	(execute_pass_list_1): Terminate if current function is null.
	(execute_pass_list): Do not push and pop function.
	* tree-pass.h: Define new TODO_discard_function.
---
 gcc/function.c  |  7 +++++++
 gcc/passes.c    | 32 ++++++++++++++++++++++++++++----
 gcc/tree-pass.h |  3 +++
 4 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/gcc/function.c b/gcc/function.c
index aaf49a4..4718fe1 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4756,6 +4756,13 @@  push_cfun (struct function *new_cfun)
 void
 pop_cfun (void)
 {
+  if (cfun_stack.is_empty ())
+    {
+      set_cfun (NULL);
+      current_function_decl = NULL_TREE;
+      return;
+    }
+
   struct function *new_cfun = cfun_stack.pop ();
   /* When in_dummy_function, we do have a cfun but current_function_decl is
      NULL.  We also allow pushing NULL cfun and subsequently changing
diff --git a/gcc/passes.c b/gcc/passes.c
index d9af93a..d764a22 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -1706,7 +1706,12 @@  do_per_function_toporder (void (*callback) (function *, void *data), void *data)
 	  order[i] = NULL;
 	  node->process = 0;
 	  if (node->has_gimple_body_p ())
-	    callback (DECL_STRUCT_FUNCTION (node->decl), data);
+	    {
+	      struct function *fn = DECL_STRUCT_FUNCTION (node->decl);
+	      push_cfun (fn);
+	      callback (fn, data);
+	      pop_cfun ();
+	    }
 	}
       symtab->remove_cgraph_removal_hook (hook);
     }
@@ -2347,6 +2352,23 @@  execute_one_pass (opt_pass *pass)
 
   current_pass = NULL;
 
+  if (todo_after & TODO_discard_function)
+    {
+      gcc_assert (cfun);
+      /* As cgraph_node::release_body expects release dominators info,
+	 we have to release it.  */
+      if (dom_info_available_p (CDI_DOMINATORS))
+	free_dominance_info (CDI_DOMINATORS);
+
+      if (dom_info_available_p (CDI_POST_DOMINATORS))
+	free_dominance_info (CDI_POST_DOMINATORS);
+
+      tree fn = cfun->decl;
+      pop_cfun ();
+      gcc_assert (!cfun);
+      cgraph_node::get (fn)->release_body ();
+    }
+
   /* Signal this is a suitable GC collection point.  */
   if (!((todo_after | pass->todo_flags_finish) & TODO_do_not_ggc_collect))
     ggc_collect ();
@@ -2361,6 +2383,9 @@  execute_pass_list_1 (opt_pass *pass)
     {
       gcc_assert (pass->type == GIMPLE_PASS
 		  || pass->type == RTL_PASS);
+
+      if (cfun == NULL)
+	return;
       if (execute_one_pass (pass) && pass->sub)
         execute_pass_list_1 (pass->sub);
 
@@ -2372,14 +2397,13 @@  execute_pass_list_1 (opt_pass *pass)
 void
 execute_pass_list (function *fn, opt_pass *pass)
 {
-  push_cfun (fn);
+  gcc_assert (fn == cfun);
   execute_pass_list_1 (pass);
-  if (fn->cfg)
+  if (cfun && fn->cfg)
     {
       free_dominance_info (CDI_DOMINATORS);
       free_dominance_info (CDI_POST_DOMINATORS);
     }
-  pop_cfun ();
 }
 
 /* Write out all LTO data.  */
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index c03e3d8..713f068 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -300,6 +300,9 @@  protected:
 /* Rebuild the callgraph edges.  */
 #define TODO_rebuild_cgraph_edges       (1 << 22)
 
+/* Release function body and stop pass manager.  */
+#define TODO_discard_function		(1 << 23)
+
 /* Internally used in execute_function_todo().  */
 #define TODO_update_ssa_any		\
     (TODO_update_ssa			\
-- 
2.6.2