Re: [PATCH] Pass manager: add support for termination of pass list

["Martin Liška" <mliska@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 2655 bytes --]

On 11/03/2015 03:44 PM, Richard Biener wrote:
> 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
>>>>
>>

Hi Richard.

There's the patch we talked about yesterday. It contains a few modification that
were necessary to make it working:

1) cgraph_node::expand_thunk uses just set_cfun (NULL) & current_function_decl = NULL because

      bb = then_bb = else_bb = return_bb
	= init_lowered_empty_function (thunk_fndecl, true, count);

in last branch of expand_thunk which calls allocate_struct_function.

2) i386.c and rs6000.c call init_function_start, as I move preamble to callers, I did the same
for these two functions

I've been running regression test and bootstrap.

Martin

[-- Attachment #2: 0001-Test.patch --]
[-- Type: text/x-patch, Size: 5957 bytes --]

From d0674bfa7d649d4a00b60da057c351b1328085c5 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 3 Nov 2015 16:28:36 +0100
Subject: [PATCH] Test.

---
 gcc/cgraphunit.c           | 10 ++++++----
 gcc/config/i386/i386.c     |  1 +
 gcc/config/rs6000/rs6000.c |  1 +
 gcc/function.c             |  5 -----
 gcc/passes.c               | 32 ++++++++++++++++++++++++++++----
 gcc/tree-pass.h            |  3 +++
 6 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 43d3185..f73d9a7 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1618,6 +1618,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       fn_block = make_node (BLOCK);
       BLOCK_VARS (fn_block) = a;
       DECL_INITIAL (thunk_fndecl) = fn_block;
+      allocate_struct_function (thunk_fndecl, false);
       init_function_start (thunk_fndecl);
       cfun->is_thunk = 1;
       insn_locations_init ();
@@ -1632,7 +1633,6 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       insn_locations_finalize ();
       init_insn_lengths ();
       free_after_compilation (cfun);
-      set_cfun (NULL);
       TREE_ASM_WRITTEN (thunk_fndecl) = 1;
       thunk.thunk_p = false;
       analyzed = false;
@@ -1944,9 +1944,11 @@ cgraph_node::expand (void)
   bitmap_obstack_initialize (NULL);
 
   /* Initialize the RTL code for the function.  */
-  current_function_decl = decl;
   saved_loc = input_location;
   input_location = DECL_SOURCE_LOCATION (decl);
+
+  gcc_assert (DECL_STRUCT_FUNCTION (decl));
+  push_cfun (DECL_STRUCT_FUNCTION (decl));
   init_function_start (decl);
 
   gimple_register_cfg_hooks ();
@@ -2014,8 +2016,8 @@ cgraph_node::expand (void)
 
   /* Make sure that BE didn't give up on compiling.  */
   gcc_assert (TREE_ASM_WRITTEN (decl));
-  set_cfun (NULL);
-  current_function_decl = NULL;
+  if (cfun)
+    pop_cfun ();
 
   /* It would make a lot more sense to output thunks before function body to get more
      forward and lest backwarding jumps.  This however would need solving problem
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 66024e2..2a965f6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10958,6 +10958,7 @@ ix86_code_end (void)
 
       DECL_INITIAL (decl) = make_node (BLOCK);
       current_function_decl = decl;
+      allocate_struct_function (decl, false);
       init_function_start (decl);
       first_function_block_is_cold = false;
       /* Make sure unwind info is emitted for the thunk if needed.  */
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 271c3f9..8bdd646 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -34594,6 +34594,7 @@ rs6000_code_end (void)
 
   DECL_INITIAL (decl) = make_node (BLOCK);
   current_function_decl = decl;
+  allocate_struct_function (decl, false);
   init_function_start (decl);
   first_function_block_is_cold = false;
   /* Make sure unwind info is emitted for the thunk if needed.  */
diff --git a/gcc/function.c b/gcc/function.c
index aaf49a4..0d7cabc 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4957,11 +4957,6 @@ init_dummy_function_start (void)
 void
 init_function_start (tree subr)
 {
-  if (subr && DECL_STRUCT_FUNCTION (subr))
-    set_cfun (DECL_STRUCT_FUNCTION (subr));
-  else
-    allocate_struct_function (subr, false);
-
   /* Initialize backend, if needed.  */
   initialize_rtl ();
 
diff --git a/gcc/passes.c b/gcc/passes.c
index f87dcf4..7a10cb6 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);
       pass = pass->next;
@@ -2371,14 +2396,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 ba53cca..49e22a9 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