diff mbox

[RFC] Do we want hierarchical options & encapsulation in a class

Message ID 5f82cd53-e428-50ce-f64a-18ddfd7dfd27@acm.org
State New
Headers show

Commit Message

Nathan Sidwell May 15, 2017, 11:18 a.m. UTC
On 05/15/2017 05:39 AM, Martin Liška wrote:
> Thanks Martin for feedback! After I spent quite some time with fiddling with
> the options, I'm not convinced we should convert options to more hierarchical

I'll respond to Martin's email properly separates, but while we're on 
dump redesign, here's a WIP patch I whipped up on Friday for the modules 
branch.  This tries to move the language-specific options to a dynamic 
registration mechanism, rather than hard wire them into dumpfile.[hc]. 
There were some awkward pieces due to the current structure of dumpfile 
registration.

I took the -fdump-class-heirachy and -fdump-translation-unit and turned 
them into -fdump-lang-class and -fdump-lang-translation.  Unfortunately 
the current LANG_HOOKS_INIT_OPTIONS is run rather too late to register 
these dumps so that they get nice low numbers.  That's because 
gcc::context::context () both creates the dump manager and then 
immediately creates all the optimization passes:
   m_dumps = new gcc::dump_manager ();
   /* Allow languages to register dumps before passes.  */
   lang_hooks.register_dumps (m_dumps);
   m_passes = new gcc::pass_manager (this);

As you can see i wedged a new lang hook between.  That's a little 
unpleasant -- perhaps
   m_passes = new gcc::pass_manager (this);
should be done later? Or the lang dumps could be unnumbered -- it is 
jarring for them to be numbered as-if succeeding the optimization passes.

(I passed m_dumps as a void * purely to avoid header file jiggery pokery 
at this step)

This patch does allow removing special class_dump_file handling from 
c-family/c-opts.c, which is nice.  It looks like -mdump-tree-original 
might be another candidate for dynamic registration?

thoughts?

nathan

Comments

Martin Liška May 15, 2017, 12:07 p.m. UTC | #1
On 05/15/2017 01:18 PM, Nathan Sidwell wrote:
> On 05/15/2017 05:39 AM, Martin Liška wrote:
>> Thanks Martin for feedback! After I spent quite some time with fiddling with
>> the options, I'm not convinced we should convert options to more hierarchical
> 
> I'll respond to Martin's email properly separates, but while we're on dump redesign, here's a WIP patch I whipped up on Friday for the modules branch.  This tries to move the language-specific options to a dynamic registration mechanism, rather than hard wire them into dumpfile.[hc]. There were some awkward pieces due to the current structure of dumpfile registration.
> 
> I took the -fdump-class-heirachy and -fdump-translation-unit and turned them into -fdump-lang-class and -fdump-lang-translation.  Unfortunately the current LANG_HOOKS_INIT_OPTIONS is run rather too late to register these dumps so that they get nice low numbers.  That's because gcc::context::context () both creates the dump manager and then immediately creates all the optimization passes:
>   m_dumps = new gcc::dump_manager ();
>   /* Allow languages to register dumps before passes.  */
>   lang_hooks.register_dumps (m_dumps);
>   m_passes = new gcc::pass_manager (this);
> 
> As you can see i wedged a new lang hook between.  That's a little unpleasant -- perhaps
>   m_passes = new gcc::pass_manager (this);
> should be done later? Or the lang dumps could be unnumbered -- it is jarring for them to be numbered as-if succeeding the optimization passes.
> 
> (I passed m_dumps as a void * purely to avoid header file jiggery pokery at this step)
> 
> This patch does allow removing special class_dump_file handling from c-family/c-opts.c, which is nice.  It looks like -mdump-tree-original might be another candidate for dynamic registration?
> 
> thoughts?

Hello.

I like the idea and I believe we should do the same with single use (in a particular pass) dump files like:

  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 (".tu", "translation-unit", DK_lang, 1),
  dump_file_info (".class", "class-hierarchy", DK_lang, 2),
  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),

Martin

> 
> nathan
>
diff mbox

Patch

Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c	(revision 247990)
+++ gcc/c-family/c-opts.c	(working copy)
@@ -102,8 +102,6 @@  static size_t include_cursor;
 /* Dump files/flags to use during parsing.  */
 static FILE *original_dump_file = NULL;
 static int original_dump_flags;
-static FILE *class_dump_file = NULL;
-static int class_dump_flags;
 
 /* Whether any standard preincluded header has been preincluded.  */
 static bool done_preinclude;
@@ -1098,10 +1096,9 @@  c_common_parse_file (void)
   for (;;)
     {
       c_finish_options ();
-      /* Open the dump files to use for the original and class dump output
+      /* Open the dump files to use for the original output
          here, to be used during parsing for the current file.  */
       original_dump_file = dump_begin (TDI_original, &original_dump_flags);
-      class_dump_file = dump_begin (TDI_class, &class_dump_flags);
       pch_init ();
       push_file_scope ();
       c_parse_file ();
@@ -1120,11 +1117,6 @@  c_common_parse_file (void)
           dump_end (TDI_original, original_dump_file);
           original_dump_file = NULL;
         }
-      if (class_dump_file)
-        {
-          dump_end (TDI_class, class_dump_file);
-          class_dump_file = NULL;
-        }
       /* If an input file is missing, abandon further compilation.
 	 cpplib has issued a diagnostic.  */
       if (!this_input_filename)
@@ -1138,17 +1130,10 @@  c_common_parse_file (void)
 FILE *
 get_dump_info (int phase, int *flags)
 {
-  gcc_assert (phase == TDI_original || phase == TDI_class);
-  if (phase == TDI_original)
-    {
-      *flags = original_dump_flags;
-      return original_dump_file;
-    }
-  else
-    {
-      *flags = class_dump_flags;
-      return class_dump_file;
-    }
+  gcc_assert (phase == TDI_original);
+  
+  *flags = original_dump_flags;
+  return original_dump_file;
 }
 
 /* Common finish hook for the C, ObjC and C++ front ends.  */
Index: gcc/context.c
===================================================================
--- gcc/context.c	(revision 247990)
+++ gcc/context.c	(working copy)
@@ -24,6 +24,8 @@  along with GCC; see the file COPYING3.
 #include "pass_manager.h"
 #include "dumpfile.h"
 #include "realmpfr.h"
+#include "tree.h"
+#include "langhooks.h"
 
 /* The singleton holder of global state: */
 gcc::context *g;
@@ -36,6 +38,8 @@  gcc::context::context ()
      dumps for the various passes), so the dump manager must be set up
      before the pass manager.  */
   m_dumps = new gcc::dump_manager ();
+  /* Allow languages to reguster dumps before passes.  */
+  lang_hooks.register_dumps (m_dumps);
   m_passes = new gcc::pass_manager (this);
 }
 
Index: gcc/cp/class.c
===================================================================
--- gcc/cp/class.c	(revision 247990)
+++ gcc/cp/class.c	(working copy)
@@ -36,6 +36,10 @@  along with GCC; see the file COPYING3.
 #include "dumpfile.h"
 #include "gimplify.h"
 #include "intl.h"
+#include "context.h"
+
+/* ID for dumping the class hierarchy.  */
+static int class_dump_id;
 
 /* The number of nested classes being processed.  If we are not in the
    scope of any class, this is zero.  */
@@ -7733,6 +7737,13 @@  init_class_processing (void)
   ridpointers[(int) RID_PROTECTED] = access_protected_node;
 }
 
+void
+register_class_dump (void *d)
+{
+  class_dump_id = ((gcc::dump_manager *)d)->dump_register
+    (".class", "lang-class", "lang-class", TDF_LANG, OPTGROUP_NONE, false);
+}
+
 /* Restore the cached PREVIOUS_CLASS_LEVEL.  */
 
 static void
@@ -8916,11 +8927,10 @@  static void
 dump_class_hierarchy (tree t)
 {
   int flags;
-  FILE *stream = get_dump_info (TDI_class, &flags);
-
-  if (stream)
+  if (FILE *d = dump_begin (class_dump_id, &flags))
     {
-      dump_class_hierarchy_1 (stream, flags, t);
+      dump_class_hierarchy_1 (d, flags, t);
+      dump_end (class_dump_id, d);
     }
 }
 
@@ -8950,7 +8960,7 @@  static void
 dump_vtable (tree t, tree binfo, tree vtable)
 {
   int flags;
-  FILE *stream = get_dump_info (TDI_class, &flags);
+  FILE *stream = dump_begin (class_dump_id, &flags);
 
   if (!stream)
     return;
@@ -8973,13 +8983,14 @@  dump_vtable (tree t, tree binfo, tree vt
       dump_array (stream, vtable);
       fprintf (stream, "\n");
     }
+  dump_end (class_dump_id, stream);
 }
 
 static void
 dump_vtt (tree t, tree vtt)
 {
   int flags;
-  FILE *stream = get_dump_info (TDI_class, &flags);
+  FILE *stream = dump_begin (class_dump_id, &flags);
 
   if (!stream)
     return;
@@ -8991,6 +9002,7 @@  dump_vtt (tree t, tree vtt)
       dump_array (stream, vtt);
       fprintf (stream, "\n");
     }
+  dump_end (class_dump_id, stream);
 }
 
 /* Dump a function or thunk and its thunkees.  */
Index: gcc/cp/cp-lang.c
===================================================================
--- gcc/cp/cp-lang.c	(revision 247990)
+++ gcc/cp/cp-lang.c	(working copy)
@@ -35,6 +35,7 @@  static tree cp_eh_personality (void);
 static tree get_template_innermost_arguments_folded (const_tree);
 static tree get_template_argument_pack_elems_folded (const_tree);
 static tree cxx_enum_underlying_base_type (const_tree);
+static void cxx_register_dumps (void *);
 
 /* Lang hooks common to C++ and ObjC++ are declared in cp/cp-objcp-common.h;
    consequently, there should be very few hooks below.  */
@@ -43,11 +44,12 @@  static tree cxx_enum_underlying_base_typ
 #define LANG_HOOKS_NAME "GNU C++"
 #undef LANG_HOOKS_INIT
 #define LANG_HOOKS_INIT cxx_init
+#undef LANG_HOOKS_REGISTER_DUMPS
+#define LANG_HOOKS_REGISTER_DUMPS cxx_register_dumps
 #undef LANG_HOOKS_CLASSIFY_RECORD
 #define LANG_HOOKS_CLASSIFY_RECORD cp_classify_record
 #undef LANG_HOOKS_GENERIC_TYPE_P
 #define LANG_HOOKS_GENERIC_TYPE_P class_tmpl_impl_spec_p
-
 #undef LANG_HOOKS_GET_INNERMOST_GENERIC_PARMS
 #define LANG_HOOKS_GET_INNERMOST_GENERIC_PARMS \
 	get_primary_template_innermost_parameters
@@ -229,6 +231,12 @@  tree cxx_enum_underlying_base_type (cons
   return underlying_type;
 }
 
+static void
+cxx_register_dumps (void *d)
+{
+  register_tu_dump (d);
+  register_class_dump (d);
+}
 
 #include "gt-cp-cp-lang.h"
 #include "gtype-cp.h"
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 247990)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -5973,6 +5973,7 @@  extern tree finish_struct			(tree, tree)
 extern void finish_struct_1			(tree);
 extern int resolves_to_fixed_type_p		(tree, int *);
 extern void init_class_processing		(void);
+extern void register_class_dump			(void *);
 extern int is_empty_class			(tree);
 extern bool is_really_empty_class		(tree);
 extern void pushclass				(tree);
@@ -6223,6 +6224,7 @@  extern int parm_index
 extern tree vtv_start_verification_constructor_init_function (void);
 extern tree vtv_finish_verification_constructor_init_function (tree);
 extern bool cp_omp_mappable_type		(tree);
+extern void register_tu_dump			(void *);
 
 /* in error.c */
 extern const char *type_as_string		(tree, int);
Index: gcc/cp/decl2.c
===================================================================
--- gcc/cp/decl2.c	(revision 247990)
+++ gcc/cp/decl2.c	(working copy)
@@ -4351,6 +4351,17 @@  generate_mangling_aliases ()
   defer_mangling_aliases = false;
 }
 
+/* ID for dumping the raw trees.  */
+static int tu_dump_id;
+
+void
+register_tu_dump (void *d)
+{
+  tu_dump_id = ((gcc::dump_manager *)d)->dump_register
+    (".tu", "lang-translation", "lang-translation",
+     TDF_LANG, OPTGROUP_NONE, false);
+}
+
 /* The entire file is now complete.  If requested, dump everything
    to a file.  */
 
@@ -4358,12 +4369,10 @@  static void
 dump_tu (void)
 {
   int flags;
-  FILE *stream = dump_begin (TDI_tu, &flags);
-
-  if (stream)
+  if (FILE *stream = dump_begin (tu_dump_id, &flags))
     {
       dump_node (global_namespace, flags & ~TDF_SLIM, stream);
-      dump_end (TDI_tu, stream);
+      dump_end (tu_dump_id, stream);
     }
 }
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 247990)
+++ gcc/doc/invoke.texi	(working copy)
@@ -540,14 +540,15 @@  Objective-C and Objective-C++ Dialects}.
 -fdisable-tree-@var{pass_name} @gol
 -fdisable-tree-@var{pass-name}=@var{range-list} @gol
 -fdump-noaddr  -fdump-unnumbered  -fdump-unnumbered-links @gol
--fdump-class-hierarchy@r{[}-@var{n}@r{]} @gol
 -fdump-final-insns@r{[}=@var{file}@r{]}
 -fdump-ipa-all  -fdump-ipa-cgraph  -fdump-ipa-inline @gol
 -fdump-lang-all @gol
+-fdump-lang-@var{switch} @gol
+-fdump-lang-@var{switch}-@var{options} @gol
+-fdump-lang-@var{switch}-@var{options}=@var{filename} @gol
 -fdump-passes @gol
 -fdump-rtl-@var{pass}  -fdump-rtl-@var{pass}=@var{filename} @gol
 -fdump-statistics @gol
--fdump-translation-unit@r{[}-@var{n}@r{]} @gol
 -fdump-tree-all @gol
 -fdump-tree-@var{switch} @gol
 -fdump-tree-@var{switch}-@var{options} @gol
@@ -12944,16 +12945,6 @@  When doing debugging dumps (see @option{
 instruction numbers for the links to the previous and next instructions
 in a sequence.
 
-@item -fdump-class-hierarchy @r{(C++ only)}
-@itemx -fdump-class-hierarchy-@var{options} @r{(C++ only)}
-@opindex fdump-class-hierarchy
-Dump a representation of each class's hierarchy and virtual function
-table layout to a file.  The file name is made by appending
-@file{.class} to the source file name, and the file is created in the
-same directory as the output file.  If the @samp{-@var{options}} form
-is used, @var{options} controls the details of the dump as described
-for the @option{-fdump-tree} options.
-
 @item -fdump-ipa-@var{switch}
 @opindex fdump-ipa
 Control the dumping at various stages of inter-procedural analysis
@@ -12981,8 +12972,19 @@  Dump language-specific information.  The
 @file{.lang} to the source file name.  Currently used for C++ modules.
 
 @item -fdump-lang-all
+@itemx -fdump-lang-@var{switch}
+@itemx -fdump-lang-@var{switch}-@var{options}
+@itemx -fdump-lang-@var{switch}-@var{options}=@var{filename}
 @opindex fdump-lang-all
-Control the dumping of language-specific information.
+@opindex fdump-lang
+Control the dumping of language-specific information.  The file name is
+generated by appending a switch-specific suffix to the source file name,
+and the file is created in the same directory as the output file. In
+case of @option{=@var{filename}} option, the dump is output on the given
+file instead of the auto named dump files.  If the @samp{-@var{options}}
+form is used, @var{options} is a list of @samp{-} separated options
+which control the details of the dump.  Not all options are applicable
+to all dumps; those that are not meaningful are ignored.
 
 @item -fdump-passes
 @opindex fdump-passes
@@ -13000,16 +13002,6 @@  whole compilation unit while @samp{-deta
 the passes generate them.  The default with no option is to sum
 counters for each function compiled.
 
-@item -fdump-translation-unit @r{(C++ only)}
-@itemx -fdump-translation-unit-@var{options} @r{(C++ only)}
-@opindex fdump-translation-unit
-Dump a representation of the tree structure for the entire translation
-unit to a file.  The file name is made by appending @file{.tu} to the
-source file name, and the file is created in the same directory as the
-output file.  If the @samp{-@var{options}} form is used, @var{options}
-controls the details of the dump as described for the
-@option{-fdump-tree} options.
-
 @item -fdump-tree-all
 @itemx -fdump-tree-@var{switch}
 @itemx -fdump-tree-@var{switch}-@var{options}
Index: gcc/dumpfile.c
===================================================================
--- gcc/dumpfile.c	(revision 247990)
+++ gcc/dumpfile.c	(working copy)
@@ -52,7 +52,7 @@  int dump_flags;
 static struct dump_file_info dump_files[TDI_end] =
 {
   {NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, false, false},
-  {".lang", "lang", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
+  {".langg", "langg", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
    OPTGROUP_OTHER, 0, 0, 0, -1, false, false},
   {".cgraph", "ipa-cgraph", NULL, NULL, NULL, NULL, NULL, TDF_IPA,
    0, 0, 0, 0, 0, false, false},
@@ -60,17 +60,13 @@  static struct dump_file_info dump_files[
    0, 0, 0, 0, 0, false, false},
   {".ipa-clones", "ipa-clones", NULL, NULL, NULL, NULL, NULL, TDF_IPA,
    0, 0, 0, 0, 0, false, false},
-  {".tu", "translation-unit", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
-   0, 0, 0, 0, 1, false, false},
-  {".class", "class-hierarchy", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
-   0, 0, 0, 0, 2, false, false},
   {".original", "tree-original", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
    0, 0, 0, 0, 3, false, false},
   {".gimple", "tree-gimple", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
    0, 0, 0, 0, 4, false, false},
   {".nested", "tree-nested", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
    0, 0, 0, 0, 5, false, false},
-#define FIRST_AUTO_NUMBERED_DUMP 6
+#define FIRST_AUTO_NUMBERED_DUMP 4
 
   {NULL, "lang-all", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
    0, 0, 0, 0, 0, false, false},
Index: gcc/dumpfile.h
===================================================================
--- gcc/dumpfile.h	(revision 247990)
+++ gcc/dumpfile.h	(working copy)
@@ -31,8 +31,6 @@  enum tree_dump_index
   TDI_cgraph,                   /* dump function call graph.  */
   TDI_inheritance,              /* dump type inheritance graph.  */
   TDI_clones,			/* dump IPA cloning decisions.  */
-  TDI_tu,			/* dump the whole translation unit.  */
-  TDI_class,			/* dump class hierarchy.  */
   TDI_original,			/* dump each function before optimizing it */
   TDI_generic,			/* dump each function after genericizing it */
   TDI_nested,			/* dump each function after unnesting it */
Index: gcc/langhooks-def.h
===================================================================
--- gcc/langhooks-def.h	(revision 247990)
+++ gcc/langhooks-def.h	(working copy)
@@ -65,6 +65,7 @@  extern tree lhd_enum_underlying_base_typ
 
 /* Declarations of default tree inlining hooks.  */
 extern void lhd_initialize_diagnostics (diagnostic_context *);
+extern void lhd_register_dumps (void *);
 extern void lhd_init_options (unsigned int,
 			      struct cl_decoded_option *);
 extern bool lhd_complain_wrong_lang_p (const struct cl_option *);
@@ -97,6 +98,7 @@  extern int lhd_type_dwarf_attribute (con
 #define LANG_HOOKS_INIT_OPTIONS_STRUCT	hook_void_gcc_optionsp
 #define LANG_HOOKS_INIT_OPTIONS		lhd_init_options
 #define LANG_HOOKS_INITIALIZE_DIAGNOSTICS lhd_initialize_diagnostics
+#define LANG_HOOKS_REGISTER_DUMPS	lhd_register_dumps
 #define LANG_HOOKS_COMPLAIN_WRONG_LANG_P lhd_complain_wrong_lang_p
 #define LANG_HOOKS_HANDLE_OPTION	lhd_handle_option
 #define LANG_HOOKS_POST_OPTIONS		lhd_post_options
@@ -294,6 +296,7 @@  extern void lhd_end_section (void);
   LANG_HOOKS_INIT_OPTIONS_STRUCT, \
   LANG_HOOKS_INIT_OPTIONS, \
   LANG_HOOKS_INITIALIZE_DIAGNOSTICS, \
+  LANG_HOOKS_REGISTER_DUMPS, \
   LANG_HOOKS_COMPLAIN_WRONG_LANG_P, \
   LANG_HOOKS_HANDLE_OPTION, \
   LANG_HOOKS_POST_OPTIONS, \
Index: gcc/langhooks.c
===================================================================
--- gcc/langhooks.c	(revision 247990)
+++ gcc/langhooks.c	(working copy)
@@ -322,6 +322,12 @@  lhd_initialize_diagnostics (diagnostic_c
 {
 }
 
+/* Called to register language-specific dumps.  */
+void
+lhd_register_dumps (void *)
+{
+}
+
 /* Called to perform language-specific options initialization.  */
 void
 lhd_init_options (unsigned int decoded_options_count ATTRIBUTE_UNUSED,
Index: gcc/langhooks.h
===================================================================
--- gcc/langhooks.h	(revision 247990)
+++ gcc/langhooks.h	(working copy)
@@ -326,6 +326,9 @@  struct lang_hooks
      global diagnostic context structure.  */
   void (*initialize_diagnostics) (diagnostic_context *);
 
+  /* Callback to register langage-specific dumps.  */
+  void (*register_dumps) (void *);
+
   /* Return true if a warning should be given about option OPTION,
      which is for the wrong language, false if it should be quietly
      ignored.  */