Remove unreachable nodes before IPA profile pass (PR ipa/87706).

Message ID 8c883adb-9ec5-c9d5-f1cb-cc48a6ccd2ea@suse.cz
State New
Headers show
Series
  • Remove unreachable nodes before IPA profile pass (PR ipa/87706).
Related show

Commit Message

Martin Liška Nov. 8, 2018, 8:24 a.m.
Hi.

In order to fix the warnings mentioned in the PR, we need
to run remove_unreachable_nodes after early tree passes. That's
however possible only within a IPA pass. Thus I'm calling that
before the profile PASS.

Patch survives regression tests on ppc64le-linux-gnu and majority
of warnings are gone in profiledbootstrap.

Ready for trunk?
Thanks,
Martin

gcc/ChangeLog:

2018-11-08  Martin Liska  <mliska@suse.cz>

	* tree-profile.c: Run TODO_remove_functions before "profile"
	pass in order to remove dead functions that will trigger
	-Wmissing-profile.
---
 gcc/tree-profile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Martin Liška Nov. 8, 2018, 11:39 a.m. | #1
On 11/8/18 12:19 PM, Jan Hubicka wrote:
>> Hi.
>>
>> In order to fix the warnings mentioned in the PR, we need
>> to run remove_unreachable_nodes after early tree passes. That's
>> however possible only within a IPA pass. Thus I'm calling that
>> before the profile PASS.
>>
>> Patch survives regression tests on ppc64le-linux-gnu and majority
>> of warnings are gone in profiledbootstrap.
>>
>> Ready for trunk?
> 
> I think we want to do that even with no -fprofile-generate because the
> unreachable code otherwise goes into all other IPA passes for no good
> reason.  So perhaps adding it as todo after the early optimization
> metapass?

That fails due to gcc_assert.
So one needs:

diff --git a/gcc/passes.c b/gcc/passes.c
index d838d909941..be92a2f3be3 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -485,7 +485,7 @@ const pass_data pass_data_all_early_optimizations =
   0, /* properties_provided */
   0, /* properties_destroyed */
   0, /* todo_flags_start */
-  0, /* todo_flags_finish */
+  TODO_remove_functions | TODO_rebuild_cgraph_edges, /* todo_flags_finish */
 };
 
 class pass_all_early_optimizations : public gimple_opt_pass
@@ -1989,7 +1989,8 @@ execute_todo (unsigned int flags)
      of IPA pass queue.  */
   if (flags & TODO_remove_functions)
     {
-      gcc_assert (!cfun);
+      gcc_assert (!cfun
+		  || strcmp (current_pass->name, "early_optimizations") == 0);
       symtab->remove_unreachable_nodes (dump_file);
     }
 

Or do you prefer to a new pass_remove_functions pass that will be added after
pass_local_optimization_passes ?

Martin

> 
> Honza
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2018-11-08  Martin Liska  <mliska@suse.cz>
>>
>> 	* tree-profile.c: Run TODO_remove_functions before "profile"
>> 	pass in order to remove dead functions that will trigger
>> 	-Wmissing-profile.
>> ---
>>  gcc/tree-profile.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>
> 
>> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
>> index d8f2a3b1ba4..c14ebc556a6 100644
>> --- a/gcc/tree-profile.c
>> +++ b/gcc/tree-profile.c
>> @@ -776,7 +776,7 @@ const pass_data pass_data_ipa_tree_profile =
>>    0, /* properties_required */
>>    0, /* properties_provided */
>>    0, /* properties_destroyed */
>> -  0, /* todo_flags_start */
>> +  TODO_remove_functions, /* todo_flags_start */
>>    TODO_dump_symtab, /* todo_flags_finish */
>>  };
>>  
>>
>
Richard Biener Nov. 8, 2018, 11:41 a.m. | #2
On Thu, Nov 8, 2018 at 12:39 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/8/18 12:19 PM, Jan Hubicka wrote:
> >> Hi.
> >>
> >> In order to fix the warnings mentioned in the PR, we need
> >> to run remove_unreachable_nodes after early tree passes. That's
> >> however possible only within a IPA pass. Thus I'm calling that
> >> before the profile PASS.
> >>
> >> Patch survives regression tests on ppc64le-linux-gnu and majority
> >> of warnings are gone in profiledbootstrap.
> >>
> >> Ready for trunk?
> >
> > I think we want to do that even with no -fprofile-generate because the
> > unreachable code otherwise goes into all other IPA passes for no good
> > reason.  So perhaps adding it as todo after the early optimization
> > metapass?
>
> That fails due to gcc_assert.
> So one needs:
>
> diff --git a/gcc/passes.c b/gcc/passes.c
> index d838d909941..be92a2f3be3 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -485,7 +485,7 @@ const pass_data pass_data_all_early_optimizations =
>    0, /* properties_provided */
>    0, /* properties_destroyed */
>    0, /* todo_flags_start */
> -  0, /* todo_flags_finish */
> +  TODO_remove_functions | TODO_rebuild_cgraph_edges, /* todo_flags_finish */
>  };
>
>  class pass_all_early_optimizations : public gimple_opt_pass
> @@ -1989,7 +1989,8 @@ execute_todo (unsigned int flags)
>       of IPA pass queue.  */
>    if (flags & TODO_remove_functions)
>      {
> -      gcc_assert (!cfun);
> +      gcc_assert (!cfun
> +                 || strcmp (current_pass->name, "early_optimizations") == 0);
>        symtab->remove_unreachable_nodes (dump_file);
>      }
>
>
> Or do you prefer to a new pass_remove_functions pass that will be added after
> pass_local_optimization_passes ?

Can you make it todo_flags_start of pass_ipa_tree_profile instead?

> Martin
>
> >
> > Honza
> >> Thanks,
> >> Martin
> >>
> >> gcc/ChangeLog:
> >>
> >> 2018-11-08  Martin Liska  <mliska@suse.cz>
> >>
> >>      * tree-profile.c: Run TODO_remove_functions before "profile"
> >>      pass in order to remove dead functions that will trigger
> >>      -Wmissing-profile.
> >> ---
> >>  gcc/tree-profile.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>
> >
> >> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
> >> index d8f2a3b1ba4..c14ebc556a6 100644
> >> --- a/gcc/tree-profile.c
> >> +++ b/gcc/tree-profile.c
> >> @@ -776,7 +776,7 @@ const pass_data pass_data_ipa_tree_profile =
> >>    0, /* properties_required */
> >>    0, /* properties_provided */
> >>    0, /* properties_destroyed */
> >> -  0, /* todo_flags_start */
> >> +  TODO_remove_functions, /* todo_flags_start */
> >>    TODO_dump_symtab, /* todo_flags_finish */
> >>  };
> >>
> >>
> >
>
Jan Hubicka Nov. 8, 2018, 11:46 a.m. | #3
> On Thu, Nov 8, 2018 at 12:39 PM Martin Liška <mliska@suse.cz> wrote:
> >
> > On 11/8/18 12:19 PM, Jan Hubicka wrote:
> > >> Hi.
> > >>
> > >> In order to fix the warnings mentioned in the PR, we need
> > >> to run remove_unreachable_nodes after early tree passes. That's
> > >> however possible only within a IPA pass. Thus I'm calling that
> > >> before the profile PASS.
> > >>
> > >> Patch survives regression tests on ppc64le-linux-gnu and majority
> > >> of warnings are gone in profiledbootstrap.
> > >>
> > >> Ready for trunk?
> > >
> > > I think we want to do that even with no -fprofile-generate because the
> > > unreachable code otherwise goes into all other IPA passes for no good
> > > reason.  So perhaps adding it as todo after the early optimization
> > > metapass?
> >
> > That fails due to gcc_assert.
> > So one needs:
> >
> > diff --git a/gcc/passes.c b/gcc/passes.c
> > index d838d909941..be92a2f3be3 100644
> > --- a/gcc/passes.c
> > +++ b/gcc/passes.c
> > @@ -485,7 +485,7 @@ const pass_data pass_data_all_early_optimizations =
> >    0, /* properties_provided */
> >    0, /* properties_destroyed */
> >    0, /* todo_flags_start */
> > -  0, /* todo_flags_finish */
> > +  TODO_remove_functions | TODO_rebuild_cgraph_edges, /* todo_flags_finish */
> >  };
> >
> >  class pass_all_early_optimizations : public gimple_opt_pass
> > @@ -1989,7 +1989,8 @@ execute_todo (unsigned int flags)
> >       of IPA pass queue.  */
> >    if (flags & TODO_remove_functions)
> >      {
> > -      gcc_assert (!cfun);
> > +      gcc_assert (!cfun
> > +                 || strcmp (current_pass->name, "early_optimizations") == 0);
> >        symtab->remove_unreachable_nodes (dump_file);
> >      }
> >
> >
> > Or do you prefer to a new pass_remove_functions pass that will be added after
> > pass_local_optimization_passes ?
> 
> Can you make it todo_flags_start of pass_ipa_tree_profile instead?

It fails because all_early_optimizations are now gimple pass, so it
should be TODO after pass_local_optimization_passes?

Honza
> 
> > Martin
> >
> > >
> > > Honza
> > >> Thanks,
> > >> Martin
> > >>
> > >> gcc/ChangeLog:
> > >>
> > >> 2018-11-08  Martin Liska  <mliska@suse.cz>
> > >>
> > >>      * tree-profile.c: Run TODO_remove_functions before "profile"
> > >>      pass in order to remove dead functions that will trigger
> > >>      -Wmissing-profile.
> > >> ---
> > >>  gcc/tree-profile.c | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >>
> > >
> > >> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
> > >> index d8f2a3b1ba4..c14ebc556a6 100644
> > >> --- a/gcc/tree-profile.c
> > >> +++ b/gcc/tree-profile.c
> > >> @@ -776,7 +776,7 @@ const pass_data pass_data_ipa_tree_profile =
> > >>    0, /* properties_required */
> > >>    0, /* properties_provided */
> > >>    0, /* properties_destroyed */
> > >> -  0, /* todo_flags_start */
> > >> +  TODO_remove_functions, /* todo_flags_start */
> > >>    TODO_dump_symtab, /* todo_flags_finish */
> > >>  };
> > >>
> > >>
> > >
> >
Martin Liška Nov. 8, 2018, 11:50 a.m. | #4
On 11/8/18 12:46 PM, Jan Hubicka wrote:
>> On Thu, Nov 8, 2018 at 12:39 PM Martin Liška <mliska@suse.cz> wrote:
>>>
>>> On 11/8/18 12:19 PM, Jan Hubicka wrote:
>>>>> Hi.
>>>>>
>>>>> In order to fix the warnings mentioned in the PR, we need
>>>>> to run remove_unreachable_nodes after early tree passes. That's
>>>>> however possible only within a IPA pass. Thus I'm calling that
>>>>> before the profile PASS.
>>>>>
>>>>> Patch survives regression tests on ppc64le-linux-gnu and majority
>>>>> of warnings are gone in profiledbootstrap.
>>>>>
>>>>> Ready for trunk?
>>>>
>>>> I think we want to do that even with no -fprofile-generate because the
>>>> unreachable code otherwise goes into all other IPA passes for no good
>>>> reason.  So perhaps adding it as todo after the early optimization
>>>> metapass?
>>>
>>> That fails due to gcc_assert.
>>> So one needs:
>>>
>>> diff --git a/gcc/passes.c b/gcc/passes.c
>>> index d838d909941..be92a2f3be3 100644
>>> --- a/gcc/passes.c
>>> +++ b/gcc/passes.c
>>> @@ -485,7 +485,7 @@ const pass_data pass_data_all_early_optimizations =
>>>    0, /* properties_provided */
>>>    0, /* properties_destroyed */
>>>    0, /* todo_flags_start */
>>> -  0, /* todo_flags_finish */
>>> +  TODO_remove_functions | TODO_rebuild_cgraph_edges, /* todo_flags_finish */
>>>  };
>>>
>>>  class pass_all_early_optimizations : public gimple_opt_pass
>>> @@ -1989,7 +1989,8 @@ execute_todo (unsigned int flags)
>>>       of IPA pass queue.  */
>>>    if (flags & TODO_remove_functions)
>>>      {
>>> -      gcc_assert (!cfun);
>>> +      gcc_assert (!cfun
>>> +                 || strcmp (current_pass->name, "early_optimizations") == 0);
>>>        symtab->remove_unreachable_nodes (dump_file);
>>>      }
>>>
>>>
>>> Or do you prefer to a new pass_remove_functions pass that will be added after
>>> pass_local_optimization_passes ?
>>
>> Can you make it todo_flags_start of pass_ipa_tree_profile instead?
> 
> It fails because all_early_optimizations are now gimple pass, so it
> should be TODO after pass_local_optimization_passes?

Please read my last email.

Martin

> 
> Honza
>>
>>> Martin
>>>
>>>>
>>>> Honza
>>>>> Thanks,
>>>>> Martin
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2018-11-08  Martin Liska  <mliska@suse.cz>
>>>>>
>>>>>      * tree-profile.c: Run TODO_remove_functions before "profile"
>>>>>      pass in order to remove dead functions that will trigger
>>>>>      -Wmissing-profile.
>>>>> ---
>>>>>  gcc/tree-profile.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>>
>>>>
>>>>> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
>>>>> index d8f2a3b1ba4..c14ebc556a6 100644
>>>>> --- a/gcc/tree-profile.c
>>>>> +++ b/gcc/tree-profile.c
>>>>> @@ -776,7 +776,7 @@ const pass_data pass_data_ipa_tree_profile =
>>>>>    0, /* properties_required */
>>>>>    0, /* properties_provided */
>>>>>    0, /* properties_destroyed */
>>>>> -  0, /* todo_flags_start */
>>>>> +  TODO_remove_functions, /* todo_flags_start */
>>>>>    TODO_dump_symtab, /* todo_flags_finish */
>>>>>  };
>>>>>
>>>>>
>>>>
>>>
Martin Liška Nov. 12, 2018, 9:19 a.m. | #5
On 11/8/18 12:46 PM, Jan Hubicka wrote:
>> On Thu, Nov 8, 2018 at 12:39 PM Martin Liška <mliska@suse.cz> wrote:
>>>
>>> On 11/8/18 12:19 PM, Jan Hubicka wrote:
>>>>> Hi.
>>>>>
>>>>> In order to fix the warnings mentioned in the PR, we need
>>>>> to run remove_unreachable_nodes after early tree passes. That's
>>>>> however possible only within a IPA pass. Thus I'm calling that
>>>>> before the profile PASS.
>>>>>
>>>>> Patch survives regression tests on ppc64le-linux-gnu and majority
>>>>> of warnings are gone in profiledbootstrap.
>>>>>
>>>>> Ready for trunk?
>>>>
>>>> I think we want to do that even with no -fprofile-generate because the
>>>> unreachable code otherwise goes into all other IPA passes for no good
>>>> reason.  So perhaps adding it as todo after the early optimization
>>>> metapass?
>>>
>>> That fails due to gcc_assert.
>>> So one needs:
>>>
>>> diff --git a/gcc/passes.c b/gcc/passes.c
>>> index d838d909941..be92a2f3be3 100644
>>> --- a/gcc/passes.c
>>> +++ b/gcc/passes.c
>>> @@ -485,7 +485,7 @@ const pass_data pass_data_all_early_optimizations =
>>>    0, /* properties_provided */
>>>    0, /* properties_destroyed */
>>>    0, /* todo_flags_start */
>>> -  0, /* todo_flags_finish */
>>> +  TODO_remove_functions | TODO_rebuild_cgraph_edges, /* todo_flags_finish */
>>>  };
>>>
>>>  class pass_all_early_optimizations : public gimple_opt_pass
>>> @@ -1989,7 +1989,8 @@ execute_todo (unsigned int flags)
>>>       of IPA pass queue.  */
>>>    if (flags & TODO_remove_functions)
>>>      {
>>> -      gcc_assert (!cfun);
>>> +      gcc_assert (!cfun
>>> +                 || strcmp (current_pass->name, "early_optimizations") == 0);
>>>        symtab->remove_unreachable_nodes (dump_file);
>>>      }
>>>
>>>
>>> Or do you prefer to a new pass_remove_functions pass that will be added after
>>> pass_local_optimization_passes ?
>>
>> Can you make it todo_flags_start of pass_ipa_tree_profile instead?
> 
> It fails because all_early_optimizations are now gimple pass, so it
> should be TODO after pass_local_optimization_passes?

Unfortunately it does not work. Following file can't be compiled:

./xgcc -B. /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/torture/inline-2.c  -O0 
/usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld: /tmp/ccCVFrPv.o: in function `bar1':
inline-2.c:(.text+0x5): undefined reference to `foo2'
/usr/lib64/gcc/x86_64-suse-linux/8/../../../../x86_64-suse-linux/bin/ld: /tmp/ccCVFrPv.o: in function `bar2':
inline-2.c:(.text+0x11): undefined reference to `foo1'
collect2: error: ld returned 1 exit status

So it's some interference with einline. Honza?

Thus I'm suggesting to add a new IPA pass.

That survives regression tests on x86_64-linux-gnu and bootstrap works.
Martin

> 
> Honza
>>
>>> Martin
>>>
>>>>
>>>> Honza
>>>>> Thanks,
>>>>> Martin
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2018-11-08  Martin Liska  <mliska@suse.cz>
>>>>>
>>>>>      * tree-profile.c: Run TODO_remove_functions before "profile"
>>>>>      pass in order to remove dead functions that will trigger
>>>>>      -Wmissing-profile.
>>>>> ---
>>>>>  gcc/tree-profile.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>>
>>>>
>>>>> diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
>>>>> index d8f2a3b1ba4..c14ebc556a6 100644
>>>>> --- a/gcc/tree-profile.c
>>>>> +++ b/gcc/tree-profile.c
>>>>> @@ -776,7 +776,7 @@ const pass_data pass_data_ipa_tree_profile =
>>>>>    0, /* properties_required */
>>>>>    0, /* properties_provided */
>>>>>    0, /* properties_destroyed */
>>>>> -  0, /* todo_flags_start */
>>>>> +  TODO_remove_functions, /* todo_flags_start */
>>>>>    TODO_dump_symtab, /* todo_flags_finish */
>>>>>  };
>>>>>
>>>>>
>>>>
>>>
From 00fd2e0870860d5e1b4e599e1f88292982a03efb Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 7 Nov 2018 13:10:57 +0100
Subject: [PATCH] Remove unreachable nodes before IPA profile pass (PR
 ipa/87706).

gcc/ChangeLog:

2018-11-12  Martin Liska  <mliska@suse.cz>

	PR ipa/87706
	* cgraphbuild.c (class pass_ipa_remove_functions): New pass.
	(make_pass_remove_functions): Likewise.
	* passes.def: Declare the new pass.
	* tree-pass.h (make_pass_remove_functions): New.
---
 gcc/cgraphbuild.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 gcc/passes.def    |  1 +
 gcc/tree-pass.h   |  1 +
 3 files changed, 46 insertions(+)

diff --git a/gcc/cgraphbuild.c b/gcc/cgraphbuild.c
index c2ad5cf2ef7..f903df38c31 100644
--- a/gcc/cgraphbuild.c
+++ b/gcc/cgraphbuild.c
@@ -547,3 +547,47 @@ make_pass_remove_cgraph_callee_edges (gcc::context *ctxt)
 {
   return new pass_remove_cgraph_callee_edges (ctxt);
 }
+
+namespace {
+
+const pass_data pass_data_ipa_remove_functions =
+{
+  IPA_PASS, /* type */
+  "*remove_functions", /* name */
+  OPTGROUP_INLINE, /* optinfo_flags */
+  TV_IPA_FNSUMMARY, /* tv_id */
+  0, /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  TODO_remove_functions /* todo_flags_finish */
+};
+
+class pass_ipa_remove_functions : public ipa_opt_pass_d
+{
+public:
+  pass_ipa_remove_functions (gcc::context *ctxt)
+    : ipa_opt_pass_d (pass_data_ipa_remove_functions, ctxt,
+		      NULL, /* generate_summary */
+		      NULL, /* write_summary */
+		      NULL, /* read_summary */
+		      NULL, /* write_optimization_summary */
+		      NULL, /* read_optimization_summary */
+		      NULL, /* stmt_fixup */
+		      0, /* function_transform_todo_flags_start */
+		      NULL, /* function_transform */
+		      NULL) /* variable_transform */
+  {}
+
+  /* opt_pass methods: */
+  virtual unsigned int execute (function *) { return 0; }
+
+}; // class pass_ipa_fn_summary
+
+} // anon namespace
+
+ipa_opt_pass_d *
+make_pass_remove_functions (gcc::context *ctxt)
+{
+  return new pass_ipa_remove_functions (ctxt);
+}
diff --git a/gcc/passes.def b/gcc/passes.def
index 24f212c8e31..f7ebc061721 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -105,6 +105,7 @@ along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_rebuild_cgraph_edges);
       NEXT_PASS (pass_local_fn_summary);
   POP_INSERT_PASSES ()
+  NEXT_PASS (pass_remove_functions);
 
   NEXT_PASS (pass_ipa_oacc);
   PUSH_INSERT_PASSES_WITHIN (pass_ipa_oacc)
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index af15adc8e0c..872d3533b49 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -514,6 +514,7 @@ extern ipa_opt_pass_d *make_pass_ipa_single_use (gcc::context *ctxt);
 extern ipa_opt_pass_d *make_pass_ipa_comdats (gcc::context *ctxt);
 extern simple_ipa_opt_pass *make_pass_materialize_all_clones (gcc::context *
 							      ctxt);
+extern ipa_opt_pass_d *make_pass_remove_functions (gcc::context *ctxt);
 
 extern gimple_opt_pass *make_pass_cleanup_cfg_post_optimizing (gcc::context
 							       *ctxt);

Patch

diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index d8f2a3b1ba4..c14ebc556a6 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -776,7 +776,7 @@  const pass_data pass_data_ipa_tree_profile =
   0, /* properties_required */
   0, /* properties_provided */
   0, /* properties_destroyed */
-  0, /* todo_flags_start */
+  TODO_remove_functions, /* todo_flags_start */
   TODO_dump_symtab, /* todo_flags_finish */
 };