On December 20, 2017 8:24:04 PM GMT+01:00, Jakub Jelinek <ja...@redhat.com>
wrote:
>Hi!
>
>Prathamesh's r254140 moved the
> if (!flag_wpa)
> ipa_free_fn_summary ();
>from the ipa-inline pass to the following ipa-pure-const pass.
>That doesn't work well though, because ipa-inline pass has
>unconditional
>gate, so is done always when real IPA passes are performed, but
>ipa-pure-const is gated on some flag_* options, so if those options
>are disabled, we keep fn summary computed until end of compilation, and
>e.g. new function insertion hooks then attempt to update the fnsummary
>even when with RTL hooks registered.
>
>Calling ipa_free_fn_summary () in ipa-inline pass if the gate of
>the following pass will not be run looks too ugly to me, and while
>there
>is another unconditional ipa pass, moving it there doesn't look too
>clean to
>me either.
>
>We already have a separate pass ipa-free-fn-summary, just use it so far
>only
>during small IPA passes. I think the cleanest is to add another ipa
>pass
>that will just free it instead of sticking it into unrelated passes
>which
>just happen to be unconditional ATM.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
>2017-12-20 Jakub Jelinek <ja...@redhat.com>
>
> PR ipa/83506
> * ipa-fnsummary.c (pass_data_ipa_free_fn_summary): Use 0 for
> todo_flags_finish.
> (pass_ipa_free_fn_summary): Add small_p private data member,
> initialize to false in the ctor.
> (pass_ipa_free_fn_summary::clone,
> pass_ipa_free_fn_summary::set_pass_param,
> pass_ipa_free_fn_summary::gate): New methods.
> (pass_ipa_free_fn_summary::execute): Return TODO_remove_functions
> | TODO_dump_symtab if small_p.
> * passes.def: Add true parm for the existing pass_ipa_free_fn_summary
> entry and add another instance of the pass with false parm after
> ipa-pure-const.
> * ipa-pure-const.c (pass_ipa_pure_const): Don't call
> ipa_free_fn_summary here.
>
> * gcc.dg/pr83506.c: New test.
> * gcc.dg/ipa/ctor-empty-1.c: Use -fdump-ipa-free-fnsummary1 instead
> of -fdump-ipa-free-fnsummary and scan in free-fnsummary1 instead of
> free-fnsummary dump.
>
>--- gcc/ipa-fnsummary.c.jj 2017-12-20 11:01:13.000000000 +0100
>+++ gcc/ipa-fnsummary.c 2017-12-20 11:43:40.462288141 +0100
>@@ -3538,26 +3538,36 @@ const pass_data pass_data_ipa_free_fn_su
> 0, /* properties_provided */
> 0, /* properties_destroyed */
> 0, /* todo_flags_start */
>- /* Early optimizations may make function unreachable. We can not
>- remove unreachable functions as part of the ealry opts pass
>because
>- TODOs are run before subpasses. Do it here. */
>- ( TODO_remove_functions | TODO_dump_symtab ), /* todo_flags_finish
>*/
>+ 0, /* todo_flags_finish */
> };
>
> class pass_ipa_free_fn_summary : public simple_ipa_opt_pass
> {
> public:
> pass_ipa_free_fn_summary (gcc::context *ctxt)
>- : simple_ipa_opt_pass (pass_data_ipa_free_fn_summary, ctxt)
>+ : simple_ipa_opt_pass (pass_data_ipa_free_fn_summary, ctxt),
>+ small_p (false)
> {}
>
> /* opt_pass methods: */
>+ opt_pass *clone () { return new pass_ipa_free_fn_summary (m_ctxt); }
>+ void set_pass_param (unsigned int n, bool param)
>+ {
>+ gcc_assert (n == 0);
>+ small_p = param;
>+ }
>+ virtual bool gate (function *) { return small_p || !flag_wpa; }
> virtual unsigned int execute (function *)
> {
> ipa_free_fn_summary ();
>- return 0;
>+ /* Early optimizations may make function unreachable. We can
>not
>+ remove unreachable functions as part of the early opts pass because
>+ TODOs are run before subpasses. Do it here. */
>+ return small_p ? TODO_remove_functions | TODO_dump_symtab : 0;
> }
>
>+private:
>+ bool small_p;
> }; // class pass_ipa_free_fn_summary
>
> } // anon namespace
>--- gcc/passes.def.jj 2017-12-18 14:57:20.000000000 +0100
>+++ gcc/passes.def 2017-12-20 11:31:32.402117369 +0100
>@@ -144,7 +144,7 @@ along with GCC; see the file COPYING3.
> PUSH_INSERT_PASSES_WITHIN (pass_ipa_tree_profile)
> NEXT_PASS (pass_feedback_split_functions);
> POP_INSERT_PASSES ()
>- NEXT_PASS (pass_ipa_free_fn_summary);
>+ NEXT_PASS (pass_ipa_free_fn_summary, true /* small_p */);
> NEXT_PASS (pass_ipa_increase_alignment);
> NEXT_PASS (pass_ipa_tm);
> NEXT_PASS (pass_ipa_lower_emutls);
>@@ -161,6 +161,7 @@ along with GCC; see the file COPYING3.
> NEXT_PASS (pass_ipa_fn_summary);
> NEXT_PASS (pass_ipa_inline);
> NEXT_PASS (pass_ipa_pure_const);
>+ NEXT_PASS (pass_ipa_free_fn_summary, false /* small_p */);
> NEXT_PASS (pass_ipa_reference);
> /* This pass needs to be scheduled after any IP code duplication. */
> NEXT_PASS (pass_ipa_single_use);
>--- gcc/ipa-pure-const.c.jj 2017-12-04 22:29:11.000000000 +0100
>+++ gcc/ipa-pure-const.c 2017-12-20 11:33:11.905956408 +0100
>@@ -2013,10 +2013,6 @@ execute (function *)
> if (has_function_state (node))
> free (get_function_state (node));
> funct_state_vec.release ();
>-
>- /* In WPA we use inline summaries for partitioning process. */
>- if (!flag_wpa)
>- ipa_free_fn_summary ();
> return remove_p ? TODO_remove_functions : 0;
> }
>
>--- gcc/testsuite/gcc.dg/pr83506.c.jj 2017-12-20 11:39:48.405212526
>+0100
>+++ gcc/testsuite/gcc.dg/pr83506.c 2017-12-20 11:39:18.000000000 +0100
>@@ -0,0 +1,14 @@
>+/* PR ipa/83506 */
>+/* { dg-do compile { target pthread } } */
>+/* { dg-options "-O1 -ftree-parallelize-loops=2 -fno-ipa-pure-const" }
>*/
>+
>+unsigned int
>+foo (unsigned int x, int y)
>+{
>+ while (y < 1)
>+ {
>+ x *= 3;
>+ ++y;
>+ }
>+ return x;
>+}
>--- gcc/testsuite/gcc.dg/ipa/ctor-empty-1.c.jj 2017-05-24
>11:59:01.000000000 +0200
>+++ gcc/testsuite/gcc.dg/ipa/ctor-empty-1.c 2017-12-20
>20:15:27.144761517 +0100
>@@ -1,7 +1,7 @@
> /* { dg-do compile } */
>-/* { dg-options "-O3 -c -fdump-ipa-free-fnsummary" } */
>+/* { dg-options "-O3 -c -fdump-ipa-free-fnsummary1" } */
> static __attribute__((constructor))
> void empty_constructor()
> {
> }
>-/* { dg-final { scan-ipa-dump "Reclaiming functions:
>empty_constructor" "free-fnsummary" } } */
>+/* { dg-final { scan-ipa-dump "Reclaiming functions:
>empty_constructor" "free-fnsummary1" } } */
>
> Jakub
