| Message ID | 1374678544-8678-3-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 >
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.
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
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
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
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
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
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
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
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
[ 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
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
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
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
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
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 *);