Patchwork [2/2] Introduce beginnings of a pipeline class.

login
register
mail settings
Submitter David Malcolm
Date July 24, 2013, 3:09 p.m.
Message ID <1374678544-8678-3-git-send-email-dmalcolm@redhat.com>
Download mbox | patch
Permalink /patch/261435/
State New
Headers show

Comments

David Malcolm - July 24, 2013, 3:09 p.m.
gcc/
	* Makefile.in (PIPELINE_H): New.
	(lto-cgraph.o): Depend on CONTEXT_H and PIPELINE_H.
	(passes.o): Likewise.
	(statistics.o): Likewise.
	(cgraphunit.o): Likewise.
	(context.o): Depend on PIPELINE_H.

	* pipeline.h: New.

	* cgraphunit.c (cgraph_add_new_function): Update for moves
	of globals to fields of pipeline.
	(analyze_function): Likewise.
	(expand_function): Likewise.
	(ipa_passes): Likewise.
	(compile): Likewise.

	* context.c (context::context): New.
	* context.h  (context::context): New.
	(context::get_passes): New.
	(context::passes_): New.

	* lto-cgraph.c (input_node): Update for moves of globals to
	fields of pipeline.

	* passes.c (all_passes): Remove, in favor of a field of the
	same name within the new class pipeline.
	(all_small_ipa_passes): Likewise.
	(all_lowering_passes): Likewise.
	(all_regular_ipa_passes): Likewise.
	(all_late_ipa_passes): Likewise.
	(all_lto_gen_passes): Likewise.
	(passes_by_id): Likewise.
	(passes_by_id_size): Likewise.
	(gcc_pass_lists): Remove, in favor of "pass_lists" field within
	the new class pipeline.
	(set_pass_for_id): Convert to...
	(pipeline::set_pass_for_id): ...method.
	(get_pass_for_id): Convert to...
	(pipeline::get_pass_for_id): ...method.
	(register_one_dump_file): Move body of implementation into...
	(pipeline::register_one_dump_file): ...here.
	(register_dump_files_1): Convert to...
	(pipeline::register_dump_files_1): ...method.
	(register_dump_files): Convert to...
	(pipeline::register_dump_files): ...method.
	(create_pass_tab): Update for moves of globals to fields of
	pipeline.
	(dump_passes): Move body of implementation into...
	(pipeline::dump_passes): ...here.
	(register_pass): Move body of implementation into...
	(pipeline::register_pass): ...here.
	(init_optimization_passes): Convert into...
	(pipeline::pipeline): ...constructor for new pipeline class, and
	initialize the pass_lists array.
	(check_profile_consistency): Update for moves of globals to
	fields of pipeline.
	(dump_profile_report): Move body of implementation into...
	(pipeline::dump_profile_report): ...here.
	(ipa_write_summaries_1): Update for moves of pass lists from
	being globals to fields of pipeline.
	(ipa_write_optimization_summaries): Likewise.
	(ipa_read_summaries):  Likewise.
	(ipa_read_optimization_summaries): Likewise.
	(execute_all_ipa_stmt_fixups): Likewise.

	* statistics.c (statistics_fini): Update for moves of globals to
	fields of pipeline.

	* toplev.c (general_init): Replace call to
	init_optimization_passes with construction of the pipeline
	instance.

	* tree-pass.h (all_passes): Remove, in favor of a field of the
	same name within the new class pipeline.
	(all_small_ipa_passes): Likewise.
	(all_lowering_passes): Likewise.
	(all_regular_ipa_passes): Likewise.
	(all_lto_gen_passes): Likewise.
	(all_late_ipa_passes): Likewise.
	(passes_by_id): Likewise.
	(passes_by_id_size): Likewise.
	(gcc_pass_lists): Remove, in favor of "pass_lists" field within
	the new class pipeline.
	(get_pass_for_id): Remove.

gcc/lto/

	* Make-lang.in (lto/lto.o:): Depend on CONTEXT_H and
	PIPELINE_H.

	* lto.c (do_whole_program_analysis): Update for move of
	all_regular_ipa_passes from a global to a field of class
	pipeline.
---
 gcc/Makefile.in      | 15 +++++---
 gcc/cgraphunit.c     | 22 +++++++-----
 gcc/context.c        |  6 ++++
 gcc/context.h        | 11 +++++-
 gcc/lto-cgraph.c     |  7 ++--
 gcc/lto/Make-lang.in |  3 +-
 gcc/lto/lto.c        |  4 ++-
 gcc/passes.c         | 97 +++++++++++++++++++++++++++++++++++-----------------
 gcc/pipeline.h       | 89 +++++++++++++++++++++++++++++++++++++++++++++++
 gcc/statistics.c     |  7 ++--
 gcc/toplev.c         |  4 +--
 gcc/tree-pass.h      | 29 ----------------
 12 files changed, 212 insertions(+), 82 deletions(-)
 create mode 100644 gcc/pipeline.h
Diego Novillo - July 24, 2013, 10:56 p.m.
Could you please add a description of what this does?

On Wed, Jul 24, 2013 at 11:09 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> gcc/
>         * Makefile.in (PIPELINE_H): New.
>         (lto-cgraph.o): Depend on CONTEXT_H and PIPELINE_H.
>         (passes.o): Likewise.
>         (statistics.o): Likewise.
>         (cgraphunit.o): Likewise.
>         (context.o): Depend on PIPELINE_H.
>
>         * pipeline.h: New.
>
>         * cgraphunit.c (cgraph_add_new_function): Update for moves
>         of globals to fields of pipeline.
>         (analyze_function): Likewise.
>         (expand_function): Likewise.
>         (ipa_passes): Likewise.
>         (compile): Likewise.
>
>         * context.c (context::context): New.
>         * context.h  (context::context): New.
>         (context::get_passes): New.
>         (context::passes_): New.
>
>         * lto-cgraph.c (input_node): Update for moves of globals to
>         fields of pipeline.
>
>         * passes.c (all_passes): Remove, in favor of a field of the
>         same name within the new class pipeline.
>         (all_small_ipa_passes): Likewise.
>         (all_lowering_passes): Likewise.
>         (all_regular_ipa_passes): Likewise.
>         (all_late_ipa_passes): Likewise.
>         (all_lto_gen_passes): Likewise.
>         (passes_by_id): Likewise.
>         (passes_by_id_size): Likewise.
>         (gcc_pass_lists): Remove, in favor of "pass_lists" field within
>         the new class pipeline.
>         (set_pass_for_id): Convert to...
>         (pipeline::set_pass_for_id): ...method.
>         (get_pass_for_id): Convert to...
>         (pipeline::get_pass_for_id): ...method.
>         (register_one_dump_file): Move body of implementation into...
>         (pipeline::register_one_dump_file): ...here.
>         (register_dump_files_1): Convert to...
>         (pipeline::register_dump_files_1): ...method.
>         (register_dump_files): Convert to...
>         (pipeline::register_dump_files): ...method.
>         (create_pass_tab): Update for moves of globals to fields of
>         pipeline.
>         (dump_passes): Move body of implementation into...
>         (pipeline::dump_passes): ...here.
>         (register_pass): Move body of implementation into...
>         (pipeline::register_pass): ...here.
>         (init_optimization_passes): Convert into...
>         (pipeline::pipeline): ...constructor for new pipeline class, and
>         initialize the pass_lists array.
>         (check_profile_consistency): Update for moves of globals to
>         fields of pipeline.
>         (dump_profile_report): Move body of implementation into...
>         (pipeline::dump_profile_report): ...here.
>         (ipa_write_summaries_1): Update for moves of pass lists from
>         being globals to fields of pipeline.
>         (ipa_write_optimization_summaries): Likewise.
>         (ipa_read_summaries):  Likewise.
>         (ipa_read_optimization_summaries): Likewise.
>         (execute_all_ipa_stmt_fixups): Likewise.
>
>         * statistics.c (statistics_fini): Update for moves of globals to
>         fields of pipeline.
>
>         * toplev.c (general_init): Replace call to
>         init_optimization_passes with construction of the pipeline
>         instance.
>
>         * tree-pass.h (all_passes): Remove, in favor of a field of the
>         same name within the new class pipeline.
>         (all_small_ipa_passes): Likewise.
>         (all_lowering_passes): Likewise.
>         (all_regular_ipa_passes): Likewise.
>         (all_lto_gen_passes): Likewise.
>         (all_late_ipa_passes): Likewise.
>         (passes_by_id): Likewise.
>         (passes_by_id_size): Likewise.
>         (gcc_pass_lists): Remove, in favor of "pass_lists" field within
>         the new class pipeline.
>         (get_pass_for_id): Remove.
>
> gcc/lto/
>
>         * Make-lang.in (lto/lto.o:): Depend on CONTEXT_H and
>         PIPELINE_H.
>
>         * lto.c (do_whole_program_analysis): Update for move of
>         all_regular_ipa_passes from a global to a field of class
>         pipeline.
> ---
>  gcc/Makefile.in      | 15 +++++---
>  gcc/cgraphunit.c     | 22 +++++++-----
>  gcc/context.c        |  6 ++++
>  gcc/context.h        | 11 +++++-
>  gcc/lto-cgraph.c     |  7 ++--
>  gcc/lto/Make-lang.in |  3 +-
>  gcc/lto/lto.c        |  4 ++-
>  gcc/passes.c         | 97 +++++++++++++++++++++++++++++++++++-----------------
>  gcc/pipeline.h       | 89 +++++++++++++++++++++++++++++++++++++++++++++++
>  gcc/statistics.c     |  7 ++--
>  gcc/toplev.c         |  4 +--
>  gcc/tree-pass.h      | 29 ----------------
>  12 files changed, 212 insertions(+), 82 deletions(-)
>  create mode 100644 gcc/pipeline.h
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index fb0cb4b..0b28c2d 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -987,6 +987,7 @@ PLUGIN_VERSION_H = plugin-version.h configargs.h
>  LIBFUNCS_H = libfuncs.h $(HASHTAB_H)
>  GRAPHITE_HTAB_H = graphite-htab.h graphite-clast-to-gimple.h $(HASH_TABLE_H)
>  CONTEXT_H = context.h
> +PIPELINE_H = pipeline.h
>
>  #
>  # Now figure out from those variables how to compile and link.
> @@ -2183,7 +2184,8 @@ lto-cgraph.o: lto-cgraph.c $(CONFIG_H) $(SYSTEM_H) coretypes.h   \
>     $(HASHTAB_H) langhooks.h $(BASIC_BLOCK_H) \
>     $(TREE_FLOW_H) $(CGRAPH_H) $(FUNCTION_H) $(GGC_H) $(DIAGNOSTIC_CORE_H) \
>     $(EXCEPT_H) $(TIMEVAR_H) pointer-set.h $(LTO_STREAMER_H) \
> -   $(GCOV_IO_H) $(DATA_STREAMER_H) $(TREE_STREAMER_H) $(TREE_PASS_H) profile.h
> +   $(GCOV_IO_H) $(DATA_STREAMER_H) $(TREE_STREAMER_H) $(TREE_PASS_H) \
> +   profile.h $(CONTEXT_H) $(PIPELINE_H)
>  lto-streamer-in.o: lto-streamer-in.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
>     $(TM_H) toplev.h $(DIAGNOSTIC_CORE_H) $(EXPR_H) $(FLAGS_H) $(PARAMS_H) \
>     input.h $(HASHTAB_H) $(BASIC_BLOCK_H) $(TREE_FLOW_H) $(TREE_PASS_H) \
> @@ -2745,7 +2747,8 @@ passes.o : passes.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
>     hosthooks.h $(CGRAPH_H) $(COVERAGE_H) $(TREE_PASS_H) $(TREE_DUMP_H) \
>     $(GGC_H) $(OPTS_H) $(TREE_FLOW_H) $(TREE_INLINE_H) \
>     gt-passes.h $(DF_H) $(PREDICT_H) $(LTO_STREAMER_H) \
> -   $(PLUGIN_H) $(IPA_UTILS_H) passes.def
> +   $(PLUGIN_H) $(IPA_UTILS_H) passes.def \
> +   $(CONTEXT_H) $(PIPELINE_H)
>
>  plugin.o : plugin.c $(PLUGIN_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h \
>     $(HASH_TABLE_H) $(DIAGNOSTIC_CORE_H) $(TREE_H) $(TREE_PASS_H) \
> @@ -2786,7 +2789,8 @@ function.o : function.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_ERROR_
>     $(TREE_PASS_H) $(DF_H) $(PARAMS_H) bb-reorder.h \
>     $(COMMON_TARGET_H)
>  statistics.o : statistics.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> -   $(TREE_PASS_H) $(TREE_DUMP_H) $(HASH_TABLE_H) statistics.h $(FUNCTION_H)
> +   $(TREE_PASS_H) $(TREE_DUMP_H) $(HASH_TABLE_H) statistics.h \
> +   $(FUNCTION_H) $(CONTEXT_H) $(PIPELINE_H)
>  stmt.o : stmt.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(DUMPFILE_H) $(TM_H) \
>     $(RTL_H) \
>     $(TREE_H) $(FLAGS_H) $(FUNCTION_H) insn-config.h hard-reg-set.h $(EXPR_H) \
> @@ -2908,7 +2912,8 @@ cgraphunit.o : cgraphunit.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
>     $(FIBHEAP_H) output.h $(PARAMS_H) $(RTL_H) $(IPA_PROP_H) \
>     gt-cgraphunit.h tree-iterator.h $(COVERAGE_H) $(TREE_DUMP_H) \
>     $(GIMPLE_PRETTY_PRINT_H) $(IPA_INLINE_H) $(IPA_UTILS_H) $(CFGLOOP_H) \
> -   $(LTO_STREAMER_H) output.h $(REGSET_H) $(EXCEPT_H) $(GCC_PLUGIN_H) plugin.h
> +   $(LTO_STREAMER_H) output.h $(REGSET_H) $(EXCEPT_H) $(GCC_PLUGIN_H) \
> +   plugin.h $(CONTEXT_H) $(PIPELINE_H)
>  cgraphclones.o : cgraphclones.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
>     $(TREE_H) langhooks.h $(TREE_INLINE_H) toplev.h $(DIAGNOSTIC_CORE_H) $(FLAGS_H) $(GGC_H) \
>     $(TARGET_H) $(CGRAPH_H) intl.h pointer-set.h $(FUNCTION_H) $(GIMPLE_H) \
> @@ -3490,7 +3495,7 @@ $(out_object_file): $(out_file) $(CONFIG_H) coretypes.h $(TM_H) $(TREE_H) \
>         $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
>                 $(out_file) $(OUTPUT_OPTION)
>  context.o: context.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(GGC_H) \
> -   $(CONTEXT_H)
> +   $(CONTEXT_H) $(PIPELINE_H)
>
>  $(common_out_object_file): $(common_out_file) $(CONFIG_H) $(SYSTEM_H) \
>      coretypes.h $(COMMON_TARGET_H) $(COMMON_TARGET_DEF_H) $(PARAMS_H) \
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index b82c2e0..dc489fb 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -194,6 +194,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "except.h"
>  #include "cfgloop.h"
>  #include "regset.h"     /* FIXME: For reg_obstack.  */
> +#include "context.h"
> +#include "pipeline.h"
>
>  /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
>     secondary queue used during optimization to accommodate passes that
> @@ -478,6 +480,7 @@ cgraph_finalize_function (tree decl, bool nested)
>  void
>  cgraph_add_new_function (tree fndecl, bool lowered)
>  {
> +  gcc::pipeline &passes = g->get_passes ();
>    struct cgraph_node *node;
>    switch (cgraph_state)
>      {
> @@ -508,7 +511,7 @@ cgraph_add_new_function (tree fndecl, bool lowered)
>             push_cfun (DECL_STRUCT_FUNCTION (fndecl));
>             gimple_register_cfg_hooks ();
>             bitmap_obstack_initialize (NULL);
> -           execute_pass_list (all_lowering_passes);
> +           execute_pass_list (passes.all_lowering_passes);
>             execute_pass_list (pass_early_local_passes.pass.sub);
>             bitmap_obstack_release (NULL);
>             pop_cfun ();
> @@ -640,7 +643,7 @@ analyze_function (struct cgraph_node *node)
>
>           gimple_register_cfg_hooks ();
>           bitmap_obstack_initialize (NULL);
> -         execute_pass_list (all_lowering_passes);
> +         execute_pass_list (g->get_passes ().all_lowering_passes);
>           free_dominance_info (CDI_POST_DOMINATORS);
>           free_dominance_info (CDI_DOMINATORS);
>           compact_blocks ();
> @@ -1588,7 +1591,7 @@ expand_function (struct cgraph_node *node)
>    /* Signal the start of passes.  */
>    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
>
> -  execute_pass_list (all_passes);
> +  execute_pass_list (g->get_passes ().all_passes);
>
>    /* Signal the end of passes.  */
>    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_END, NULL);
> @@ -1807,6 +1810,8 @@ output_in_order (void)
>  static void
>  ipa_passes (void)
>  {
> +  gcc::pipeline &passes = g->get_passes ();
> +
>    set_cfun (NULL);
>    current_function_decl = NULL;
>    gimple_register_cfg_hooks ();
> @@ -1816,7 +1821,7 @@ ipa_passes (void)
>
>    if (!in_lto_p)
>      {
> -      execute_ipa_pass_list (all_small_ipa_passes);
> +      execute_ipa_pass_list (passes.all_small_ipa_passes);
>        if (seen_error ())
>         return;
>      }
> @@ -1843,14 +1848,15 @@ ipa_passes (void)
>        cgraph_process_new_functions ();
>
>        execute_ipa_summary_passes
> -       ((struct ipa_opt_pass_d *) all_regular_ipa_passes);
> +       ((struct ipa_opt_pass_d *) passes.all_regular_ipa_passes);
>      }
>
>    /* Some targets need to handle LTO assembler output specially.  */
>    if (flag_generate_lto)
>      targetm.asm_out.lto_start ();
>
> -  execute_ipa_summary_passes ((struct ipa_opt_pass_d *) all_lto_gen_passes);
> +  execute_ipa_summary_passes ((struct ipa_opt_pass_d *)
> +                             passes.all_lto_gen_passes);
>
>    if (!in_lto_p)
>      ipa_write_summaries ();
> @@ -1859,7 +1865,7 @@ ipa_passes (void)
>      targetm.asm_out.lto_end ();
>
>    if (!flag_ltrans && (in_lto_p || !flag_lto || flag_fat_lto_objects))
> -    execute_ipa_pass_list (all_regular_ipa_passes);
> +    execute_ipa_pass_list (passes.all_regular_ipa_passes);
>    invoke_plugin_callbacks (PLUGIN_ALL_IPA_PASSES_END, NULL);
>
>    bitmap_obstack_release (NULL);
> @@ -1985,7 +1991,7 @@ compile (void)
>
>    cgraph_materialize_all_clones ();
>    bitmap_obstack_initialize (NULL);
> -  execute_ipa_pass_list (all_late_ipa_passes);
> +  execute_ipa_pass_list (g->get_passes ().all_late_ipa_passes);
>    symtab_remove_unreachable_nodes (true, dump_file);
>  #ifdef ENABLE_CHECKING
>    verify_symtab ();
> diff --git a/gcc/context.c b/gcc/context.c
> index 76e0dde..8ec2e60 100644
> --- a/gcc/context.c
> +++ b/gcc/context.c
> @@ -22,6 +22,12 @@ along with GCC; see the file COPYING3.  If not see
>  #include "coretypes.h"
>  #include "ggc.h"
>  #include "context.h"
> +#include "pipeline.h"
>
>  /* The singleton holder of global state: */
>  gcc::context *g;
> +
> +gcc::context::context()
> +{
> +  passes_ = new gcc::pipeline(this);
> +}
> diff --git a/gcc/context.h b/gcc/context.h
> index 3caf02f..a83f7b2 100644
> --- a/gcc/context.h
> +++ b/gcc/context.h
> @@ -22,14 +22,23 @@ along with GCC; see the file COPYING3.  If not see
>
>  namespace gcc {
>
> +class pipeline;
> +
>  /* GCC's internal state can be divided into zero or more
>     "parallel universe" of state; an instance of this class is one such
>     context of state.  */
>  class context
>  {
>  public:
> +  context();
> +
> +  /* Pass-management.  */
> +
> +  pipeline &get_passes () { gcc_assert (passes_); return *passes_; }
>
> -  /* Currently empty.  */
> +private:
> +  /* Pass-management.  */
> +  pipeline *passes_;
>
>  }; // class context
>
> diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> index 4a287f6..d322d9a 100644
> --- a/gcc/lto-cgraph.c
> +++ b/gcc/lto-cgraph.c
> @@ -47,6 +47,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "gcov-io.h"
>  #include "tree-pass.h"
>  #include "profile.h"
> +#include "context.h"
> +#include "pipeline.h"
>
>  static void output_cgraph_opt_summary (void);
>  static void input_cgraph_opt_summary (vec<symtab_node>  nodes);
> @@ -936,6 +938,7 @@ input_node (struct lto_file_decl_data *file_data,
>             enum LTO_symtab_tags tag,
>             vec<symtab_node> nodes)
>  {
> +  gcc::pipeline &passes = g->get_passes ();
>    tree fn_decl;
>    struct cgraph_node *node;
>    struct bitpack_d bp;
> @@ -981,8 +984,8 @@ input_node (struct lto_file_decl_data *file_data,
>        struct opt_pass *pass;
>        int pid = streamer_read_hwi (ib);
>
> -      gcc_assert (pid < passes_by_id_size);
> -      pass = passes_by_id[pid];
> +      gcc_assert (pid < passes.passes_by_id_size);
> +      pass = passes.passes_by_id[pid];
>        node->ipa_transforms_to_apply.safe_push ((struct ipa_opt_pass_d *) pass);
>      }
>
> diff --git a/gcc/lto/Make-lang.in b/gcc/lto/Make-lang.in
> index 5f2f475..1acd176 100644
> --- a/gcc/lto/Make-lang.in
> +++ b/gcc/lto/Make-lang.in
> @@ -85,7 +85,8 @@ lto/lto.o: lto/lto.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(OPTS_H) \
>         langhooks.h $(VEC_H) $(BITMAP_H) pointer-set.h $(IPA_PROP_H) \
>         $(COMMON_H) debug.h $(GIMPLE_H) $(LTO_H) $(LTO_TREE_H) \
>         $(LTO_TAGS_H) $(LTO_STREAMER_H) $(SPLAY_TREE_H) gt-lto-lto.h \
> -       $(TREE_STREAMER_H) $(DATA_STREAMER_H) lto/lto-partition.h
> +       $(TREE_STREAMER_H) $(DATA_STREAMER_H) lto/lto-partition.h \
> +       $(CONTEXT_H) $(PIPELINE_H)
>  lto/lto-partition.o: lto/lto-partition.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
>         toplev.h $(TREE_H) $(TM_H) \
>         $(CGRAPH_H) $(TIMEVAR_H) \
> diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
> index c0f9328..e7ca937 100644
> --- a/gcc/lto/lto.c
> +++ b/gcc/lto/lto.c
> @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "splay-tree.h"
>  #include "lto-partition.h"
>  #include "data-streamer.h"
> +#include "context.h"
> +#include "pipeline.h"
>
>  static GTY(()) tree first_personality_decl;
>
> @@ -3694,7 +3696,7 @@ do_whole_program_analysis (void)
>    bitmap_obstack_initialize (NULL);
>    cgraph_state = CGRAPH_STATE_IPA_SSA;
>
> -  execute_ipa_pass_list (all_regular_ipa_passes);
> +  execute_ipa_pass_list (g->get_passes ().all_regular_ipa_passes);
>    symtab_remove_unreachable_nodes (false, dump_file);
>
>    if (cgraph_dump_file)
> diff --git a/gcc/passes.c b/gcc/passes.c
> index 94fb586..e625bf2 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -70,6 +70,10 @@ along with GCC; see the file COPYING3.  If not see
>  #include "plugin.h"
>  #include "ipa-utils.h"
>  #include "tree-pretty-print.h" /* for dump_function_header */
> +#include "context.h"
> +#include "pipeline.h"
> +
> +using namespace gcc;
>
>  /* This is used for debugging.  It allows the current pass to printed
>     from anywhere in compilation.
> @@ -439,23 +443,11 @@ static struct rtl_opt_pass pass_postreload =
>
>
>
> -/* The root of the compilation pass tree, once constructed.  */
> -struct opt_pass *all_passes, *all_small_ipa_passes, *all_lowering_passes,
> -  *all_regular_ipa_passes, *all_late_ipa_passes, *all_lto_gen_passes;
> -
> -/* This is used by plugins, and should also be used in register_pass.  */
> -#define DEF_PASS_LIST(LIST) &LIST,
> -struct opt_pass **gcc_pass_lists[] = { GCC_PASS_LISTS NULL };
> -#undef DEF_PASS_LIST
> -
> -/* A map from static pass id to optimization pass.  */
> -struct opt_pass **passes_by_id;
> -int passes_by_id_size;
> -
>  /* Set the static pass number of pass PASS to ID and record that
>     in the mapping from static pass number to pass.  */
>
> -static void
> +void
> +pipeline::
>  set_pass_for_id (int id, struct opt_pass *pass)
>  {
>    pass->static_pass_number = id;
> @@ -472,7 +464,7 @@ set_pass_for_id (int id, struct opt_pass *pass)
>  /* Return the pass with the static pass number ID.  */
>
>  struct opt_pass *
> -get_pass_for_id (int id)
> +pipeline::get_pass_for_id (int id) const
>  {
>    if (id >= passes_by_id_size)
>      return NULL;
> @@ -486,6 +478,12 @@ get_pass_for_id (int id)
>  void
>  register_one_dump_file (struct opt_pass *pass)
>  {
> +  g->get_passes ().register_one_dump_file (pass);
> +}
> +
> +void
> +pipeline::register_one_dump_file (struct opt_pass *pass)
> +{
>    char *dot_name, *flag_name, *glob_name;
>    const char *name, *full_name, *prefix;
>    char num[10];
> @@ -535,7 +533,8 @@ register_one_dump_file (struct opt_pass *pass)
>
>  /* Recursive worker function for register_dump_files.  */
>
> -static int
> +int
> +pipeline::
>  register_dump_files_1 (struct opt_pass *pass, int properties)
>  {
>    do
> @@ -567,7 +566,8 @@ register_dump_files_1 (struct opt_pass *pass, int properties)
>     PROPERTIES reflects the properties that are guaranteed to be available at
>     the beginning of the pipeline.  */
>
> -static void
> +void
> +pipeline::
>  register_dump_files (struct opt_pass *pass,int properties)
>  {
>    pass->properties_required |= properties;
> @@ -663,7 +663,7 @@ create_pass_tab (void)
>    if (!flag_dump_passes)
>      return;
>
> -  pass_tab.safe_grow_cleared (passes_by_id_size + 1);
> +  pass_tab.safe_grow_cleared (g->get_passes ().passes_by_id_size + 1);
>    name_to_pass_map.traverse <void *, passes_pass_traverse> (NULL);
>  }
>
> @@ -714,6 +714,12 @@ dump_pass_list (struct opt_pass *pass, int indent)
>  void
>  dump_passes (void)
>  {
> +  g->get_passes ().dump_passes ();
> +}
> +
> +void
> +pipeline::dump_passes () const
> +{
>    struct cgraph_node *n, *node = NULL;
>
>    create_pass_tab();
> @@ -1188,6 +1194,13 @@ position_pass (struct register_pass_info *new_pass_info,
>  void
>  register_pass (struct register_pass_info *pass_info)
>  {
> +  g->get_passes ().register_pass (pass_info);
> +
> +}
> +
> +void
> +pipeline::register_pass (struct register_pass_info *pass_info)
> +{
>    bool all_instances, success;
>
>    /* The checks below could fail in buggy plugins.  Existing GCC
> @@ -1277,11 +1290,21 @@ register_pass (struct register_pass_info *pass_info)
>                                         -> all_passes
>  */
>
> -void
> -init_optimization_passes (void)
> +pipeline::pipeline (context *ctxt)
> +: all_passes(NULL), all_small_ipa_passes(NULL), all_lowering_passes(NULL),
> +  all_regular_ipa_passes(NULL), all_lto_gen_passes(NULL),
> +  all_late_ipa_passes(NULL), passes_by_id(NULL), passes_by_id_size(0),
> +  ctxt_(ctxt)
>  {
>    struct opt_pass **p;
>
> +  /* Initialize the pass_lists array.  */
> +#define DEF_PASS_LIST(LIST) pass_lists[PASS_LIST_NO_##LIST] = &LIST;
> +  GCC_PASS_LISTS
> +#undef DEF_PASS_LIST
> +
> +  /* Build the tree of passes.  */
> +
>  #define INSERT_PASSES_AFTER(PASS) \
>    p = &(PASS);
>
> @@ -1432,12 +1455,13 @@ static struct profile_record *profile_record;
>  static void
>  check_profile_consistency (int index, int subpass, bool run)
>  {
> +  pipeline &passes = g->get_passes ();
>    if (index == -1)
>      return;
>    if (!profile_record)
>      profile_record = XCNEWVEC (struct profile_record,
> -                              passes_by_id_size);
> -  gcc_assert (index < passes_by_id_size && index >= 0);
> +                              passes.passes_by_id_size);
> +  gcc_assert (index < passes.passes_by_id_size && index >= 0);
>    gcc_assert (subpass < 2);
>    profile_record[index].run |= run;
>    account_profile_record (&profile_record[index], subpass);
> @@ -1448,6 +1472,12 @@ check_profile_consistency (int index, int subpass, bool run)
>  void
>  dump_profile_report (void)
>  {
> +  g->get_passes ().dump_profile_report ();
> +}
> +
> +void
> +pipeline::dump_profile_report () const
> +{
>    int i, j;
>    int last_freq_in = 0, last_count_in = 0, last_freq_out = 0, last_count_out = 0;
>    gcov_type last_time = 0, last_size = 0;
> @@ -2067,14 +2097,15 @@ ipa_write_summaries_2 (struct opt_pass *pass, struct lto_out_decl_state *state)
>  static void
>  ipa_write_summaries_1 (lto_symtab_encoder_t encoder)
>  {
> +  pipeline &passes = g->get_passes ();
>    struct lto_out_decl_state *state = lto_new_out_decl_state ();
>    state->symtab_node_encoder = encoder;
>
>    lto_push_out_decl_state (state);
>
>    gcc_assert (!flag_wpa);
> -  ipa_write_summaries_2 (all_regular_ipa_passes, state);
> -  ipa_write_summaries_2 (all_lto_gen_passes, state);
> +  ipa_write_summaries_2 (passes.all_regular_ipa_passes, state);
> +  ipa_write_summaries_2 (passes.all_lto_gen_passes, state);
>
>    gcc_assert (lto_get_out_decl_state () == state);
>    lto_pop_out_decl_state ();
> @@ -2205,8 +2236,9 @@ ipa_write_optimization_summaries (lto_symtab_encoder_t encoder)
>      }
>
>    gcc_assert (flag_wpa);
> -  ipa_write_optimization_summaries_1 (all_regular_ipa_passes, state);
> -  ipa_write_optimization_summaries_1 (all_lto_gen_passes, state);
> +  pipeline &passes = g->get_passes ();
> +  ipa_write_optimization_summaries_1 (passes.all_regular_ipa_passes, state);
> +  ipa_write_optimization_summaries_1 (passes.all_lto_gen_passes, state);
>
>    gcc_assert (lto_get_out_decl_state () == state);
>    lto_pop_out_decl_state ();
> @@ -2259,8 +2291,9 @@ ipa_read_summaries_1 (struct opt_pass *pass)
>  void
>  ipa_read_summaries (void)
>  {
> -  ipa_read_summaries_1 (all_regular_ipa_passes);
> -  ipa_read_summaries_1 (all_lto_gen_passes);
> +  pipeline &passes = g->get_passes ();
> +  ipa_read_summaries_1 (passes.all_regular_ipa_passes);
> +  ipa_read_summaries_1 (passes.all_lto_gen_passes);
>  }
>
>  /* Same as execute_pass_list but assume that subpasses of IPA passes
> @@ -2308,8 +2341,9 @@ ipa_read_optimization_summaries_1 (struct opt_pass *pass)
>  void
>  ipa_read_optimization_summaries (void)
>  {
> -  ipa_read_optimization_summaries_1 (all_regular_ipa_passes);
> -  ipa_read_optimization_summaries_1 (all_lto_gen_passes);
> +  pipeline &passes = g->get_passes ();
> +  ipa_read_optimization_summaries_1 (passes.all_regular_ipa_passes);
> +  ipa_read_optimization_summaries_1 (passes.all_lto_gen_passes);
>  }
>
>  /* Same as execute_pass_list but assume that subpasses of IPA passes
> @@ -2384,7 +2418,8 @@ execute_ipa_stmt_fixups (struct opt_pass *pass,
>  void
>  execute_all_ipa_stmt_fixups (struct cgraph_node *node, gimple *stmts)
>  {
> -  execute_ipa_stmt_fixups (all_regular_ipa_passes, node, stmts);
> +  pipeline &passes = g->get_passes ();
> +  execute_ipa_stmt_fixups (passes.all_regular_ipa_passes, node, stmts);
>  }
>
>
> diff --git a/gcc/pipeline.h b/gcc/pipeline.h
> new file mode 100644
> index 0000000..37c90d7
> --- /dev/null
> +++ b/gcc/pipeline.h
> @@ -0,0 +1,89 @@
> +/* pipeline.h - The pipeline of optimization passes
> +   Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free
> +Software Foundation; either version 3, or (at your option) any later
> +version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCC_PIPELINE_H
> +#define GCC_PIPELINE_H
> +
> +class opt_pass;
> +struct register_pass_info;
> +
> +/* Define a list of pass lists so that both passes.c and plugins can easily
> +   find all the pass lists.  */
> +#define GCC_PASS_LISTS \
> +  DEF_PASS_LIST (all_lowering_passes) \
> +  DEF_PASS_LIST (all_small_ipa_passes) \
> +  DEF_PASS_LIST (all_regular_ipa_passes) \
> +  DEF_PASS_LIST (all_lto_gen_passes) \
> +  DEF_PASS_LIST (all_passes)
> +
> +#define DEF_PASS_LIST(LIST) PASS_LIST_NO_##LIST,
> +enum pass_list
> +{
> +  GCC_PASS_LISTS
> +  PASS_LIST_NUM
> +};
> +#undef DEF_PASS_LIST
> +
> +namespace gcc {
> +
> +class context;
> +
> +class pipeline
> +{
> +public:
> +  pipeline(context *ctxt);
> +
> +  void register_pass (struct register_pass_info *pass_info);
> +  void register_one_dump_file (struct opt_pass *pass);
> +
> +  opt_pass *get_pass_for_id (int id) const;
> +
> +  void dump_passes () const;
> +
> +  void dump_profile_report () const;
> +
> +public:
> +  /* The root of the compilation pass tree, once constructed.  */
> +  opt_pass *all_passes;
> +  opt_pass *all_small_ipa_passes;
> +  opt_pass *all_lowering_passes;
> +  opt_pass *all_regular_ipa_passes;
> +  opt_pass *all_lto_gen_passes;
> +  opt_pass *all_late_ipa_passes;
> +
> +  /* A map from static pass id to optimization pass.  */
> +  opt_pass **passes_by_id;
> +  int passes_by_id_size;
> +
> +  opt_pass **pass_lists[PASS_LIST_NUM];
> +
> +private:
> +  void set_pass_for_id (int id, opt_pass *pass);
> +  int register_dump_files_1 (struct opt_pass *pass, int properties);
> +  void register_dump_files (struct opt_pass *pass, int properties);
> +
> +private:
> +  context *ctxt_;
> +
> +}; // class pipeline
> +
> +} // namespace gcc
> +
> +#endif /* ! GCC_PIPELINE_H */
> +
> diff --git a/gcc/statistics.c b/gcc/statistics.c
> index 3077cc0..c7e6abd 100644
> --- a/gcc/statistics.c
> +++ b/gcc/statistics.c
> @@ -26,6 +26,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "statistics.h"
>  #include "hash-table.h"
>  #include "function.h"
> +#include "context.h"
> +#include "pipeline.h"
>
>  static int statistics_dump_nr;
>  static int statistics_dump_flags;
> @@ -235,6 +237,7 @@ statistics_fini_1 (statistics_counter_t **slot, opt_pass *pass)
>  void
>  statistics_fini (void)
>  {
> +  gcc::pipeline &passes = g->get_passes ();
>    if (!statistics_dump_file)
>      return;
>
> @@ -243,10 +246,10 @@ statistics_fini (void)
>        unsigned i;
>        for (i = 0; i < nr_statistics_hashes; ++i)
>         if (statistics_hashes[i].is_created ()
> -           && get_pass_for_id (i) != NULL)
> +           && passes.get_pass_for_id (i) != NULL)
>           statistics_hashes[i]
>             .traverse_noresize <opt_pass *, statistics_fini_1>
> -           (get_pass_for_id (i));
> +           (passes.get_pass_for_id (i));
>      }
>
>    dump_end (statistics_dump_nr, statistics_dump_file);
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index de28a2d..1cf98dc 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1158,10 +1158,10 @@ general_init (const char *argv0)
>       processing.  */
>    init_ggc_heuristics();
>
> -  /* Create the singleton holder for global state.  */
> +  /* Create the singleton holder for global state.
> +     Doing so also creates the pipeline of passes.  */
>    g = new gcc::context();
>
> -  init_optimization_passes ();
>    statistics_early_init ();
>    finish_params ();
>  }
> diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
> index 547f355..16442ed 100644
> --- a/gcc/tree-pass.h
> +++ b/gcc/tree-pass.h
> @@ -489,35 +489,9 @@ extern struct gimple_opt_pass pass_inline_parameters;
>  extern struct gimple_opt_pass pass_update_address_taken;
>  extern struct gimple_opt_pass pass_convert_switch;
>
> -/* The root of the compilation pass tree, once constructed.  */
> -extern struct opt_pass *all_passes, *all_small_ipa_passes, *all_lowering_passes,
> -                       *all_regular_ipa_passes, *all_lto_gen_passes, *all_late_ipa_passes;
> -
> -/* Define a list of pass lists so that both passes.c and plugins can easily
> -   find all the pass lists.  */
> -#define GCC_PASS_LISTS \
> -  DEF_PASS_LIST (all_lowering_passes) \
> -  DEF_PASS_LIST (all_small_ipa_passes) \
> -  DEF_PASS_LIST (all_regular_ipa_passes) \
> -  DEF_PASS_LIST (all_lto_gen_passes) \
> -  DEF_PASS_LIST (all_passes)
> -
> -#define DEF_PASS_LIST(LIST) PASS_LIST_NO_##LIST,
> -enum
> -{
> -  GCC_PASS_LISTS
> -  PASS_LIST_NUM
> -};
> -#undef DEF_PASS_LIST
> -
> -/* This is used by plugins, and should also be used in
> -   passes.c:register_pass.  */
> -extern struct opt_pass **gcc_pass_lists[];
> -
>  /* Current optimization pass.  */
>  extern struct opt_pass *current_pass;
>
> -extern struct opt_pass * get_pass_for_id (int);
>  extern bool execute_one_pass (struct opt_pass *);
>  extern void execute_pass_list (struct opt_pass *);
>  extern void execute_ipa_pass_list (struct opt_pass *);
> @@ -547,9 +521,6 @@ extern void register_pass (struct register_pass_info *);
>     directly in jump threading, and avoid peeling them next time.  */
>  extern bool first_pass_instance;
>
> -extern struct opt_pass **passes_by_id;
> -extern int passes_by_id_size;
> -
>  /* Declare for plugins.  */
>  extern void do_per_function_toporder (void (*) (void *), void *);
>
> --
> 1.7.11.7
>
Diego Novillo - July 24, 2013, 11:10 p.m.
On Wed, Jul 24, 2013 at 6:56 PM, Diego Novillo <dnovillo@google.com> wrote:
> Could you please add a description of what this does?

Sorry.  You did, but in a previous message that I had managed to miss.
 Maybe include a reference to it in future postings?

Diego.
David Malcolm - July 25, 2013, 12:27 a.m.
On Wed, 2013-07-24 at 19:10 -0400, Diego Novillo wrote:
> On Wed, Jul 24, 2013 at 6:56 PM, Diego Novillo <dnovillo@google.com> wrote:
> > Could you please add a description of what this does?
> 
> Sorry.  You did, but in a previous message that I had managed to miss.
>  Maybe include a reference to it in future postings?

FWIW I used "git format-patch" with the --cover-letter option and then
put the bulk of the description in the cover letter i.e. the [PATCH 0/2]
email i.e. http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01088.html - is
that what you're referring to, or to some other email (e.g. the
discussion about passes.def in the other thread, which is:
http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01028.html )

Is the cover letter and patch series approach OK for this list?

Also, and sorry if I'm being slow here, but are you still looking for
more descriptive text within the ChangeLog header, within the body of
the code, or in the email?

Thanks
Dave
Martin Jambor - July 25, 2013, 12:18 p.m.
Hi,

On Wed, Jul 24, 2013 at 08:27:43PM -0400, David Malcolm wrote:
> On Wed, 2013-07-24 at 19:10 -0400, Diego Novillo wrote:
> > On Wed, Jul 24, 2013 at 6:56 PM, Diego Novillo <dnovillo@google.com> wrote:
> > > Could you please add a description of what this does?
> > 
> > Sorry.  You did, but in a previous message that I had managed to miss.
> >  Maybe include a reference to it in future postings?
> 
> FWIW I used "git format-patch" with the --cover-letter option and then
> put the bulk of the description in the cover letter i.e. the [PATCH 0/2]
> email i.e. http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01088.html - is
> that what you're referring to, or to some other email (e.g. the
> discussion about passes.def in the other thread, which is:
> http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01028.html )
> 
> Is the cover letter and patch series approach OK for this list?

Yes it is, many people including me use similar setups, although
usually we put descriptions of individual patches at the top of the
message with the patch itself rather than to the cover letter.  It is
more convenient for everyone, especially when the thread grows large
and much more convenient for people using clients like gmail which do
not thread and group messages with exactly the same subject instead.

Martin
Martin Jambor - July 25, 2013, 1:08 p.m.
Hi,

I don't know why it's me again but again I do have a few comments.

One global remark first: If we are going to start using the gcc
namespace (I understand it you need for isolation of symbols once you
use gcc as library, right?), I'm wondering whether mixing "using
namespace gcc" and explicit identifier qualifications is a good idea.
We have never had a discussion about namespace conventions but I'd
suggest that you add the using directive at the top of all gcc source
files that need it and never use explicit gcc:: qualification (unless
there is a reason for it, of course, like in symbol definitions).

But perhaps someone else with more C++ experience disagrees?

A few more comments inline:

On Wed, Jul 24, 2013 at 11:09:04AM -0400, David Malcolm wrote:
> gcc/
> 	* Makefile.in (PIPELINE_H): New.
> 	(lto-cgraph.o): Depend on CONTEXT_H and PIPELINE_H.
> 	(passes.o): Likewise.
> 	(statistics.o): Likewise.
> 	(cgraphunit.o): Likewise.
> 	(context.o): Depend on PIPELINE_H.
> 
> 	* pipeline.h: New.
> 
> 	* cgraphunit.c (cgraph_add_new_function): Update for moves
> 	of globals to fields of pipeline.
> 	(analyze_function): Likewise.
> 	(expand_function): Likewise.
> 	(ipa_passes): Likewise.
> 	(compile): Likewise.
> 
> 	* context.c (context::context): New.
> 	* context.h  (context::context): New.
> 	(context::get_passes): New.
> 	(context::passes_): New.
> 
> 	* lto-cgraph.c (input_node): Update for moves of globals to
> 	fields of pipeline.
> 
> 	* passes.c (all_passes): Remove, in favor of a field of the
> 	same name within the new class pipeline.
> 	(all_small_ipa_passes): Likewise.
> 	(all_lowering_passes): Likewise.
> 	(all_regular_ipa_passes): Likewise.
> 	(all_late_ipa_passes): Likewise.
> 	(all_lto_gen_passes): Likewise.
> 	(passes_by_id): Likewise.
> 	(passes_by_id_size): Likewise.
> 	(gcc_pass_lists): Remove, in favor of "pass_lists" field within
> 	the new class pipeline.
> 	(set_pass_for_id): Convert to...
> 	(pipeline::set_pass_for_id): ...method.
> 	(get_pass_for_id): Convert to...
> 	(pipeline::get_pass_for_id): ...method.
> 	(register_one_dump_file): Move body of implementation into...
> 	(pipeline::register_one_dump_file): ...here.
> 	(register_dump_files_1): Convert to...
> 	(pipeline::register_dump_files_1): ...method.
> 	(register_dump_files): Convert to...
> 	(pipeline::register_dump_files): ...method.
> 	(create_pass_tab): Update for moves of globals to fields of
> 	pipeline.
> 	(dump_passes): Move body of implementation into...
> 	(pipeline::dump_passes): ...here.
> 	(register_pass): Move body of implementation into...
> 	(pipeline::register_pass): ...here.
> 	(init_optimization_passes): Convert into...
> 	(pipeline::pipeline): ...constructor for new pipeline class, and
> 	initialize the pass_lists array.
> 	(check_profile_consistency): Update for moves of globals to
> 	fields of pipeline.
> 	(dump_profile_report): Move body of implementation into...
> 	(pipeline::dump_profile_report): ...here.
> 	(ipa_write_summaries_1): Update for moves of pass lists from
> 	being globals to fields of pipeline.
> 	(ipa_write_optimization_summaries): Likewise.
> 	(ipa_read_summaries):  Likewise.
> 	(ipa_read_optimization_summaries): Likewise.
> 	(execute_all_ipa_stmt_fixups): Likewise.
> 
> 	* statistics.c (statistics_fini): Update for moves of globals to
> 	fields of pipeline.
> 
> 	* toplev.c (general_init): Replace call to
> 	init_optimization_passes with construction of the pipeline
> 	instance.
> 
> 	* tree-pass.h (all_passes): Remove, in favor of a field of the
> 	same name within the new class pipeline.
> 	(all_small_ipa_passes): Likewise.
> 	(all_lowering_passes): Likewise.
> 	(all_regular_ipa_passes): Likewise.
> 	(all_lto_gen_passes): Likewise.
> 	(all_late_ipa_passes): Likewise.
> 	(passes_by_id): Likewise.
> 	(passes_by_id_size): Likewise.
> 	(gcc_pass_lists): Remove, in favor of "pass_lists" field within
> 	the new class pipeline.
> 	(get_pass_for_id): Remove.
> 
> gcc/lto/
> 
> 	* Make-lang.in (lto/lto.o:): Depend on CONTEXT_H and
> 	PIPELINE_H.
> 
> 	* lto.c (do_whole_program_analysis): Update for move of
> 	all_regular_ipa_passes from a global to a field of class
> 	pipeline.

...

> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index b82c2e0..dc489fb 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -194,6 +194,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "except.h"
>  #include "cfgloop.h"
>  #include "regset.h"     /* FIXME: For reg_obstack.  */
> +#include "context.h"
> +#include "pipeline.h"
>  
>  /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
>     secondary queue used during optimization to accommodate passes that
> @@ -478,6 +480,7 @@ cgraph_finalize_function (tree decl, bool nested)
>  void
>  cgraph_add_new_function (tree fndecl, bool lowered)
>  {
> +  gcc::pipeline &passes = g->get_passes ();
>    struct cgraph_node *node;
>    switch (cgraph_state)
>      {
> @@ -508,7 +511,7 @@ cgraph_add_new_function (tree fndecl, bool lowered)
>  	    push_cfun (DECL_STRUCT_FUNCTION (fndecl));
>  	    gimple_register_cfg_hooks ();
>  	    bitmap_obstack_initialize (NULL);
> -	    execute_pass_list (all_lowering_passes);
> +	    execute_pass_list (passes.all_lowering_passes);
>  	    execute_pass_list (pass_early_local_passes.pass.sub);
>  	    bitmap_obstack_release (NULL);
>  	    pop_cfun ();
> @@ -640,7 +643,7 @@ analyze_function (struct cgraph_node *node)
>  
>  	  gimple_register_cfg_hooks ();
>  	  bitmap_obstack_initialize (NULL);
> -	  execute_pass_list (all_lowering_passes);
> +	  execute_pass_list (g->get_passes ().all_lowering_passes);
>  	  free_dominance_info (CDI_POST_DOMINATORS);
>  	  free_dominance_info (CDI_DOMINATORS);
>  	  compact_blocks ();
> @@ -1588,7 +1591,7 @@ expand_function (struct cgraph_node *node)
>    /* Signal the start of passes.  */
>    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
>  
> -  execute_pass_list (all_passes);
> +  execute_pass_list (g->get_passes ().all_passes);
>  
>    /* Signal the end of passes.  */
>    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_END, NULL);
> @@ -1807,6 +1810,8 @@ output_in_order (void)
>  static void
>  ipa_passes (void)
>  {
> +  gcc::pipeline &passes = g->get_passes ();
> +
>    set_cfun (NULL);
>    current_function_decl = NULL;
>    gimple_register_cfg_hooks ();
> @@ -1816,7 +1821,7 @@ ipa_passes (void)
>  
>    if (!in_lto_p)
>      {
> -      execute_ipa_pass_list (all_small_ipa_passes);
> +      execute_ipa_pass_list (passes.all_small_ipa_passes);
>        if (seen_error ())
>  	return;
>      }
> @@ -1843,14 +1848,15 @@ ipa_passes (void)
>        cgraph_process_new_functions ();
>  
>        execute_ipa_summary_passes
> -	((struct ipa_opt_pass_d *) all_regular_ipa_passes);
> +	((struct ipa_opt_pass_d *) passes.all_regular_ipa_passes);
>      }
>  
>    /* Some targets need to handle LTO assembler output specially.  */
>    if (flag_generate_lto)
>      targetm.asm_out.lto_start ();
>  
> -  execute_ipa_summary_passes ((struct ipa_opt_pass_d *) all_lto_gen_passes);
> +  execute_ipa_summary_passes ((struct ipa_opt_pass_d *)
> +			      passes.all_lto_gen_passes);

Hehe.  I understand you have a lot of work with this, but perhaps you
could also turn struct opt_pass and its "descendants" into a struct
hierarchy too?  (I'd really use structs, not classes, and for now put
off any other re-engineering like visibilities and such).  And put
them into pipeline.h?  But perhaps not, I'll try to remember to do it
later :-)

>  
>    if (!in_lto_p)
>      ipa_write_summaries ();
> @@ -1859,7 +1865,7 @@ ipa_passes (void)
>      targetm.asm_out.lto_end ();
>  
>    if (!flag_ltrans && (in_lto_p || !flag_lto || flag_fat_lto_objects))
> -    execute_ipa_pass_list (all_regular_ipa_passes);
> +    execute_ipa_pass_list (passes.all_regular_ipa_passes);
>    invoke_plugin_callbacks (PLUGIN_ALL_IPA_PASSES_END, NULL);
>  
>    bitmap_obstack_release (NULL);
> @@ -1985,7 +1991,7 @@ compile (void)
>  
>    cgraph_materialize_all_clones ();
>    bitmap_obstack_initialize (NULL);
> -  execute_ipa_pass_list (all_late_ipa_passes);
> +  execute_ipa_pass_list (g->get_passes ().all_late_ipa_passes);
>    symtab_remove_unreachable_nodes (true, dump_file);
>  #ifdef ENABLE_CHECKING
>    verify_symtab ();
> diff --git a/gcc/context.c b/gcc/context.c
> index 76e0dde..8ec2e60 100644
> --- a/gcc/context.c
> +++ b/gcc/context.c
> @@ -22,6 +22,12 @@ along with GCC; see the file COPYING3.  If not see
>  #include "coretypes.h"
>  #include "ggc.h"
>  #include "context.h"
> +#include "pipeline.h"
>  
>  /* The singleton holder of global state: */
>  gcc::context *g;
> +
> +gcc::context::context()
> +{
> +  passes_ = new gcc::pipeline(this);
> +}

Missing spaces before parentheses (yeah, I dislike them too, but...)

> diff --git a/gcc/context.h b/gcc/context.h
> index 3caf02f..a83f7b2 100644
> --- a/gcc/context.h
> +++ b/gcc/context.h
> @@ -22,14 +22,23 @@ along with GCC; see the file COPYING3.  If not see
>  
>  namespace gcc {
>  
> +class pipeline;
> +
>  /* GCC's internal state can be divided into zero or more
>     "parallel universe" of state; an instance of this class is one such
>     context of state.  */
>  class context
>  {
>  public:
> +  context();
> +
> +  /* Pass-management.  */
> +
> +  pipeline &get_passes () { gcc_assert (passes_); return *passes_; }

I don't like that you return a reference instead of a pointer.  I
believe that in a project like gcc where we are about to mix C++ code
with large portion of original C stuff, we should always strongly
prefer good old pointers to references to avoid confusion.  Especially
in return value types.  (Yeah, I know that in some cases there are
substantial reasons to return references but this does not seem to be
one of them.)

>  
> -  /* Currently empty.  */
> +private:
> +  /* Pass-management.  */
> +  pipeline *passes_;
>  
>  }; // class context
>  
> diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> index 4a287f6..d322d9a 100644
> --- a/gcc/lto-cgraph.c
> +++ b/gcc/lto-cgraph.c
> @@ -47,6 +47,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "gcov-io.h"
>  #include "tree-pass.h"
>  #include "profile.h"
> +#include "context.h"
> +#include "pipeline.h"
>  
>  static void output_cgraph_opt_summary (void);
>  static void input_cgraph_opt_summary (vec<symtab_node>  nodes);
> @@ -936,6 +938,7 @@ input_node (struct lto_file_decl_data *file_data,
>  	    enum LTO_symtab_tags tag,
>  	    vec<symtab_node> nodes)
>  {
> +  gcc::pipeline &passes = g->get_passes ();

The same here and at a few other places.  It may be just me not being
used to references... nevertheless, if someone really wants to use
them like this, at least make them const and you will save a night of
frantic debugging to someone, probably to yourself.  But I strongly
prefer pointers... it's hard to describe how strongly I prefer them.

>    tree fn_decl;
>    struct cgraph_node *node;
>    struct bitpack_d bp;
> @@ -981,8 +984,8 @@ input_node (struct lto_file_decl_data *file_data,
>        struct opt_pass *pass;
>        int pid = streamer_read_hwi (ib);
>  
> -      gcc_assert (pid < passes_by_id_size);
> -      pass = passes_by_id[pid];
> +      gcc_assert (pid < passes.passes_by_id_size);
> +      pass = passes.passes_by_id[pid];
>        node->ipa_transforms_to_apply.safe_push ((struct ipa_opt_pass_d *) pass);
>      }
>  

...

> diff --git a/gcc/pipeline.h b/gcc/pipeline.h
> new file mode 100644
> index 0000000..37c90d7
> --- /dev/null
> +++ b/gcc/pipeline.h
> @@ -0,0 +1,89 @@
> +/* pipeline.h - The pipeline of optimization passes
> +   Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free
> +Software Foundation; either version 3, or (at your option) any later
> +version.
> +
> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> +for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +<http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GCC_PIPELINE_H
> +#define GCC_PIPELINE_H
> +
> +class opt_pass;
> +struct register_pass_info;
> +
> +/* Define a list of pass lists so that both passes.c and plugins can easily
> +   find all the pass lists.  */
> +#define GCC_PASS_LISTS \
> +  DEF_PASS_LIST (all_lowering_passes) \
> +  DEF_PASS_LIST (all_small_ipa_passes) \
> +  DEF_PASS_LIST (all_regular_ipa_passes) \
> +  DEF_PASS_LIST (all_lto_gen_passes) \
> +  DEF_PASS_LIST (all_passes)
> +
> +#define DEF_PASS_LIST(LIST) PASS_LIST_NO_##LIST,
> +enum pass_list

An enum is a list?  I understand this is nit-picking in moved existing
code but I'd change it to something less misleading anyway.  Is it
impossible to keep the union name-less for some reason?

> +{
> +  GCC_PASS_LISTS
> +  PASS_LIST_NUM
> +};
> +#undef DEF_PASS_LIST
> +
> +namespace gcc {
> +
> +class context;
> +
> +class pipeline
> +{
> +public:
> +  pipeline(context *ctxt);
> +
> +  void register_pass (struct register_pass_info *pass_info);
> +  void register_one_dump_file (struct opt_pass *pass);
> +
> +  opt_pass *get_pass_for_id (int id) const;
> +
> +  void dump_passes () const;
> +
> +  void dump_profile_report () const;
> +
> +public:

Extra public?  Or do we want that to divide data members from methods?

Thanks for all the work and sorry for the nagging.  However, you might
be setting up quite a few C++ precedents so I thought it would be
better to pay attention :-)

Martin
David Malcolm - July 29, 2013, 6:20 p.m.
On Thu, 2013-07-25 at 15:08 +0200, Martin Jambor wrote:
> Hi,
> 
> I don't know why it's me again but again I do have a few comments.

Thanks for looking over the patch.

> One global remark first: If we are going to start using the gcc
> namespace (I understand it you need for isolation of symbols once you
> use gcc as library, right?), I'm wondering whether mixing "using
> namespace gcc" and explicit identifier qualifications is a good idea.
> We have never had a discussion about namespace conventions but I'd
> suggest that you add the using directive at the top of all gcc source
> files that need it and never use explicit gcc:: qualification (unless
> there is a reason for it, of course, like in symbol definitions).
> 
> But perhaps someone else with more C++ experience disagrees?

http://gcc.gnu.org/codingconventions.html#Namespace_Use says:

> "Namespaces are encouraged. All separable libraries should have a
unique global namespace."
[...snip...]
> "Header files should have neither using directives nor namespace-scope
using declarations."

and the rationale doc says:
> "Using them within an implementation file can help conciseness."

However, there doesn't seem to be a discussion on the merits of the
various forms of "using" directives.

These aren't the GCC coding conventions, but the Google conventions:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces
forbid using directives of the form:
  using NAMESPACE;
but suggest using directives of the form:
  using NAMESPACE::NAME;
to pick out individual names from a namespace, though *not* in global
scope in a header (to avoid polluting the global namespace).

I like this approach - how about using it for frequently used names in
a .c/.cc file, keeping the names alphabetizing by "fully qualified
path", so e.g.:

  #include "foo.h"
  ...
  #include "bar.h"

  using gcc::context;
  using gcc::pass_manager;
  ...etc, individually grabbing the names we'll be needing

  // code follows

and thus avoiding grabbing whole namespaces, whilst keeping the code
concise.

I may want to violate that rule within gtype-desc.c, as per:
http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01262.html
to simplify handling of GTY and "gcc::"

> A few more comments inline:
Likewise

[...]

> > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> > index b82c2e0..dc489fb 100644
> > --- a/gcc/cgraphunit.c
> > +++ b/gcc/cgraphunit.c
> > @@ -194,6 +194,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "except.h"
> >  #include "cfgloop.h"
> >  #include "regset.h"     /* FIXME: For reg_obstack.  */
> > +#include "context.h"
> > +#include "pipeline.h"
> >  
> >  /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
> >     secondary queue used during optimization to accommodate passes that
> > @@ -478,6 +480,7 @@ cgraph_finalize_function (tree decl, bool nested)
> >  void
> >  cgraph_add_new_function (tree fndecl, bool lowered)
> >  {
> > +  gcc::pipeline &passes = g->get_passes ();
> >    struct cgraph_node *node;
> >    switch (cgraph_state)
> >      {
> > @@ -508,7 +511,7 @@ cgraph_add_new_function (tree fndecl, bool lowered)
> >  	    push_cfun (DECL_STRUCT_FUNCTION (fndecl));
> >  	    gimple_register_cfg_hooks ();
> >  	    bitmap_obstack_initialize (NULL);
> > -	    execute_pass_list (all_lowering_passes);
> > +	    execute_pass_list (passes.all_lowering_passes);
> >  	    execute_pass_list (pass_early_local_passes.pass.sub);
> >  	    bitmap_obstack_release (NULL);
> >  	    pop_cfun ();
> > @@ -640,7 +643,7 @@ analyze_function (struct cgraph_node *node)
> >  
> >  	  gimple_register_cfg_hooks ();
> >  	  bitmap_obstack_initialize (NULL);
> > -	  execute_pass_list (all_lowering_passes);
> > +	  execute_pass_list (g->get_passes ().all_lowering_passes);
> >  	  free_dominance_info (CDI_POST_DOMINATORS);
> >  	  free_dominance_info (CDI_DOMINATORS);
> >  	  compact_blocks ();
> > @@ -1588,7 +1591,7 @@ expand_function (struct cgraph_node *node)
> >    /* Signal the start of passes.  */
> >    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
> >  
> > -  execute_pass_list (all_passes);
> > +  execute_pass_list (g->get_passes ().all_passes);
> >  
> >    /* Signal the end of passes.  */
> >    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_END, NULL);
> > @@ -1807,6 +1810,8 @@ output_in_order (void)
> >  static void
> >  ipa_passes (void)
> >  {
> > +  gcc::pipeline &passes = g->get_passes ();
> > +
> >    set_cfun (NULL);
> >    current_function_decl = NULL;
> >    gimple_register_cfg_hooks ();
> > @@ -1816,7 +1821,7 @@ ipa_passes (void)
> >  
> >    if (!in_lto_p)
> >      {
> > -      execute_ipa_pass_list (all_small_ipa_passes);
> > +      execute_ipa_pass_list (passes.all_small_ipa_passes);
> >        if (seen_error ())
> >  	return;
> >      }
> > @@ -1843,14 +1848,15 @@ ipa_passes (void)
> >        cgraph_process_new_functions ();
> >  
> >        execute_ipa_summary_passes
> > -	((struct ipa_opt_pass_d *) all_regular_ipa_passes);
> > +	((struct ipa_opt_pass_d *) passes.all_regular_ipa_passes);
> >      }
> >  
> >    /* Some targets need to handle LTO assembler output specially.  */
> >    if (flag_generate_lto)
> >      targetm.asm_out.lto_start ();
> >  
> > -  execute_ipa_summary_passes ((struct ipa_opt_pass_d *) all_lto_gen_passes);
> > +  execute_ipa_summary_passes ((struct ipa_opt_pass_d *)
> > +			      passes.all_lto_gen_passes);
> 
> Hehe.  I understand you have a lot of work with this, but perhaps you
> could also turn struct opt_pass and its "descendants" into a struct
> hierarchy too?  (I'd really use structs, not classes, and for now put
> off any other re-engineering like visibilities and such).  And put
> them into pipeline.h?  But perhaps not, I'll try to remember to do it
> later :-)

I do this in http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01252.html

> >    if (!in_lto_p)
> >      ipa_write_summaries ();
> > @@ -1859,7 +1865,7 @@ ipa_passes (void)
> >      targetm.asm_out.lto_end ();
> >  
> >    if (!flag_ltrans && (in_lto_p || !flag_lto || flag_fat_lto_objects))
> > -    execute_ipa_pass_list (all_regular_ipa_passes);
> > +    execute_ipa_pass_list (passes.all_regular_ipa_passes);
> >    invoke_plugin_callbacks (PLUGIN_ALL_IPA_PASSES_END, NULL);
> >  
> >    bitmap_obstack_release (NULL);
> > @@ -1985,7 +1991,7 @@ compile (void)
> >  
> >    cgraph_materialize_all_clones ();
> >    bitmap_obstack_initialize (NULL);
> > -  execute_ipa_pass_list (all_late_ipa_passes);
> > +  execute_ipa_pass_list (g->get_passes ().all_late_ipa_passes);
> >    symtab_remove_unreachable_nodes (true, dump_file);
> >  #ifdef ENABLE_CHECKING
> >    verify_symtab ();
> > diff --git a/gcc/context.c b/gcc/context.c
> > index 76e0dde..8ec2e60 100644
> > --- a/gcc/context.c
> > +++ b/gcc/context.c
> > @@ -22,6 +22,12 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "coretypes.h"
> >  #include "ggc.h"
> >  #include "context.h"
> > +#include "pipeline.h"
> >  
> >  /* The singleton holder of global state: */
> >  gcc::context *g;
> > +
> > +gcc::context::context()
> > +{
> > +  passes_ = new gcc::pipeline(this);
> > +}
> 
> Missing spaces before parentheses (yeah, I dislike them too, but...)

I somehow got it into my head that constructors were an exception to
this rule, based on looking at the base-class constructor examples here:
http://gcc.gnu.org/codingconventions.html#Member_Form

which lack a space before parens.

But yes, I'll ensure that "new CLASSNAME ()" invocations have the space.

> > diff --git a/gcc/context.h b/gcc/context.h
> > index 3caf02f..a83f7b2 100644
> > --- a/gcc/context.h
> > +++ b/gcc/context.h
> > @@ -22,14 +22,23 @@ along with GCC; see the file COPYING3.  If not see
> >  
> >  namespace gcc {
> >  
> > +class pipeline;
> > +
> >  /* GCC's internal state can be divided into zero or more
> >     "parallel universe" of state; an instance of this class is one such
> >     context of state.  */
> >  class context
> >  {
> >  public:
> > +  context();
> > +
> > +  /* Pass-management.  */
> > +
> > +  pipeline &get_passes () { gcc_assert (passes_); return *passes_; }
> 
> I don't like that you return a reference instead of a pointer.  I
> believe that in a project like gcc where we are about to mix C++ code
> with large portion of original C stuff, we should always strongly
> prefer good old pointers to references to avoid confusion.  Especially
> in return value types.  (Yeah, I know that in some cases there are
> substantial reasons to return references but this does not seem to be
> one of them.)

Note that as per
  http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01365.html
we'll use "pass_manager" rather than "pipeline", so this would look
like:
  pass_manager &get_passes () { gcc_assert (passes_); return *passes_; }

We were chatting about C++ references on IRC on Friday, and IIRC there
was a strong objection to passing *arguments* that are non-const
references, rather than return values - mutation of *arguments* being
surprising and a place for bugs to arise (though that may have been
Diego who was arguing that, citing
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Reference_Arguments
).

I prefer to have get_passes return a reference rather a pointer since
which thing is being referred to isn't going to change, and is non-NULL.
One could write it as a "gcc::pipeline const * passes", but that doesn't
capture the non-NULL-ness of it.
[within the class data, "passes_" needs to be a *pointer* so that the
PCH deserialization can work, in case the object has been relocated].
Having the get_passes method do the assertion of non-NULLness and
dereference means that there's a single place where the non-NULLness is
asserted.

I guess this is a bigger point though: how do GCC maintainers feel about
C++ references in general?

Looking at the GCC Coding Conventions:
  http://gcc.gnu.org/codingconventions.html
and
  http://gcc.gnu.org/codingrationale.html
I don't see any mention of C++ references.

Are C++ references permissible inside GCC's own code (outside of
libstdc++, of course) and is the above usage sane?

There is another question here, which is how people feel about
accessors/getters?
I deliberately used one here since at my Cauldron talk someone (Roland?)
pointed out that if we want to optimize the single-state build, we can
change the insides of this method in one place and e.g. put the pass
manager in a fixed location in the bss segment, at which point field
accesses are of fixed locations, which is far cleaner that the various
macro based approaches I had proposed.  (how would this interact with
references vs pointers, if at all?)

> > -  /* Currently empty.  */
> > +private:
> > +  /* Pass-management.  */
> > +  pipeline *passes_;
> >  
> >  }; // class context
> >  
> > diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> > index 4a287f6..d322d9a 100644
> > --- a/gcc/lto-cgraph.c
> > +++ b/gcc/lto-cgraph.c
> > @@ -47,6 +47,8 @@ along with GCC; see the file COPYING3.  If not see
> >  #include "gcov-io.h"
> >  #include "tree-pass.h"
> >  #include "profile.h"
> > +#include "context.h"
> > +#include "pipeline.h"
> >  
> >  static void output_cgraph_opt_summary (void);
> >  static void input_cgraph_opt_summary (vec<symtab_node>  nodes);
> > @@ -936,6 +938,7 @@ input_node (struct lto_file_decl_data *file_data,
> >  	    enum LTO_symtab_tags tag,
> >  	    vec<symtab_node> nodes)
> >  {
> > +  gcc::pipeline &passes = g->get_passes ();
> 
> The same here and at a few other places.  It may be just me not being
> used to references... nevertheless, if someone really wants to use
> them like this, at least make them const and you will save a night of
> frantic debugging to someone, probably to yourself.  But I strongly
> prefer pointers... it's hard to describe how strongly I prefer them.

OK.  How do others feel?   As I said above, I like the above idiom,
since it puts the assertion of non-NULLness in a single place.

FWIW, I've changed the above to use a "using gcc::pass_manager", so in
my working copy it currently reads:

   pass_manager &passes = g->get_passes ();

[...snip...]

> > diff --git a/gcc/pipeline.h b/gcc/pipeline.h
> > new file mode 100644
> > index 0000000..37c90d7
> > --- /dev/null
> > +++ b/gcc/pipeline.h
> > @@ -0,0 +1,89 @@
> > +/* pipeline.h - The pipeline of optimization passes
> > +   Copyright (C) 2013 Free Software Foundation, Inc.
> > +
> > +This file is part of GCC.
> > +
> > +GCC is free software; you can redistribute it and/or modify it under
> > +the terms of the GNU General Public License as published by the Free
> > +Software Foundation; either version 3, or (at your option) any later
> > +version.
> > +
> > +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> > +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> > +for more details.
> > +
> > +You should have received a copy of the GNU General Public License
> > +along with GCC; see the file COPYING3.  If not see
> > +<http://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef GCC_PIPELINE_H
> > +#define GCC_PIPELINE_H
> > +
> > +class opt_pass;
> > +struct register_pass_info;
> > +
> > +/* Define a list of pass lists so that both passes.c and plugins can easily
> > +   find all the pass lists.  */
> > +#define GCC_PASS_LISTS \
> > +  DEF_PASS_LIST (all_lowering_passes) \
> > +  DEF_PASS_LIST (all_small_ipa_passes) \
> > +  DEF_PASS_LIST (all_regular_ipa_passes) \
> > +  DEF_PASS_LIST (all_lto_gen_passes) \
> > +  DEF_PASS_LIST (all_passes)
> > +
> > +#define DEF_PASS_LIST(LIST) PASS_LIST_NO_##LIST,
> > +enum pass_list
> 
> An enum is a list?  I understand this is nit-picking in moved existing
> code but I'd change it to something less misleading anyway.  Is it
> impossible to keep the union name-less for some reason?

Is anyone actually using the pass lists?

> > +{
> > +  GCC_PASS_LISTS
> > +  PASS_LIST_NUM
> > +};
> > +#undef DEF_PASS_LIST
> > +
> > +namespace gcc {
> > +
> > +class context;
> > +
> > +class pipeline
> > +{
> > +public:
> > +  pipeline(context *ctxt);
> > +
> > +  void register_pass (struct register_pass_info *pass_info);
> > +  void register_one_dump_file (struct opt_pass *pass);
> > +
> > +  opt_pass *get_pass_for_id (int id) const;
> > +
> > +  void dump_passes () const;
> > +
> > +  void dump_profile_report () const;
> > +
> > +public:
> 
> Extra public?  Or do we want that to divide data members from methods?

I added it as a marker to separate methods from data members, but I
suspect I'm, ahem, "setting precedent" here.

http://gcc.gnu.org/codingconventions.html#Class_Form lists the order in
which things should be declared within a class.

> Thanks for all the work and sorry for the nagging.  However, you might
> be setting up quite a few C++ precedents so I thought it would be
> better to pay attention :-)

Agreed - thanks for looking through the patch.

Dave
Oleg Endo - July 29, 2013, 7:02 p.m.
On Mon, 2013-07-29 at 14:20 -0400, David Malcolm wrote:
> > 
> > The same here and at a few other places.  It may be just me not being
> > used to references... nevertheless, if someone really wants to use
> > them like this, at least make them const and you will save a night of
> > frantic debugging to someone, probably to yourself.  But I strongly
> > prefer pointers... it's hard to describe how strongly I prefer them.
> 
> OK.  How do others feel?   As I said above, I like the above idiom,
> since it puts the assertion of non-NULLness in a single place.

I'm voting for references.  References can be seen as yet another
software structuring tool that instantly communicate some properties
such as you mentioned above.  In addition to that it's also a hint of
ownership, i.e. if I get an object& from somewhere I know that I better
not even think about whether to delete it or not.

Cheers,
Oleg
Martin Jambor - July 30, 2013, 9:11 a.m.
Hi,

thanks for the email I was supposed to write.

On Mon, Jul 29, 2013 at 02:20:02PM -0400, David Malcolm wrote:
> On Thu, 2013-07-25 at 15:08 +0200, Martin Jambor wrote:
> > Hi,
> > 
> > I don't know why it's me again but again I do have a few comments.
> 
> Thanks for looking over the patch.
> 
> > One global remark first: If we are going to start using the gcc
> > namespace (I understand it you need for isolation of symbols once you
> > use gcc as library, right?), I'm wondering whether mixing "using
> > namespace gcc" and explicit identifier qualifications is a good idea.
> > We have never had a discussion about namespace conventions but I'd
> > suggest that you add the using directive at the top of all gcc source
> > files that need it and never use explicit gcc:: qualification (unless
> > there is a reason for it, of course, like in symbol definitions).
> > 
> > But perhaps someone else with more C++ experience disagrees?
> 
> http://gcc.gnu.org/codingconventions.html#Namespace_Use says:
> 
> > "Namespaces are encouraged. All separable libraries should have a
> unique global namespace."
> [...snip...]
> > "Header files should have neither using directives nor namespace-scope
> using declarations."
> 
> and the rationale doc says:
> > "Using them within an implementation file can help conciseness."
> 
> However, there doesn't seem to be a discussion on the merits of the
> various forms of "using" directives.
> 
> These aren't the GCC coding conventions, but the Google conventions:
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces
> forbid using directives of the form:
>   using NAMESPACE;
> but suggest using directives of the form:
>   using NAMESPACE::NAME;
> to pick out individual names from a namespace, though *not* in global
> scope in a header (to avoid polluting the global namespace).
> 
> I like this approach - how about using it for frequently used names in
> a .c/.cc file, keeping the names alphabetizing by "fully qualified
> path", so e.g.:
> 
>   #include "foo.h"
>   ...
>   #include "bar.h"
> 
>   using gcc::context;
>   using gcc::pass_manager;
>   ...etc, individually grabbing the names we'll be needing
> 
>   // code follows
> 
> and thus avoiding grabbing whole namespaces, whilst keeping the code
> concise.

I don't know.  Before your patches there were no namespaces and the
one flat global namespace is cluttered, all symbols from included
header files are there.  I thought you just wanted to move (some part
of) this to the gcc namespace so that users of gcc-as-library will not
clash with it.  So I guess it depends on what you want to move to the
gcc namespace.  This enumeration approach would be viable if there
were only a few things in it, we can't expect people to put tens or
hundreds of such using directives to their sources.

In any event, I do not consider this to be really important, I was
just curious.  One day there will be great bikeshedding about division
of our current flat namespace, I certainly hope that day has not come
yet, though ;-)

> 
> I may want to violate that rule within gtype-desc.c, as per:
> http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01262.html
> to simplify handling of GTY and "gcc::"
> 
> > A few more comments inline:
> Likewise
> 
> [...]
> 
> > > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> > > index b82c2e0..dc489fb 100644
> > > --- a/gcc/cgraphunit.c
> > > +++ b/gcc/cgraphunit.c
> > > @@ -194,6 +194,8 @@ along with GCC; see the file COPYING3.  If not see
> > >  #include "except.h"
> > >  #include "cfgloop.h"
> > >  #include "regset.h"     /* FIXME: For reg_obstack.  */
> > > +#include "context.h"
> > > +#include "pipeline.h"
> > >  
> > >  /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
> > >     secondary queue used during optimization to accommodate passes that
> > > @@ -478,6 +480,7 @@ cgraph_finalize_function (tree decl, bool nested)
> > >  void
> > >  cgraph_add_new_function (tree fndecl, bool lowered)
> > >  {
> > > +  gcc::pipeline &passes = g->get_passes ();
> > >    struct cgraph_node *node;
> > >    switch (cgraph_state)
> > >      {
> > > @@ -508,7 +511,7 @@ cgraph_add_new_function (tree fndecl, bool lowered)
> > >  	    push_cfun (DECL_STRUCT_FUNCTION (fndecl));
> > >  	    gimple_register_cfg_hooks ();
> > >  	    bitmap_obstack_initialize (NULL);
> > > -	    execute_pass_list (all_lowering_passes);
> > > +	    execute_pass_list (passes.all_lowering_passes);
> > >  	    execute_pass_list (pass_early_local_passes.pass.sub);
> > >  	    bitmap_obstack_release (NULL);
> > >  	    pop_cfun ();
> > > @@ -640,7 +643,7 @@ analyze_function (struct cgraph_node *node)
> > >  
> > >  	  gimple_register_cfg_hooks ();
> > >  	  bitmap_obstack_initialize (NULL);
> > > -	  execute_pass_list (all_lowering_passes);
> > > +	  execute_pass_list (g->get_passes ().all_lowering_passes);
> > >  	  free_dominance_info (CDI_POST_DOMINATORS);
> > >  	  free_dominance_info (CDI_DOMINATORS);
> > >  	  compact_blocks ();
> > > @@ -1588,7 +1591,7 @@ expand_function (struct cgraph_node *node)
> > >    /* Signal the start of passes.  */
> > >    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
> > >  
> > > -  execute_pass_list (all_passes);
> > > +  execute_pass_list (g->get_passes ().all_passes);
> > >  
> > >    /* Signal the end of passes.  */
> > >    invoke_plugin_callbacks (PLUGIN_ALL_PASSES_END, NULL);
> > > @@ -1807,6 +1810,8 @@ output_in_order (void)
> > >  static void
> > >  ipa_passes (void)
> > >  {
> > > +  gcc::pipeline &passes = g->get_passes ();
> > > +
> > >    set_cfun (NULL);
> > >    current_function_decl = NULL;
> > >    gimple_register_cfg_hooks ();
> > > @@ -1816,7 +1821,7 @@ ipa_passes (void)
> > >  
> > >    if (!in_lto_p)
> > >      {
> > > -      execute_ipa_pass_list (all_small_ipa_passes);
> > > +      execute_ipa_pass_list (passes.all_small_ipa_passes);
> > >        if (seen_error ())
> > >  	return;
> > >      }
> > > @@ -1843,14 +1848,15 @@ ipa_passes (void)
> > >        cgraph_process_new_functions ();
> > >  
> > >        execute_ipa_summary_passes
> > > -	((struct ipa_opt_pass_d *) all_regular_ipa_passes);
> > > +	((struct ipa_opt_pass_d *) passes.all_regular_ipa_passes);
> > >      }
> > >  
> > >    /* Some targets need to handle LTO assembler output specially.  */
> > >    if (flag_generate_lto)
> > >      targetm.asm_out.lto_start ();
> > >  
> > > -  execute_ipa_summary_passes ((struct ipa_opt_pass_d *) all_lto_gen_passes);
> > > +  execute_ipa_summary_passes ((struct ipa_opt_pass_d *)
> > > +			      passes.all_lto_gen_passes);
> > 
> > Hehe.  I understand you have a lot of work with this, but perhaps you
> > could also turn struct opt_pass and its "descendants" into a struct
> > hierarchy too?  (I'd really use structs, not classes, and for now put
> > off any other re-engineering like visibilities and such).  And put
> > them into pipeline.h?  But perhaps not, I'll try to remember to do it
> > later :-)
> 
> I do this in http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01252.html

Great!

> > > diff --git a/gcc/context.h b/gcc/context.h
> > > index 3caf02f..a83f7b2 100644
> > > --- a/gcc/context.h
> > > +++ b/gcc/context.h
> > > @@ -22,14 +22,23 @@ along with GCC; see the file COPYING3.  If not see
> > >  
> > >  namespace gcc {
> > >  
> > > +class pipeline;
> > > +
> > >  /* GCC's internal state can be divided into zero or more
> > >     "parallel universe" of state; an instance of this class is one such
> > >     context of state.  */
> > >  class context
> > >  {
> > >  public:
> > > +  context();
> > > +
> > > +  /* Pass-management.  */
> > > +
> > > +  pipeline &get_passes () { gcc_assert (passes_); return *passes_; }
> > 
> > I don't like that you return a reference instead of a pointer.  I
> > believe that in a project like gcc where we are about to mix C++ code
> > with large portion of original C stuff, we should always strongly
> > prefer good old pointers to references to avoid confusion.  Especially
> > in return value types.  (Yeah, I know that in some cases there are
> > substantial reasons to return references but this does not seem to be
> > one of them.)
> 
> Note that as per
>   http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01365.html
> we'll use "pass_manager" rather than "pipeline", so this would look
> like:
>   pass_manager &get_passes () { gcc_assert (passes_); return *passes_; }
> 
> We were chatting about C++ references on IRC on Friday, and IIRC there
> was a strong objection to passing *arguments* that are non-const
> references, rather than return values - mutation of *arguments* being
> surprising and a place for bugs to arise (though that may have been
> Diego who was arguing that, citing
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Reference_Arguments
> ).

That rule is a very good one and I think all the worries apply very
well to getter methods too.  (Although, really real C++ projects have
both getters and const getters, I think, so in a really real C++ code
your getter would be a const getter anyway ;-).

Moreover, I think we should be extra careful here because GCC is
transitioning from C and many developers are used to expect C
semantics, we will be mixing new C++ code with C code a lot in the
future and even though we hope to reduce that, people will be cutting
and pasting all this stuff around.

> 
> I prefer to have get_passes return a reference rather a pointer since
> which thing is being referred to isn't going to change, and is non-NULL.
> One could write it as a "gcc::pipeline const * passes", but that doesn't
> capture the non-NULL-ness of it.
> [within the class data, "passes_" needs to be a *pointer* so that the
> PCH deserialization can work, in case the object has been relocated].
> Having the get_passes method do the assertion of non-NULLness and
> dereference means that there's a single place where the non-NULLness is
> asserted.

I'm not afraid of bugs when we dereference a NULL, those are easy to
fix.  On the other hand I am afraid of bugs caused by inadvertent
modification of data that is hard to spot, those can be horrible to
debug.

> 
> I guess this is a bigger point though: how do GCC maintainers feel about
> C++ references in general?
> 
> Looking at the GCC Coding Conventions:
>   http://gcc.gnu.org/codingconventions.html
> and
>   http://gcc.gnu.org/codingrationale.html
> I don't see any mention of C++ references.
> 
> Are C++ references permissible inside GCC's own code (outside of
> libstdc++, of course) and is the above usage sane?

We already use them in vectors (and they have already confused me
there although I don't remember how exactly - but it was a syntax
problem).  I personally do not really like them and consider them a
bit alien.  However, I'm not really against const references where it
can save some typing or even against non-const ones when they are
necessary (but the only such situation I can think of is operator
overloading).

> 
> There is another question here, which is how people feel about
> accessors/getters?

I think people generally like them, except when they are stepping
through multitudes of them in gdb.  There is a tradeoff but I think
that at the moment people are expected to add them for interfaces with
wider use (and can choose to avoid them when interface use is not so
wide, e.g. restricted to a particular pass).

> I deliberately used one here since at my Cauldron talk someone (Roland?)
> pointed out that if we want to optimize the single-state build, we can
> change the insides of this method in one place and e.g. put the pass
> manager in a fixed location in the bss segment, at which point field
> accesses are of fixed locations, which is far cleaner that the various
> macro based approaches I had proposed.  (how would this interact with
> references vs pointers, if at all?)
> 
> > > -  /* Currently empty.  */
> > > +private:
> > > +  /* Pass-management.  */
> > > +  pipeline *passes_;
> > >  
> > >  }; // class context
> > >  
> > > diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> > > index 4a287f6..d322d9a 100644
> > > --- a/gcc/lto-cgraph.c
> > > +++ b/gcc/lto-cgraph.c
> > > @@ -47,6 +47,8 @@ along with GCC; see the file COPYING3.  If not see
> > >  #include "gcov-io.h"
> > >  #include "tree-pass.h"
> > >  #include "profile.h"
> > > +#include "context.h"
> > > +#include "pipeline.h"
> > >  
> > >  static void output_cgraph_opt_summary (void);
> > >  static void input_cgraph_opt_summary (vec<symtab_node>  nodes);
> > > @@ -936,6 +938,7 @@ input_node (struct lto_file_decl_data *file_data,
> > >  	    enum LTO_symtab_tags tag,
> > >  	    vec<symtab_node> nodes)
> > >  {
> > > +  gcc::pipeline &passes = g->get_passes ();
> > 
> > The same here and at a few other places.  It may be just me not being
> > used to references... nevertheless, if someone really wants to use
> > them like this, at least make them const and you will save a night of
> > frantic debugging to someone, probably to yourself.  But I strongly
> > prefer pointers... it's hard to describe how strongly I prefer them.
> 
> OK.  How do others feel?   As I said above, I like the above idiom,
> since it puts the assertion of non-NULLness in a single place.
> 
> FWIW, I've changed the above to use a "using gcc::pass_manager", so in
> my working copy it currently reads:
> 
>    pass_manager &passes = g->get_passes ();
> 
> [...snip...]
> 
> > > diff --git a/gcc/pipeline.h b/gcc/pipeline.h
> > > new file mode 100644
> > > index 0000000..37c90d7
> > > --- /dev/null
> > > +++ b/gcc/pipeline.h
> > > @@ -0,0 +1,89 @@
> > > +/* pipeline.h - The pipeline of optimization passes
> > > +   Copyright (C) 2013 Free Software Foundation, Inc.
> > > +
> > > +This file is part of GCC.
> > > +
> > > +GCC is free software; you can redistribute it and/or modify it under
> > > +the terms of the GNU General Public License as published by the Free
> > > +Software Foundation; either version 3, or (at your option) any later
> > > +version.
> > > +
> > > +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> > > +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > > +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> > > +for more details.
> > > +
> > > +You should have received a copy of the GNU General Public License
> > > +along with GCC; see the file COPYING3.  If not see
> > > +<http://www.gnu.org/licenses/>.  */
> > > +
> > > +#ifndef GCC_PIPELINE_H
> > > +#define GCC_PIPELINE_H
> > > +
> > > +class opt_pass;
> > > +struct register_pass_info;
> > > +
> > > +/* Define a list of pass lists so that both passes.c and plugins can easily
> > > +   find all the pass lists.  */
> > > +#define GCC_PASS_LISTS \
> > > +  DEF_PASS_LIST (all_lowering_passes) \
> > > +  DEF_PASS_LIST (all_small_ipa_passes) \
> > > +  DEF_PASS_LIST (all_regular_ipa_passes) \
> > > +  DEF_PASS_LIST (all_lto_gen_passes) \
> > > +  DEF_PASS_LIST (all_passes)
> > > +
> > > +#define DEF_PASS_LIST(LIST) PASS_LIST_NO_##LIST,
> > > +enum pass_list
> > 
> > An enum is a list?  I understand this is nit-picking in moved existing
> > code but I'd change it to something less misleading anyway.  Is it
> > impossible to keep the union name-less for some reason?
> 
> Is anyone actually using the pass lists?

IIRC, it was anonymous before so it's safe to say nobody does, it is
just there to define the enum elements (to be used as integers?), I
suppose.

Thanks,

Martin
Martin Jambor - July 30, 2013, 9:30 a.m.
Hi,

On Mon, Jul 29, 2013 at 09:02:53PM +0200, Oleg Endo wrote:
> On Mon, 2013-07-29 at 14:20 -0400, David Malcolm wrote:
> > > 
> > > The same here and at a few other places.  It may be just me not being
> > > used to references... nevertheless, if someone really wants to use
> > > them like this, at least make them const and you will save a night of
> > > frantic debugging to someone, probably to yourself.  But I strongly
> > > prefer pointers... it's hard to describe how strongly I prefer them.
> > 
> > OK.  How do others feel?   As I said above, I like the above idiom,
> > since it puts the assertion of non-NULLness in a single place.
> 
> I'm voting for references.  References can be seen as yet another
> software structuring tool that instantly communicate some properties
> such as you mentioned above.  In addition to that it's also a hint of
> ownership, i.e. if I get an object& from somewhere I know that I better
> not even think about whether to delete it or not.
> 

well, let me stress again that we should think about this in the
context of GCC.  In GCC, we are used to C semantics of the dot
operator and have a lot of existing code that we will continue to use
and mix with new code with the same assumption.  Putting a reference
where none has been before might result in silent and hard to spot
erroneous modifications.

At the same time, we are used to lookup whether the pointer we got can
be NULL or not if necessary.  Moreover, in GCC, NULL-dereferences are
not particularly dangerous and are easy to debug.

In the context of GCC, there will be no ownership hints of this kind
simply because new code will co-exist with tons of old code which uses
pointers and will not give any such hints and so you can't assume you
could free its pointers.  Let me not even mention GC.

Therefore, I'd avoid non const references where we can use pointers.

Thanks,

Martin
Oleg Endo - July 30, 2013, 6:50 p.m.
On Tue, 2013-07-30 at 11:30 +0200, Martin Jambor wrote:
> Hi,
> 
> On Mon, Jul 29, 2013 at 09:02:53PM +0200, Oleg Endo wrote:
> > On Mon, 2013-07-29 at 14:20 -0400, David Malcolm wrote:
> > > > 
> > > > The same here and at a few other places.  It may be just me not being
> > > > used to references... nevertheless, if someone really wants to use
> > > > them like this, at least make them const and you will save a night of
> > > > frantic debugging to someone, probably to yourself.  But I strongly
> > > > prefer pointers... it's hard to describe how strongly I prefer them.
> > > 
> > > OK.  How do others feel?   As I said above, I like the above idiom,
> > > since it puts the assertion of non-NULLness in a single place.
> > 
> > I'm voting for references.  References can be seen as yet another
> > software structuring tool that instantly communicate some properties
> > such as you mentioned above.  In addition to that it's also a hint of
> > ownership, i.e. if I get an object& from somewhere I know that I better
> > not even think about whether to delete it or not.
> > 
> 
> well, let me stress again that we should think about this in the
> context of GCC.  In GCC, we are used to C semantics of the dot
> operator

How are the dot operator semantics in C different from the dot operator
semantics in C++?

>  and have a lot of existing code that we will continue to use
> and mix with new code with the same assumption.  Putting a reference
> where none has been before might result in silent and hard to spot
> erroneous modifications.

Sorry, I have difficulty understanding your reasoning here.  Could you
provide an example for "Putting a reference where none has been before
might result in silent and hard to spot erroneous modifications."?

Cheers,
Oleg
Gabriel Dos Reis - Aug. 3, 2013, 4:30 a.m.
[ Adding Benjamin, Diego, Lawrence ]

General remarks first:
When we designed the coding standards for GCC, an overriding
philosophy was that we did not want to be prescriptive.  Rather, we
explicitly wanted to encourage common sense and trust the judgment
of maintainers to make sound and appropriate judgments.

More than a year later, I still believe that was the right thing to do
and I would not want us to start being prescriptive.

On Mon, Jul 29, 2013 at 1:20 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2013-07-25 at 15:08 +0200, Martin Jambor wrote:
>> Hi,
>>
>> I don't know why it's me again but again I do have a few comments.
>
> Thanks for looking over the patch.
>
>> One global remark first: If we are going to start using the gcc
>> namespace (I understand it you need for isolation of symbols once you
>> use gcc as library, right?), I'm wondering whether mixing "using
>> namespace gcc" and explicit identifier qualifications is a good idea.
>> We have never had a discussion about namespace conventions but I'd
>> suggest that you add the using directive at the top of all gcc source
>> files that need it and never use explicit gcc:: qualification (unless
>> there is a reason for it, of course, like in symbol definitions).
>>
>> But perhaps someone else with more C++ experience disagrees?
>
> http://gcc.gnu.org/codingconventions.html#Namespace_Use says:
>
>> "Namespaces are encouraged. All separable libraries should have a
> unique global namespace."
> [...snip...]
>> "Header files should have neither using directives nor namespace-scope
> using declarations."
>
> and the rationale doc says:
>> "Using them within an implementation file can help conciseness."
>
> However, there doesn't seem to be a discussion on the merits of the
> various forms of "using" directives.

It is a "common wisdom" among C++ programmers that using directives
in header files generally negate the benefits of namespaces, and can
lead to surprises.  This is because most of the time, people don't go
scrutinize header files; they include them (based on documentation
of declared signatures) in many translation units.

There is one notable exception to this, which is known as
"namespace composition":

  http://stackoverflow.com/questions/16871390/why-is-namespace-composition-so-rarely-used

We should probably amend the coding standards and include that hint,
but I don't feel strongly about it.

>
> These aren't the GCC coding conventions, but the Google conventions:
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces
> forbid using directives of the form:
>   using NAMESPACE;
> but suggest using directives of the form:
>   using NAMESPACE::NAME;
> to pick out individual names from a namespace, though *not* in global
> scope in a header (to avoid polluting the global namespace).

The using declarations are fine, as indicated.

>
> I like this approach - how about using it for frequently used names in
> a .c/.cc file, keeping the names alphabetizing by "fully qualified
> path", so e.g.:
>
>   #include "foo.h"
>   ...
>   #include "bar.h"
>
>   using gcc::context;
>   using gcc::pass_manager;
>   ...etc, individually grabbing the names we'll be needing
>
>   // code follows
>
> and thus avoiding grabbing whole namespaces, whilst keeping the code
> concise.

Agreed.

[…]

> Note that as per
>   http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01365.html
> we'll use "pass_manager" rather than "pipeline", so this would look
> like:
>   pass_manager &get_passes () { gcc_assert (passes_); return *passes_; }
>
> We were chatting about C++ references on IRC on Friday, and IIRC there
> was a strong objection to passing *arguments* that are non-const
> references,

I hope nobody is objecting to std::swap for example.

> rather than return values - mutation of *arguments* being
> surprising and a place for bugs to arise (though that may have been
> Diego who was arguing that, citing
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Reference_Arguments
> ).
>
> I prefer to have get_passes return a reference rather a pointer since
> which thing is being referred to isn't going to change, and is non-NULL.
> One could write it as a "gcc::pipeline const * passes", but that doesn't
> capture the non-NULL-ness of it.
> [within the class data, "passes_" needs to be a *pointer* so that the
> PCH deserialization can work, in case the object has been relocated].
> Having the get_passes method do the assertion of non-NULLness and
> dereference means that there's a single place where the non-NULLness is
> asserted.
>
> I guess this is a bigger point though: how do GCC maintainers feel about
> C++ references in general?

I think we should use them where appropriate, and trust maintainers
of specific areas of the compiler to exercise sound judgments instead
of us trying to cast ideology into stones.

>
> Looking at the GCC Coding Conventions:
>   http://gcc.gnu.org/codingconventions.html
> and
>   http://gcc.gnu.org/codingrationale.html
> I don't see any mention of C++ references.

Because, there shouldn't be.  They are part of basic
C++ programming techniques.

> Are C++ references permissible inside GCC's own code (outside of
> libstdc++, of course) and is the above usage sane?

Sound usage should be freely used.

> There is another question here, which is how people feel about
> accessors/getters?

Instead of general blanket rules,  I think the decision should be based
on specific abstractions and usage, not mechanical rules.   There are
places where I see getters/setters as poor abstraction and insufficient
thought applied (an example would be writing setters/getters for std::pair),
and other times where it reflects sound and brilliant abstraction.

> I deliberately used one here since at my Cauldron talk someone (Roland?)
> pointed out that if we want to optimize the single-state build, we can
> change the insides of this method in one place and e.g. put the pass
> manager in a fixed location in the bss segment, at which point field
> accesses are of fixed locations, which is far cleaner that the various
> macro based approaches I had proposed.  (how would this interact with
> references vs pointers, if at all?)

Like I said, it depends on the abstraction and effects desired :-)

-- Gaby
Gabriel Dos Reis - Aug. 3, 2013, 4:35 a.m.
On Tue, Jul 30, 2013 at 4:11 AM, Martin Jambor <mjambor@suse.cz> wrote:

> Moreover, I think we should be extra careful here because GCC is
> transitioning from C and many developers are used to expect C
> semantics, we will be mixing new C++ code with C code a lot in the
> future and even though we hope to reduce that, people will be cutting
> and pasting all this stuff around.

Yes, we can't reject writing idiomatic C++ just because there would be
a period of transition where old codes and new codes co-exist.
It wouldn't make much sense.

-- Gaby
Gabriel Dos Reis - Aug. 3, 2013, 4:38 a.m.
On Tue, Jul 30, 2013 at 4:30 AM, Martin Jambor <mjambor@suse.cz> wrote:

>> I'm voting for references.  References can be seen as yet another
>> software structuring tool that instantly communicate some properties
>> such as you mentioned above.  In addition to that it's also a hint of
>> ownership, i.e. if I get an object& from somewhere I know that I better
>> not even think about whether to delete it or not.
>>
>
> well, let me stress again that we should think about this in the
> context of GCC.  In GCC, we are used to C semantics of the dot
> operator and have a lot of existing code that we will continue to use
> and mix with new code with the same assumption.  Putting a reference
> where none has been before might result in silent and hard to spot
> erroneous modifications.

I cannot make sense of this.

> At the same time, we are used to lookup whether the pointer we got can
> be NULL or not if necessary.  Moreover, in GCC, NULL-dereferences are
> not particularly dangerous and are easy to debug.

That should not be reason to avoid idiomatic constructions.

> In the context of GCC, there will be no ownership hints of this kind
> simply because new code will co-exist with tons of old code which uses
> pointers and will not give any such hints and so you can't assume you
> could free its pointers.  Let me not even mention GC.

References are not just about ownership (though they might imply it).


-- Gaby
Martin Jambor - Aug. 5, 2013, 11:24 a.m.
Hi,

On Fri, Aug 02, 2013 at 11:30:01PM -0500, Gabriel Dos Reis wrote:
> [ Adding Benjamin, Diego, Lawrence ]
> 
> General remarks first:
> When we designed the coding standards for GCC, an overriding
> philosophy was that we did not want to be prescriptive.  Rather, we
> explicitly wanted to encourage common sense and trust the judgment
> of maintainers to make sound and appropriate judgments.
> 
> More than a year later, I still believe that was the right thing to do
> and I would not want us to start being prescriptive.
> 
> 
> It is a "common wisdom" among C++ programmers that using directives
> in header files generally negate the benefits of namespaces, and can
> lead to surprises.  This is because most of the time, people don't go
> scrutinize header files; they include them (based on documentation
> of declared signatures) in many translation units.

Perhaps it was a mistake of mine but I never intended to discuss using
"using" in header files.  I was basically wondering whether we should
start a C file with "using namespace gcc" instead of sprinkling the
source below with "gcc::" qualifiers.  I suppose it probably depends
on what we intend to move in that particular namespace (or into
another namespace in general but I think that we should probably
discuss this only after a great deal of Andrew's re-architecting
actually happens and we see the clearer interfaces that will hopefully
emerge).

> 
> There is one notable exception to this, which is known as
> "namespace composition":
> 
>   http://stackoverflow.com/questions/16871390/why-is-namespace-composition-so-rarely-used
> 
> We should probably amend the coding standards and include that hint,
> but I don't feel strongly about it.
> 
> >
> > These aren't the GCC coding conventions, but the Google conventions:
> > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces
> > forbid using directives of the form:
> >   using NAMESPACE;
> > but suggest using directives of the form:
> >   using NAMESPACE::NAME;
> > to pick out individual names from a namespace, though *not* in global
> > scope in a header (to avoid polluting the global namespace).
> 
> The using declarations are fine, as indicated.
> 
> >
> > I like this approach - how about using it for frequently used names in
> > a .c/.cc file, keeping the names alphabetizing by "fully qualified
> > path", so e.g.:
> >
> >   #include "foo.h"
> >   ...
> >   #include "bar.h"
> >
> >   using gcc::context;
> >   using gcc::pass_manager;
> >   ...etc, individually grabbing the names we'll be needing
> >
> >   // code follows
> >
> > and thus avoiding grabbing whole namespaces, whilst keeping the code
> > concise.
> 
> Agreed.

If that's the general feeling, I'll comply.  With only one big gcc
namespace, I do not see the point though.  Once we'll divide our
namespace, it will make sense.  (But as I wrote above, I think that
it will take and should take some time before we get there.)

> 
> […]
> 
> > Note that as per
> >   http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01365.html
> > we'll use "pass_manager" rather than "pipeline", so this would look
> > like:
> >   pass_manager &get_passes () { gcc_assert (passes_); return *passes_; }
> >
> > We were chatting about C++ references on IRC on Friday, and IIRC there
> > was a strong objection to passing *arguments* that are non-const
> > references,
> 
> I hope nobody is objecting to std::swap for example.

I'm not sure how I could object to std::swap or what good would that
be.  Apart from being the standard, it is also rather exceptional due
to its nature and its name.  Despite this special example, I think the
rule of not passing arguments as non-const references is a good one,
and it seems I am not alone.  Where I may differ from others is that I
also think that getter functions should (generally) return pointers or
const references too, and for similar reasons - the caller may
unintentionally modify data that the programmers thought were on the
stack but were in fact elsewhere else and "belonged" to someplace
else.

Nevertheless, I do not want spend too much time arguing about this, it
seems I have not persuaded people that it will lead to bugs which will
be hard to debug and so let's drop this discussion and hope that I am
wrong (though I will swear a lot if I ever I stumble across such a
bug).

Martin
Gabriel Dos Reis - Aug. 5, 2013, 2:35 p.m.
On Mon, Aug 5, 2013 at 6:24 AM, Martin Jambor <mjambor@suse.cz> wrote:
[…]
>> > Note that as per
>> >   http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01365.html
>> > we'll use "pass_manager" rather than "pipeline", so this would look
>> > like:
>> >   pass_manager &get_passes () { gcc_assert (passes_); return *passes_; }
>> >
>> > We were chatting about C++ references on IRC on Friday, and IIRC there
>> > was a strong objection to passing *arguments* that are non-const
>> > references,
>>
>> I hope nobody is objecting to std::swap for example.
>
> I'm not sure how I could object to std::swap or what good would that
> be.  Apart from being the standard, it is also rather exceptional due
> to its nature and its name.

I suspect it is a mistake to think this situation is unique.  I also think
that we can keep going on and a long list of examples, and that might
still be labelled special case.  Reading into a variable is just another
example.

> Despite this special example, I think the
> rule of not passing arguments as non-const references is a good one,
> and it seems I am not alone.  Where I may differ from others is that I
> also think that getter functions should (generally) return pointers or
> const references too, and for similar reasons - the caller may
> unintentionally modify data that the programmers thought were on the
> stack but were in fact elsewhere else and "belonged" to someplace
> else.

But the rule does not solve that problem -- 'const' does not mean this is
on stack, and absence of const does not mean this is on free store.
Being on stack does no necessarily mean don't change, and being on
free store does not mean you can freely change.

As a matter of fact, in the existing code base, we have many objects
on the stack that are passed around by non-const pointer. That is just
fine for the most part.  The notion of const is largely orthogonal to the
notion lifetime (stack of free store.)

These are problems that depend on the specific algorithms being
implemented.  Legislating on const reference or not won't solve
any problem there.

-- Gaby

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index fb0cb4b..0b28c2d 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -987,6 +987,7 @@  PLUGIN_VERSION_H = plugin-version.h configargs.h
 LIBFUNCS_H = libfuncs.h $(HASHTAB_H)
 GRAPHITE_HTAB_H = graphite-htab.h graphite-clast-to-gimple.h $(HASH_TABLE_H)
 CONTEXT_H = context.h
+PIPELINE_H = pipeline.h
 
 #
 # Now figure out from those variables how to compile and link.
@@ -2183,7 +2184,8 @@  lto-cgraph.o: lto-cgraph.c $(CONFIG_H) $(SYSTEM_H) coretypes.h   \
    $(HASHTAB_H) langhooks.h $(BASIC_BLOCK_H) \
    $(TREE_FLOW_H) $(CGRAPH_H) $(FUNCTION_H) $(GGC_H) $(DIAGNOSTIC_CORE_H) \
    $(EXCEPT_H) $(TIMEVAR_H) pointer-set.h $(LTO_STREAMER_H) \
-   $(GCOV_IO_H) $(DATA_STREAMER_H) $(TREE_STREAMER_H) $(TREE_PASS_H) profile.h
+   $(GCOV_IO_H) $(DATA_STREAMER_H) $(TREE_STREAMER_H) $(TREE_PASS_H) \
+   profile.h $(CONTEXT_H) $(PIPELINE_H)
 lto-streamer-in.o: lto-streamer-in.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
    $(TM_H) toplev.h $(DIAGNOSTIC_CORE_H) $(EXPR_H) $(FLAGS_H) $(PARAMS_H) \
    input.h $(HASHTAB_H) $(BASIC_BLOCK_H) $(TREE_FLOW_H) $(TREE_PASS_H) \
@@ -2745,7 +2747,8 @@  passes.o : passes.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
    hosthooks.h $(CGRAPH_H) $(COVERAGE_H) $(TREE_PASS_H) $(TREE_DUMP_H) \
    $(GGC_H) $(OPTS_H) $(TREE_FLOW_H) $(TREE_INLINE_H) \
    gt-passes.h $(DF_H) $(PREDICT_H) $(LTO_STREAMER_H) \
-   $(PLUGIN_H) $(IPA_UTILS_H) passes.def
+   $(PLUGIN_H) $(IPA_UTILS_H) passes.def \
+   $(CONTEXT_H) $(PIPELINE_H)
 
 plugin.o : plugin.c $(PLUGIN_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h \
    $(HASH_TABLE_H) $(DIAGNOSTIC_CORE_H) $(TREE_H) $(TREE_PASS_H) \
@@ -2786,7 +2789,8 @@  function.o : function.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_ERROR_
    $(TREE_PASS_H) $(DF_H) $(PARAMS_H) bb-reorder.h \
    $(COMMON_TARGET_H)
 statistics.o : statistics.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
-   $(TREE_PASS_H) $(TREE_DUMP_H) $(HASH_TABLE_H) statistics.h $(FUNCTION_H)
+   $(TREE_PASS_H) $(TREE_DUMP_H) $(HASH_TABLE_H) statistics.h \
+   $(FUNCTION_H) $(CONTEXT_H) $(PIPELINE_H)
 stmt.o : stmt.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(DUMPFILE_H) $(TM_H) \
    $(RTL_H) \
    $(TREE_H) $(FLAGS_H) $(FUNCTION_H) insn-config.h hard-reg-set.h $(EXPR_H) \
@@ -2908,7 +2912,8 @@  cgraphunit.o : cgraphunit.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
    $(FIBHEAP_H) output.h $(PARAMS_H) $(RTL_H) $(IPA_PROP_H) \
    gt-cgraphunit.h tree-iterator.h $(COVERAGE_H) $(TREE_DUMP_H) \
    $(GIMPLE_PRETTY_PRINT_H) $(IPA_INLINE_H) $(IPA_UTILS_H) $(CFGLOOP_H) \
-   $(LTO_STREAMER_H) output.h $(REGSET_H) $(EXCEPT_H) $(GCC_PLUGIN_H) plugin.h
+   $(LTO_STREAMER_H) output.h $(REGSET_H) $(EXCEPT_H) $(GCC_PLUGIN_H) \
+   plugin.h $(CONTEXT_H) $(PIPELINE_H)
 cgraphclones.o : cgraphclones.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
    $(TREE_H) langhooks.h $(TREE_INLINE_H) toplev.h $(DIAGNOSTIC_CORE_H) $(FLAGS_H) $(GGC_H) \
    $(TARGET_H) $(CGRAPH_H) intl.h pointer-set.h $(FUNCTION_H) $(GIMPLE_H) \
@@ -3490,7 +3495,7 @@  $(out_object_file): $(out_file) $(CONFIG_H) coretypes.h $(TM_H) $(TREE_H) \
 	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
 		$(out_file) $(OUTPUT_OPTION)
 context.o: context.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(GGC_H) \
-   $(CONTEXT_H)
+   $(CONTEXT_H) $(PIPELINE_H)
 
 $(common_out_object_file): $(common_out_file) $(CONFIG_H) $(SYSTEM_H) \
     coretypes.h $(COMMON_TARGET_H) $(COMMON_TARGET_DEF_H) $(PARAMS_H) \
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index b82c2e0..dc489fb 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -194,6 +194,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "except.h"
 #include "cfgloop.h"
 #include "regset.h"     /* FIXME: For reg_obstack.  */
+#include "context.h"
+#include "pipeline.h"
 
 /* Queue of cgraph nodes scheduled to be added into cgraph.  This is a
    secondary queue used during optimization to accommodate passes that
@@ -478,6 +480,7 @@  cgraph_finalize_function (tree decl, bool nested)
 void
 cgraph_add_new_function (tree fndecl, bool lowered)
 {
+  gcc::pipeline &passes = g->get_passes ();
   struct cgraph_node *node;
   switch (cgraph_state)
     {
@@ -508,7 +511,7 @@  cgraph_add_new_function (tree fndecl, bool lowered)
 	    push_cfun (DECL_STRUCT_FUNCTION (fndecl));
 	    gimple_register_cfg_hooks ();
 	    bitmap_obstack_initialize (NULL);
-	    execute_pass_list (all_lowering_passes);
+	    execute_pass_list (passes.all_lowering_passes);
 	    execute_pass_list (pass_early_local_passes.pass.sub);
 	    bitmap_obstack_release (NULL);
 	    pop_cfun ();
@@ -640,7 +643,7 @@  analyze_function (struct cgraph_node *node)
 
 	  gimple_register_cfg_hooks ();
 	  bitmap_obstack_initialize (NULL);
-	  execute_pass_list (all_lowering_passes);
+	  execute_pass_list (g->get_passes ().all_lowering_passes);
 	  free_dominance_info (CDI_POST_DOMINATORS);
 	  free_dominance_info (CDI_DOMINATORS);
 	  compact_blocks ();
@@ -1588,7 +1591,7 @@  expand_function (struct cgraph_node *node)
   /* Signal the start of passes.  */
   invoke_plugin_callbacks (PLUGIN_ALL_PASSES_START, NULL);
 
-  execute_pass_list (all_passes);
+  execute_pass_list (g->get_passes ().all_passes);
 
   /* Signal the end of passes.  */
   invoke_plugin_callbacks (PLUGIN_ALL_PASSES_END, NULL);
@@ -1807,6 +1810,8 @@  output_in_order (void)
 static void
 ipa_passes (void)
 {
+  gcc::pipeline &passes = g->get_passes ();
+
   set_cfun (NULL);
   current_function_decl = NULL;
   gimple_register_cfg_hooks ();
@@ -1816,7 +1821,7 @@  ipa_passes (void)
 
   if (!in_lto_p)
     {
-      execute_ipa_pass_list (all_small_ipa_passes);
+      execute_ipa_pass_list (passes.all_small_ipa_passes);
       if (seen_error ())
 	return;
     }
@@ -1843,14 +1848,15 @@  ipa_passes (void)
       cgraph_process_new_functions ();
 
       execute_ipa_summary_passes
-	((struct ipa_opt_pass_d *) all_regular_ipa_passes);
+	((struct ipa_opt_pass_d *) passes.all_regular_ipa_passes);
     }
 
   /* Some targets need to handle LTO assembler output specially.  */
   if (flag_generate_lto)
     targetm.asm_out.lto_start ();
 
-  execute_ipa_summary_passes ((struct ipa_opt_pass_d *) all_lto_gen_passes);
+  execute_ipa_summary_passes ((struct ipa_opt_pass_d *)
+			      passes.all_lto_gen_passes);
 
   if (!in_lto_p)
     ipa_write_summaries ();
@@ -1859,7 +1865,7 @@  ipa_passes (void)
     targetm.asm_out.lto_end ();
 
   if (!flag_ltrans && (in_lto_p || !flag_lto || flag_fat_lto_objects))
-    execute_ipa_pass_list (all_regular_ipa_passes);
+    execute_ipa_pass_list (passes.all_regular_ipa_passes);
   invoke_plugin_callbacks (PLUGIN_ALL_IPA_PASSES_END, NULL);
 
   bitmap_obstack_release (NULL);
@@ -1985,7 +1991,7 @@  compile (void)
 
   cgraph_materialize_all_clones ();
   bitmap_obstack_initialize (NULL);
-  execute_ipa_pass_list (all_late_ipa_passes);
+  execute_ipa_pass_list (g->get_passes ().all_late_ipa_passes);
   symtab_remove_unreachable_nodes (true, dump_file);
 #ifdef ENABLE_CHECKING
   verify_symtab ();
diff --git a/gcc/context.c b/gcc/context.c
index 76e0dde..8ec2e60 100644
--- a/gcc/context.c
+++ b/gcc/context.c
@@ -22,6 +22,12 @@  along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "ggc.h"
 #include "context.h"
+#include "pipeline.h"
 
 /* The singleton holder of global state: */
 gcc::context *g;
+
+gcc::context::context()
+{
+  passes_ = new gcc::pipeline(this);
+}
diff --git a/gcc/context.h b/gcc/context.h
index 3caf02f..a83f7b2 100644
--- a/gcc/context.h
+++ b/gcc/context.h
@@ -22,14 +22,23 @@  along with GCC; see the file COPYING3.  If not see
 
 namespace gcc {
 
+class pipeline;
+
 /* GCC's internal state can be divided into zero or more
    "parallel universe" of state; an instance of this class is one such
    context of state.  */
 class context
 {
 public:
+  context();
+
+  /* Pass-management.  */
+
+  pipeline &get_passes () { gcc_assert (passes_); return *passes_; }
 
-  /* Currently empty.  */
+private:
+  /* Pass-management.  */
+  pipeline *passes_;
 
 }; // class context
 
diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index 4a287f6..d322d9a 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -47,6 +47,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "gcov-io.h"
 #include "tree-pass.h"
 #include "profile.h"
+#include "context.h"
+#include "pipeline.h"
 
 static void output_cgraph_opt_summary (void);
 static void input_cgraph_opt_summary (vec<symtab_node>  nodes);
@@ -936,6 +938,7 @@  input_node (struct lto_file_decl_data *file_data,
 	    enum LTO_symtab_tags tag,
 	    vec<symtab_node> nodes)
 {
+  gcc::pipeline &passes = g->get_passes ();
   tree fn_decl;
   struct cgraph_node *node;
   struct bitpack_d bp;
@@ -981,8 +984,8 @@  input_node (struct lto_file_decl_data *file_data,
       struct opt_pass *pass;
       int pid = streamer_read_hwi (ib);
 
-      gcc_assert (pid < passes_by_id_size);
-      pass = passes_by_id[pid];
+      gcc_assert (pid < passes.passes_by_id_size);
+      pass = passes.passes_by_id[pid];
       node->ipa_transforms_to_apply.safe_push ((struct ipa_opt_pass_d *) pass);
     }
 
diff --git a/gcc/lto/Make-lang.in b/gcc/lto/Make-lang.in
index 5f2f475..1acd176 100644
--- a/gcc/lto/Make-lang.in
+++ b/gcc/lto/Make-lang.in
@@ -85,7 +85,8 @@  lto/lto.o: lto/lto.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(OPTS_H) \
 	langhooks.h $(VEC_H) $(BITMAP_H) pointer-set.h $(IPA_PROP_H) \
 	$(COMMON_H) debug.h $(GIMPLE_H) $(LTO_H) $(LTO_TREE_H) \
 	$(LTO_TAGS_H) $(LTO_STREAMER_H) $(SPLAY_TREE_H) gt-lto-lto.h \
-	$(TREE_STREAMER_H) $(DATA_STREAMER_H) lto/lto-partition.h
+	$(TREE_STREAMER_H) $(DATA_STREAMER_H) lto/lto-partition.h \
+	$(CONTEXT_H) $(PIPELINE_H)
 lto/lto-partition.o: lto/lto-partition.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
 	toplev.h $(TREE_H) $(TM_H) \
 	$(CGRAPH_H) $(TIMEVAR_H) \
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index c0f9328..e7ca937 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -46,6 +46,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "splay-tree.h"
 #include "lto-partition.h"
 #include "data-streamer.h"
+#include "context.h"
+#include "pipeline.h"
 
 static GTY(()) tree first_personality_decl;
 
@@ -3694,7 +3696,7 @@  do_whole_program_analysis (void)
   bitmap_obstack_initialize (NULL);
   cgraph_state = CGRAPH_STATE_IPA_SSA;
 
-  execute_ipa_pass_list (all_regular_ipa_passes);
+  execute_ipa_pass_list (g->get_passes ().all_regular_ipa_passes);
   symtab_remove_unreachable_nodes (false, dump_file);
 
   if (cgraph_dump_file)
diff --git a/gcc/passes.c b/gcc/passes.c
index 94fb586..e625bf2 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -70,6 +70,10 @@  along with GCC; see the file COPYING3.  If not see
 #include "plugin.h"
 #include "ipa-utils.h"
 #include "tree-pretty-print.h" /* for dump_function_header */
+#include "context.h"
+#include "pipeline.h"
+
+using namespace gcc;
 
 /* This is used for debugging.  It allows the current pass to printed
    from anywhere in compilation.
@@ -439,23 +443,11 @@  static struct rtl_opt_pass pass_postreload =
 
 
 
-/* The root of the compilation pass tree, once constructed.  */
-struct opt_pass *all_passes, *all_small_ipa_passes, *all_lowering_passes,
-  *all_regular_ipa_passes, *all_late_ipa_passes, *all_lto_gen_passes;
-
-/* This is used by plugins, and should also be used in register_pass.  */
-#define DEF_PASS_LIST(LIST) &LIST,
-struct opt_pass **gcc_pass_lists[] = { GCC_PASS_LISTS NULL };
-#undef DEF_PASS_LIST
-
-/* A map from static pass id to optimization pass.  */
-struct opt_pass **passes_by_id;
-int passes_by_id_size;
-
 /* Set the static pass number of pass PASS to ID and record that
    in the mapping from static pass number to pass.  */
 
-static void
+void
+pipeline::
 set_pass_for_id (int id, struct opt_pass *pass)
 {
   pass->static_pass_number = id;
@@ -472,7 +464,7 @@  set_pass_for_id (int id, struct opt_pass *pass)
 /* Return the pass with the static pass number ID.  */
 
 struct opt_pass *
-get_pass_for_id (int id)
+pipeline::get_pass_for_id (int id) const
 {
   if (id >= passes_by_id_size)
     return NULL;
@@ -486,6 +478,12 @@  get_pass_for_id (int id)
 void
 register_one_dump_file (struct opt_pass *pass)
 {
+  g->get_passes ().register_one_dump_file (pass);
+}
+
+void
+pipeline::register_one_dump_file (struct opt_pass *pass)
+{
   char *dot_name, *flag_name, *glob_name;
   const char *name, *full_name, *prefix;
   char num[10];
@@ -535,7 +533,8 @@  register_one_dump_file (struct opt_pass *pass)
 
 /* Recursive worker function for register_dump_files.  */
 
-static int
+int
+pipeline::
 register_dump_files_1 (struct opt_pass *pass, int properties)
 {
   do
@@ -567,7 +566,8 @@  register_dump_files_1 (struct opt_pass *pass, int properties)
    PROPERTIES reflects the properties that are guaranteed to be available at
    the beginning of the pipeline.  */
 
-static void
+void
+pipeline::
 register_dump_files (struct opt_pass *pass,int properties)
 {
   pass->properties_required |= properties;
@@ -663,7 +663,7 @@  create_pass_tab (void)
   if (!flag_dump_passes)
     return;
 
-  pass_tab.safe_grow_cleared (passes_by_id_size + 1);
+  pass_tab.safe_grow_cleared (g->get_passes ().passes_by_id_size + 1);
   name_to_pass_map.traverse <void *, passes_pass_traverse> (NULL);
 }
 
@@ -714,6 +714,12 @@  dump_pass_list (struct opt_pass *pass, int indent)
 void
 dump_passes (void)
 {
+  g->get_passes ().dump_passes ();
+}
+
+void
+pipeline::dump_passes () const
+{
   struct cgraph_node *n, *node = NULL;
 
   create_pass_tab();
@@ -1188,6 +1194,13 @@  position_pass (struct register_pass_info *new_pass_info,
 void
 register_pass (struct register_pass_info *pass_info)
 {
+  g->get_passes ().register_pass (pass_info);
+
+}
+
+void
+pipeline::register_pass (struct register_pass_info *pass_info)
+{
   bool all_instances, success;
 
   /* The checks below could fail in buggy plugins.  Existing GCC
@@ -1277,11 +1290,21 @@  register_pass (struct register_pass_info *pass_info)
 				        -> all_passes
 */
 
-void
-init_optimization_passes (void)
+pipeline::pipeline (context *ctxt)
+: all_passes(NULL), all_small_ipa_passes(NULL), all_lowering_passes(NULL),
+  all_regular_ipa_passes(NULL), all_lto_gen_passes(NULL),
+  all_late_ipa_passes(NULL), passes_by_id(NULL), passes_by_id_size(0),
+  ctxt_(ctxt)
 {
   struct opt_pass **p;
 
+  /* Initialize the pass_lists array.  */
+#define DEF_PASS_LIST(LIST) pass_lists[PASS_LIST_NO_##LIST] = &LIST;
+  GCC_PASS_LISTS
+#undef DEF_PASS_LIST
+
+  /* Build the tree of passes.  */
+
 #define INSERT_PASSES_AFTER(PASS) \
   p = &(PASS);
 
@@ -1432,12 +1455,13 @@  static struct profile_record *profile_record;
 static void
 check_profile_consistency (int index, int subpass, bool run)
 {
+  pipeline &passes = g->get_passes ();
   if (index == -1)
     return;
   if (!profile_record)
     profile_record = XCNEWVEC (struct profile_record,
-			       passes_by_id_size);
-  gcc_assert (index < passes_by_id_size && index >= 0);
+			       passes.passes_by_id_size);
+  gcc_assert (index < passes.passes_by_id_size && index >= 0);
   gcc_assert (subpass < 2);
   profile_record[index].run |= run;
   account_profile_record (&profile_record[index], subpass);
@@ -1448,6 +1472,12 @@  check_profile_consistency (int index, int subpass, bool run)
 void
 dump_profile_report (void)
 {
+  g->get_passes ().dump_profile_report ();
+}
+
+void
+pipeline::dump_profile_report () const
+{
   int i, j;
   int last_freq_in = 0, last_count_in = 0, last_freq_out = 0, last_count_out = 0;
   gcov_type last_time = 0, last_size = 0;
@@ -2067,14 +2097,15 @@  ipa_write_summaries_2 (struct opt_pass *pass, struct lto_out_decl_state *state)
 static void
 ipa_write_summaries_1 (lto_symtab_encoder_t encoder)
 {
+  pipeline &passes = g->get_passes ();
   struct lto_out_decl_state *state = lto_new_out_decl_state ();
   state->symtab_node_encoder = encoder;
 
   lto_push_out_decl_state (state);
 
   gcc_assert (!flag_wpa);
-  ipa_write_summaries_2 (all_regular_ipa_passes, state);
-  ipa_write_summaries_2 (all_lto_gen_passes, state);
+  ipa_write_summaries_2 (passes.all_regular_ipa_passes, state);
+  ipa_write_summaries_2 (passes.all_lto_gen_passes, state);
 
   gcc_assert (lto_get_out_decl_state () == state);
   lto_pop_out_decl_state ();
@@ -2205,8 +2236,9 @@  ipa_write_optimization_summaries (lto_symtab_encoder_t encoder)
     }
 
   gcc_assert (flag_wpa);
-  ipa_write_optimization_summaries_1 (all_regular_ipa_passes, state);
-  ipa_write_optimization_summaries_1 (all_lto_gen_passes, state);
+  pipeline &passes = g->get_passes ();
+  ipa_write_optimization_summaries_1 (passes.all_regular_ipa_passes, state);
+  ipa_write_optimization_summaries_1 (passes.all_lto_gen_passes, state);
 
   gcc_assert (lto_get_out_decl_state () == state);
   lto_pop_out_decl_state ();
@@ -2259,8 +2291,9 @@  ipa_read_summaries_1 (struct opt_pass *pass)
 void
 ipa_read_summaries (void)
 {
-  ipa_read_summaries_1 (all_regular_ipa_passes);
-  ipa_read_summaries_1 (all_lto_gen_passes);
+  pipeline &passes = g->get_passes ();
+  ipa_read_summaries_1 (passes.all_regular_ipa_passes);
+  ipa_read_summaries_1 (passes.all_lto_gen_passes);
 }
 
 /* Same as execute_pass_list but assume that subpasses of IPA passes
@@ -2308,8 +2341,9 @@  ipa_read_optimization_summaries_1 (struct opt_pass *pass)
 void
 ipa_read_optimization_summaries (void)
 {
-  ipa_read_optimization_summaries_1 (all_regular_ipa_passes);
-  ipa_read_optimization_summaries_1 (all_lto_gen_passes);
+  pipeline &passes = g->get_passes ();
+  ipa_read_optimization_summaries_1 (passes.all_regular_ipa_passes);
+  ipa_read_optimization_summaries_1 (passes.all_lto_gen_passes);
 }
 
 /* Same as execute_pass_list but assume that subpasses of IPA passes
@@ -2384,7 +2418,8 @@  execute_ipa_stmt_fixups (struct opt_pass *pass,
 void
 execute_all_ipa_stmt_fixups (struct cgraph_node *node, gimple *stmts)
 {
-  execute_ipa_stmt_fixups (all_regular_ipa_passes, node, stmts);
+  pipeline &passes = g->get_passes ();
+  execute_ipa_stmt_fixups (passes.all_regular_ipa_passes, node, stmts);
 }
 
 
diff --git a/gcc/pipeline.h b/gcc/pipeline.h
new file mode 100644
index 0000000..37c90d7
--- /dev/null
+++ b/gcc/pipeline.h
@@ -0,0 +1,89 @@ 
+/* pipeline.h - The pipeline of optimization passes
+   Copyright (C) 2013 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_PIPELINE_H
+#define GCC_PIPELINE_H
+
+class opt_pass;
+struct register_pass_info;
+
+/* Define a list of pass lists so that both passes.c and plugins can easily
+   find all the pass lists.  */
+#define GCC_PASS_LISTS \
+  DEF_PASS_LIST (all_lowering_passes) \
+  DEF_PASS_LIST (all_small_ipa_passes) \
+  DEF_PASS_LIST (all_regular_ipa_passes) \
+  DEF_PASS_LIST (all_lto_gen_passes) \
+  DEF_PASS_LIST (all_passes)
+
+#define DEF_PASS_LIST(LIST) PASS_LIST_NO_##LIST,
+enum pass_list
+{
+  GCC_PASS_LISTS
+  PASS_LIST_NUM
+};
+#undef DEF_PASS_LIST
+
+namespace gcc {
+
+class context;
+
+class pipeline
+{
+public:
+  pipeline(context *ctxt);
+
+  void register_pass (struct register_pass_info *pass_info);
+  void register_one_dump_file (struct opt_pass *pass);
+
+  opt_pass *get_pass_for_id (int id) const;
+
+  void dump_passes () const;
+
+  void dump_profile_report () const;
+
+public:
+  /* The root of the compilation pass tree, once constructed.  */
+  opt_pass *all_passes;
+  opt_pass *all_small_ipa_passes;
+  opt_pass *all_lowering_passes;
+  opt_pass *all_regular_ipa_passes;
+  opt_pass *all_lto_gen_passes;
+  opt_pass *all_late_ipa_passes;
+
+  /* A map from static pass id to optimization pass.  */
+  opt_pass **passes_by_id;
+  int passes_by_id_size;
+
+  opt_pass **pass_lists[PASS_LIST_NUM];
+
+private:
+  void set_pass_for_id (int id, opt_pass *pass);
+  int register_dump_files_1 (struct opt_pass *pass, int properties);
+  void register_dump_files (struct opt_pass *pass, int properties);
+
+private:
+  context *ctxt_;
+
+}; // class pipeline
+
+} // namespace gcc
+
+#endif /* ! GCC_PIPELINE_H */
+
diff --git a/gcc/statistics.c b/gcc/statistics.c
index 3077cc0..c7e6abd 100644
--- a/gcc/statistics.c
+++ b/gcc/statistics.c
@@ -26,6 +26,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "statistics.h"
 #include "hash-table.h"
 #include "function.h"
+#include "context.h"
+#include "pipeline.h"
 
 static int statistics_dump_nr;
 static int statistics_dump_flags;
@@ -235,6 +237,7 @@  statistics_fini_1 (statistics_counter_t **slot, opt_pass *pass)
 void
 statistics_fini (void)
 {
+  gcc::pipeline &passes = g->get_passes ();
   if (!statistics_dump_file)
     return;
 
@@ -243,10 +246,10 @@  statistics_fini (void)
       unsigned i;
       for (i = 0; i < nr_statistics_hashes; ++i)
 	if (statistics_hashes[i].is_created ()
-	    && get_pass_for_id (i) != NULL)
+	    && passes.get_pass_for_id (i) != NULL)
 	  statistics_hashes[i]
 	    .traverse_noresize <opt_pass *, statistics_fini_1>
-	    (get_pass_for_id (i));
+	    (passes.get_pass_for_id (i));
     }
 
   dump_end (statistics_dump_nr, statistics_dump_file);
diff --git a/gcc/toplev.c b/gcc/toplev.c
index de28a2d..1cf98dc 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1158,10 +1158,10 @@  general_init (const char *argv0)
      processing.  */
   init_ggc_heuristics();
 
-  /* Create the singleton holder for global state.  */
+  /* Create the singleton holder for global state.
+     Doing so also creates the pipeline of passes.  */
   g = new gcc::context();
 
-  init_optimization_passes ();
   statistics_early_init ();
   finish_params ();
 }
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 547f355..16442ed 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -489,35 +489,9 @@  extern struct gimple_opt_pass pass_inline_parameters;
 extern struct gimple_opt_pass pass_update_address_taken;
 extern struct gimple_opt_pass pass_convert_switch;
 
-/* The root of the compilation pass tree, once constructed.  */
-extern struct opt_pass *all_passes, *all_small_ipa_passes, *all_lowering_passes,
-                       *all_regular_ipa_passes, *all_lto_gen_passes, *all_late_ipa_passes;
-
-/* Define a list of pass lists so that both passes.c and plugins can easily
-   find all the pass lists.  */
-#define GCC_PASS_LISTS \
-  DEF_PASS_LIST (all_lowering_passes) \
-  DEF_PASS_LIST (all_small_ipa_passes) \
-  DEF_PASS_LIST (all_regular_ipa_passes) \
-  DEF_PASS_LIST (all_lto_gen_passes) \
-  DEF_PASS_LIST (all_passes)
-
-#define DEF_PASS_LIST(LIST) PASS_LIST_NO_##LIST,
-enum
-{
-  GCC_PASS_LISTS
-  PASS_LIST_NUM
-};
-#undef DEF_PASS_LIST
-
-/* This is used by plugins, and should also be used in
-   passes.c:register_pass.  */
-extern struct opt_pass **gcc_pass_lists[];
-
 /* Current optimization pass.  */
 extern struct opt_pass *current_pass;
 
-extern struct opt_pass * get_pass_for_id (int);
 extern bool execute_one_pass (struct opt_pass *);
 extern void execute_pass_list (struct opt_pass *);
 extern void execute_ipa_pass_list (struct opt_pass *);
@@ -547,9 +521,6 @@  extern void register_pass (struct register_pass_info *);
    directly in jump threading, and avoid peeling them next time.  */
 extern bool first_pass_instance;
 
-extern struct opt_pass **passes_by_id;
-extern int passes_by_id_size;
-
 /* Declare for plugins.  */
 extern void do_per_function_toporder (void (*) (void *), void *);