diff mbox

[stage1] Make parloops gate more strict

Message ID 5509AFC2.7000301@mentor.com
State New
Headers show

Commit Message

Tom de Vries March 18, 2015, 5:02 p.m. UTC 
On 18-03-15 12:18, Richard Biener wrote:
> On Wed, Mar 18, 2015 at 12:03 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> On 18-03-15 11:16, Richard Biener wrote:
>>>
>>> On Fri, Mar 13, 2015 at 4:28 PM, Tom de Vries <Tom_deVries@mentor.com>
>>> wrote:
>>>>
>>>> On 13-03-15 13:36, Richard Biener wrote:
>>>>>
>>>>>
>>>>> On Fri, Mar 13, 2015 at 1:07 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>>>>
>>>>>>
>>>>>> On Fri, Mar 13, 2015 at 01:04:57PM +0100, Richard Biener wrote:
>>>>>>>
>>>>>>>
>>>>>>> Not really (I don't like -fdump-passes ...), but we need to make sure
>>>>>>> that -fdump-passes doesn't crash (because it runs very early and
>>>>>>> with cfun == NULL I think)
>>>>>>
>>>>>>
>>>>>>
>>>>>> If it runs with cfun == NULL, then supposedly the gates that are
>>>>>> dependent
>>>>>> on current function should for -fdump-passes purposes also return true
>>>>>> if cfun == NULL (well, of course do all the unconditional checks).
>>>>>> Though of course, with optimize/target attributes this is harder, as
>>>>>> different functions can use different options.
>>>>>
>>>>>
>>>>>
>>>>> Yes, one reason why I think -fdump-passes is just broken
>>>>> implementation-wise.
>>>>>
>>>>
>>>> Atm fdump-passes doesn't run with cfun == NULL.
>>>>
>>>>   From pass_manager::dump_passes:
>>>> ...
>>>>     FOR_EACH_FUNCTION (n)
>>>>       if (DECL_STRUCT_FUNCTION (n->decl))
>>>>         {
>>>>           node = n;
>>>>           break;
>>>>         }
>>>>
>>>>     if (!node)
>>>>       return;
>>>>
>>>>     push_cfun (DECL_STRUCT_FUNCTION (node->decl));
>>>
>>>
>>> Um - this now picks a random function which may be one with
>>> an optimize or target attribute associated to it.
>>>
>>
>> Indeed.
>>
>> Attached patch removes that code, and runs the gates with cfun == NULL for
>> -fdump-passes. It at least builds, and allows us to compile
>> src/gcc/testsuite/gcc.dg/dump-pass.c with -O2 -fdump-passes.
>>
>> Should I bootstrap and reg-test, or do you see a problem with this approach?
>
> Yeah - it makes the -fdump-passes "hack" more pervasive throughout
> the compiler.
>
> I suppose it should instead build & push a "dummy" sturct function.
>

Like this?

> Well, or simply don't care for it's brokeness.

I'm afraid leaving it broken just means we'll keep coming back to it. So I'd 
prefer either fixing or removing.

Thanks,
- Tom

Comments

Richard Biener March 19, 2015, 9 a.m. UTC | #1 
On Wed, Mar 18, 2015 at 6:02 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 18-03-15 12:18, Richard Biener wrote:
>>
>> On Wed, Mar 18, 2015 at 12:03 PM, Tom de Vries <Tom_deVries@mentor.com>
>> wrote:
>>>
>>> On 18-03-15 11:16, Richard Biener wrote:
>>>>
>>>>
>>>> On Fri, Mar 13, 2015 at 4:28 PM, Tom de Vries <Tom_deVries@mentor.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 13-03-15 13:36, Richard Biener wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Fri, Mar 13, 2015 at 1:07 PM, Jakub Jelinek <jakub@redhat.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Mar 13, 2015 at 01:04:57PM +0100, Richard Biener wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Not really (I don't like -fdump-passes ...), but we need to make
>>>>>>>> sure
>>>>>>>> that -fdump-passes doesn't crash (because it runs very early and
>>>>>>>> with cfun == NULL I think)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> If it runs with cfun == NULL, then supposedly the gates that are
>>>>>>> dependent
>>>>>>> on current function should for -fdump-passes purposes also return
>>>>>>> true
>>>>>>> if cfun == NULL (well, of course do all the unconditional checks).
>>>>>>> Though of course, with optimize/target attributes this is harder, as
>>>>>>> different functions can use different options.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Yes, one reason why I think -fdump-passes is just broken
>>>>>> implementation-wise.
>>>>>>
>>>>>
>>>>> Atm fdump-passes doesn't run with cfun == NULL.
>>>>>
>>>>>   From pass_manager::dump_passes:
>>>>> ...
>>>>>     FOR_EACH_FUNCTION (n)
>>>>>       if (DECL_STRUCT_FUNCTION (n->decl))
>>>>>         {
>>>>>           node = n;
>>>>>           break;
>>>>>         }
>>>>>
>>>>>     if (!node)
>>>>>       return;
>>>>>
>>>>>     push_cfun (DECL_STRUCT_FUNCTION (node->decl));
>>>>
>>>>
>>>>
>>>> Um - this now picks a random function which may be one with
>>>> an optimize or target attribute associated to it.
>>>>
>>>
>>> Indeed.
>>>
>>> Attached patch removes that code, and runs the gates with cfun == NULL
>>> for
>>> -fdump-passes. It at least builds, and allows us to compile
>>> src/gcc/testsuite/gcc.dg/dump-pass.c with -O2 -fdump-passes.
>>>
>>> Should I bootstrap and reg-test, or do you see a problem with this
>>> approach?
>>
>>
>> Yeah - it makes the -fdump-passes "hack" more pervasive throughout
>> the compiler.
>>
>> I suppose it should instead build & push a "dummy" sturct function.
>>
>
> Like this?

Looks good to me.

Richard.

>> Well, or simply don't care for it's brokeness.
>
>
> I'm afraid leaving it broken just means we'll keep coming back to it. So I'd
> prefer either fixing or removing.
>
> Thanks,
> - Tom
>
diff mbox

Patch

Fix fdump-passes
---
 gcc/function.c  | 37 ++++++++++++++++++++++++++++++++-----
 gcc/function.h  |  2 ++
 gcc/passes.c    | 17 ++---------------
 gcc/tree-chkp.c |  6 ++++--
 4 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/gcc/function.c b/gcc/function.c
index 2c3d142..9ddbad8 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -4862,6 +4862,29 @@  prepare_function_start (void)
   frame_pointer_needed = 0;
 }
 
+void
+push_dummy_function (bool with_decl)
+{
+  tree fn_decl, fn_type, fn_result_decl;
+
+  gcc_assert (!in_dummy_function);
+  in_dummy_function = true;
+
+  if (with_decl)
+    {
+      fn_type = build_function_type_list (void_type_node, NULL_TREE);
+      fn_decl = build_decl (UNKNOWN_LOCATION, FUNCTION_DECL, NULL_TREE,
+			    fn_type);
+      fn_result_decl = build_decl (UNKNOWN_LOCATION, RESULT_DECL,
+					 NULL_TREE, void_type_node);
+      DECL_RESULT (fn_decl) = fn_result_decl;
+    }
+  else
+    fn_decl = NULL_TREE;
+
+  push_struct_function (fn_decl);
+}
+
 /* Initialize the rtl expansion mechanism so that we can do simple things
    like generate sequences.  This is used to provide a context during global
    initialization of some passes.  You must call expand_dummy_function_end
@@ -4870,9 +4893,7 @@  prepare_function_start (void)
 void
 init_dummy_function_start (void)
 {
-  gcc_assert (!in_dummy_function);
-  in_dummy_function = true;
-  push_struct_function (NULL_TREE);
+  push_dummy_function (false);
   prepare_function_start ();
 }
 
@@ -5144,6 +5165,13 @@  expand_function_start (tree subr)
     stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
 }
 
+void
+pop_dummy_function (void)
+{
+  pop_cfun ();
+  in_dummy_function = false;
+}
+
 /* Undo the effects of init_dummy_function_start.  */
 void
 expand_dummy_function_end (void)
@@ -5159,8 +5187,7 @@  expand_dummy_function_end (void)
 
   free_after_parsing (cfun);
   free_after_compilation (cfun);
-  pop_cfun ();
-  in_dummy_function = false;
+  pop_dummy_function ();
 }
 
 /* Helper for diddle_return_value.  */
diff --git a/gcc/function.h b/gcc/function.h
index b89c5ae..349f80c 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -899,6 +899,8 @@  extern int get_next_funcdef_no (void);
 extern int get_last_funcdef_no (void);
 extern void allocate_struct_function (tree, bool);
 extern void push_struct_function (tree fndecl);
+extern void push_dummy_function (bool);
+extern void pop_dummy_function (void);
 extern void init_dummy_function_start (void);
 extern void init_function_start (tree);
 extern void stack_protect_epilogue (void);
diff --git a/gcc/passes.c b/gcc/passes.c
index 23a90d9..bca8dbb 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -944,32 +944,19 @@  dump_passes (void)
 void
 pass_manager::dump_passes () const
 {
-  struct cgraph_node *n, *node = NULL;
+  push_dummy_function (true);
 
   create_pass_tab ();
 
-  FOR_EACH_FUNCTION (n)
-    if (DECL_STRUCT_FUNCTION (n->decl))
-      {
-	node = n;
-	break;
-      }
-
-  if (!node)
-    return;
-
-  push_cfun (DECL_STRUCT_FUNCTION (node->decl));
-
   dump_pass_list (all_lowering_passes, 1);
   dump_pass_list (all_small_ipa_passes, 1);
   dump_pass_list (all_regular_ipa_passes, 1);
   dump_pass_list (all_late_ipa_passes, 1);
   dump_pass_list (all_passes, 1);
 
-  pop_cfun ();
+  pop_dummy_function ();
 }
 
-
 /* Returns the pass with NAME.  */
 
 static opt_pass *
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index d2df4ba..16afadf 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -4337,8 +4337,10 @@  chkp_execute (void)
 static bool
 chkp_gate (void)
 {
-  return cgraph_node::get (cfun->decl)->instrumentation_clone
-    || lookup_attribute ("chkp ctor", DECL_ATTRIBUTES (cfun->decl));
+  cgraph_node *node = cgraph_node::get (cfun->decl);
+  return ((node != NULL
+	   && node->instrumentation_clone)
+	   || lookup_attribute ("chkp ctor", DECL_ATTRIBUTES (cfun->decl)));
 }
 
 namespace {
-- 
1.9.1