diff mbox

gcc::context creation

Message ID 20170605114736.GE2154@tucnak
State New
Headers show

Commit Message

Jakub Jelinek June 5, 2017, 11:47 a.m. UTC 
On Mon, Jun 05, 2017 at 06:57:47AM -0400, Nathan Sidwell wrote:
> On 06/05/2017 05:46 AM, Jakub Jelinek wrote:
> 
> > Here is a patch to implement that.  I chose to keep the 3 dumps predefined
> > dumps and just tweak their registered number, otherwise the changes would be
> > bigger.
> 
> Yeah, that can wait for another day.
> 
> >  The patch additionally renames TDI_generic to TDI_gimple (to match
> > the dump name and content) and because we have more than 256 dumps these
> > days and we register them all even at -O0, the patch avoids 4 unnecessary
> > reallocations.  Ok for trunk if testing passes?
> 
> LGTM, two nits below.  (I can't approve though)
> 
> > +/* Allow languages and middle-end to register their dumps before the
> > +   optimization passes.  */
> blank line needed here, I think?

Whether there is a blank between function and its function comment
is something we aren't consistent in, but it seems that in dumpfile.c
there is a blank line, so I'm adjusting.

> > +void
> > +gcc::dump_manager::
> > +register_dumps ()
> > +{
> > +  lang_hooks.register_dumps (this);
> > +  /* If this assert fails, some FE registered more than
> > +     FIRST_ME_AUTO_NUMBERED_DUMP - FIRST_AUTO_NUMBERED_DUMP
> > +     dump files.  Bump FIRST_ME_AUTO_NUMBERED_DUMP accordingly.  */
> > +  gcc_assert (m_next_dump <= FIRST_ME_AUTO_NUMBERED_DUMP);
> > +  if (m_next_dump < FIRST_ME_AUTO_NUMBERED_DUMP)
> > +    m_next_dump = FIRST_ME_AUTO_NUMBERED_DUMP;
> 
> Unconditionally set it? there's the protecting assert.  It's almost as if
> you want 'else gcc_unreachable ()' (with suitable morph of < to <=).

Unconditional set is fine, else gcc_unreachable (); looks ugly to me.

Here is updated patch:

2017-06-05  Jakub Jelinek  <jakub@redhat.com>

	* dumpfile.h (enum tree_dump_index): Rename TDI_generic to
	TDI_gimple.
	(class dump_manager): Add register_dumps method.
	* dumpfile.c: Include langhooks.h.
	(dump_files): Use 0 instead of 3/4/5 for TDI_{original,gimple,nested}.
	(FIRST_AUTO_NUMBERED_DUMP): Decrease to 1.
	(FIRST_ME_AUTO_NUMBERED_DUMP): Define.
	(dump_manager::dump_register): Start with 512 entries instead of 32.
	(dump_manager::register_dumps): New method.
	* toplev.c (general_init): Instead of invoking register_dumps
	langhook, invoke register_dumps method on the dump manager.
	* gimplify.c (gimplify_function_tree): Use TDI_gimple instead of
	TDI_generic.

	* gimple-parser.c (c_parser_parse_gimple_body): Use TDI_gimple instead
	of TDI_generic.



	Jakub

Comments

Nathan Sidwell June 5, 2017, 12:46 p.m. UTC | #1 
On 06/05/2017 07:47 AM, Jakub Jelinek wrote:

> Whether there is a blank between function and its function comment
> is something we aren't consistent in, but it seems that in dumpfile.c
> there is a blank line, so I'm adjusting.

Ok (I only know the blank line rule because someone nitted one of my 
patches)


> +#define FIRST_AUTO_NUMBERED_DUMP 1
> +#define FIRST_ME_AUTO_NUMBERED_DUMP 3

When you commit, could you set this to 4 (at least).  Right now you have 
no headroom for C++ FE changes.  And I have a branch with a new lang dump :)

nathan
Jakub Jelinek June 5, 2017, 12:50 p.m. UTC | #2 
On Mon, Jun 05, 2017 at 08:46:21AM -0400, Nathan Sidwell wrote:
> On 06/05/2017 07:47 AM, Jakub Jelinek wrote:
> 
> > Whether there is a blank between function and its function comment
> > is something we aren't consistent in, but it seems that in dumpfile.c
> > there is a blank line, so I'm adjusting.
> 
> Ok (I only know the blank line rule because someone nitted one of my
> patches)
> 
> 
> > +#define FIRST_AUTO_NUMBERED_DUMP 1
> > +#define FIRST_ME_AUTO_NUMBERED_DUMP 3
> 
> When you commit, could you set this to 4 (at least).  Right now you have no
> headroom for C++ FE changes.  And I have a branch with a new lang dump :)

It was the intent that there is no unnecessary gap, the difference
between those two should be simply the maximum any FE registers.
So, on your branch you'd bump it to 4 and on trunk when merging your branch.

	Jakub
Nathan Sidwell June 5, 2017, 2:55 p.m. UTC | #3 
On 06/05/2017 08:50 AM, Jakub Jelinek wrote:

> It was the intent that there is no unnecessary gap, the difference
> between those two should be simply the maximum any FE registers.
> So, on your branch you'd bump it to 4 and on trunk when merging your branch.

I can live with that.

nathan
Richard Biener June 6, 2017, 12:38 p.m. UTC | #4 
On Mon, 5 Jun 2017, Nathan Sidwell wrote:

> On 06/05/2017 08:50 AM, Jakub Jelinek wrote:
> 
> > It was the intent that there is no unnecessary gap, the difference
> > between those two should be simply the maximum any FE registers.
> > So, on your branch you'd bump it to 4 and on trunk when merging your branch.
> 
> I can live with that.

Me too, in case you still need approval.

Richard.
diff mbox

Patch

--- gcc/dumpfile.h.jj	2017-06-02 09:01:10.000000000 +0200
+++ gcc/dumpfile.h	2017-06-05 11:20:07.563008969 +0200
@@ -31,7 +31,7 @@  enum tree_dump_index
   TDI_inheritance,		/* dump type inheritance graph.  */
   TDI_clones,			/* dump IPA cloning decisions.  */
   TDI_original,			/* dump each function before optimizing it */
-  TDI_generic,			/* dump each function after genericizing it */
+  TDI_gimple,			/* dump each function after gimplifying it */
   TDI_nested,			/* dump each function after unnesting it */
 
   TDI_lang_all,			/* enable all the language dumps.  */
@@ -212,6 +212,11 @@  public:
   dump_register (const char *suffix, const char *swtch, const char *glob,
 		 dump_kind dkind, int optgroup_flags, bool take_ownership);
 
+  /* Allow languages and middle-end to register their dumps before the
+     optimization passes.  */
+  void
+  register_dumps ();
+
   /* Return the dump_file_info for the given phase.  */
   struct dump_file_info *
   get_dump_file_info (int phase) const;
--- gcc/dumpfile.c.jj	2017-06-02 09:01:10.000000000 +0200
+++ gcc/dumpfile.c	2017-06-05 11:21:27.381013150 +0200
@@ -27,6 +27,7 @@  along with GCC; see the file COPYING3.
 #include "dumpfile.h"
 #include "context.h"
 #include "tree-cfg.h"
+#include "langhooks.h"
 
 /* If non-NULL, return one past-the-end of the matching SUBPART of
    the WHOLE string.  */
@@ -59,10 +60,11 @@  static struct dump_file_info dump_files[
   DUMP_FILE_INFO (".cgraph", "ipa-cgraph", DK_ipa, 0),
   DUMP_FILE_INFO (".type-inheritance", "ipa-type-inheritance", DK_ipa, 0),
   DUMP_FILE_INFO (".ipa-clones", "ipa-clones", DK_ipa, 0),
-  DUMP_FILE_INFO (".original", "tree-original", DK_tree, 3),
-  DUMP_FILE_INFO (".gimple", "tree-gimple", DK_tree, 4),
-  DUMP_FILE_INFO (".nested", "tree-nested", DK_tree, 5),
-#define FIRST_AUTO_NUMBERED_DUMP 3
+  DUMP_FILE_INFO (".original", "tree-original", DK_tree, 0),
+  DUMP_FILE_INFO (".gimple", "tree-gimple", DK_tree, 0),
+  DUMP_FILE_INFO (".nested", "tree-nested", DK_tree, 0),
+#define FIRST_AUTO_NUMBERED_DUMP 1
+#define FIRST_ME_AUTO_NUMBERED_DUMP 3
 
   DUMP_FILE_INFO (NULL, "lang-all", DK_lang, 0),
   DUMP_FILE_INFO (NULL, "tree-all", DK_tree, 0),
@@ -179,7 +181,7 @@  dump_register (const char *suffix, const
   if (count >= m_extra_dump_files_alloced)
     {
       if (m_extra_dump_files_alloced == 0)
-	m_extra_dump_files_alloced = 32;
+	m_extra_dump_files_alloced = 512;
       else
 	m_extra_dump_files_alloced *= 2;
       m_extra_dump_files = XRESIZEVEC (struct dump_file_info,
@@ -200,6 +202,25 @@  dump_register (const char *suffix, const
 }
 
 
+/* Allow languages and middle-end to register their dumps before the
+   optimization passes.  */
+
+void
+gcc::dump_manager::
+register_dumps ()
+{
+  lang_hooks.register_dumps (this);
+  /* If this assert fails, some FE registered more than
+     FIRST_ME_AUTO_NUMBERED_DUMP - FIRST_AUTO_NUMBERED_DUMP
+     dump files.  Bump FIRST_ME_AUTO_NUMBERED_DUMP accordingly.  */
+  gcc_assert (m_next_dump <= FIRST_ME_AUTO_NUMBERED_DUMP);
+  m_next_dump = FIRST_ME_AUTO_NUMBERED_DUMP;
+  dump_files[TDI_original].num = m_next_dump++;
+  dump_files[TDI_gimple].num = m_next_dump++;
+  dump_files[TDI_nested].num = m_next_dump++;
+}
+
+
 /* Return the dump_file_info for the given phase.  */
 
 struct dump_file_info *
--- gcc/toplev.c.jj	2017-06-02 09:01:10.000000000 +0200
+++ gcc/toplev.c	2017-06-05 11:04:04.085038583 +0200
@@ -1168,9 +1168,9 @@  general_init (const char *argv0, bool in
      dump manager.  */
   g = new gcc::context ();
 
-  /* Allow languages to register their dumps before the optimization
-     passes.  */
-  lang_hooks.register_dumps (g->get_dumps ());
+  /* Allow languages and middle-end to register their dumps before the
+     optimization passes.  */
+  g->get_dumps ()->register_dumps ();
 
   /* Create the passes.  */
   g->set_passes (new gcc::pass_manager (g));
--- gcc/gimplify.c.jj	2017-06-02 09:01:20.000000000 +0200
+++ gcc/gimplify.c	2017-06-05 11:21:45.097792114 +0200
@@ -12732,7 +12732,7 @@  gimplify_function_tree (tree fndecl)
 
   pop_cfun ();
 
-  dump_function (TDI_generic, fndecl);
+  dump_function (TDI_gimple, fndecl);
 }
 
 /* Return a dummy expression of type TYPE in order to keep going after an
--- gcc/c/gimple-parser.c.jj	2017-06-02 09:01:07.000000000 +0200
+++ gcc/c/gimple-parser.c	2017-06-05 11:22:04.636548346 +0200
@@ -116,7 +116,7 @@  c_parser_parse_gimple_body (c_parser *pa
      we have to go through lowering again.  */
   cfun->curr_properties = PROP_gimple_any;
 
-  dump_function (TDI_generic, current_function_decl);
+  dump_function (TDI_gimple, current_function_decl);
 }
 
 /* Parse a compound statement in gimple function body.