This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix ICE with -ftree-parallelize-loops=2 -fno-ipa-pure-const (PR ipa/83506)


On December 20, 2017 8:24:04 PM GMT+01:00, Jakub Jelinek <jakub@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  <jakub@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]