On Fri, 28 Sep 2018, David Malcolm wrote: > Attempts to dump via -fopt-info from a plugin pass fail, due > to the dfi->alt_state for such passes never being set. > > This is because the -fopt-info options were being set up per-pass > during option-parsing (via gcc::dump_manager::opt_info_enable_passes), > but this data was not retained or used it for passes created later > (for plugins and target-specific passes). > > This patch fixes the issue by storing the -fopt-info options into > gcc::dump_manager, refactoring the dfi-setup code out of > opt_info_enable_passes, and reusing it for such passes, fixing the > issue. The patch adds a demo plugin to test that dumping from a > plugin works. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > OK for trunk?
OK. Richard. > gcc/ChangeLog: > * dumpfile.c (gcc::dump_manager::dump_manager): Initialize new > fields. > (gcc::dump_manager::~dump_manager): Free m_optinfo_filename. > (gcc::dump_manager::register_pass): New member function, adapted > from loop body in gcc::pass_manager::register_pass, adding a > call to update_dfi_for_opt_info. > (gcc::dump_manager::opt_info_enable_passes): Store the > -fopt-info options into the new fields. Move the loop > bodies into... > (gcc::dump_manager::update_dfi_for_opt_info): ...this new member > function. > * dumpfile.h (struct opt_pass): New forward decl. > (gcc::dump_manager::register_pass): New decl. > (gcc::dump_manager::update_dfi_for_opt_info): New decl. > (class gcc::dump_manager): Add fields "m_optgroup_flags", > "m_optinfo_flags", and "m_optinfo_filename". > * passes.c (gcc::pass_manager::register_pass): Move all of the > dump-handling code to gcc::dump_manager::register_pass. > > gcc/testsuite/ChangeLog: > * gcc.dg/plugin/dump-1.c: New test. > * gcc.dg/plugin/dump_plugin.c: New test plugin. > * gcc.dg/plugin/plugin.exp (plugin_test_list): Add the above. > --- > gcc/dumpfile.c | 120 +++++++++++++++++-------- > gcc/dumpfile.h | 12 +++ > gcc/passes.c | 30 ++----- > gcc/testsuite/gcc.dg/plugin/dump-1.c | 28 ++++++ > gcc/testsuite/gcc.dg/plugin/dump_plugin.c | 143 > ++++++++++++++++++++++++++++++ > gcc/testsuite/gcc.dg/plugin/plugin.exp | 2 + > 6 files changed, 275 insertions(+), 60 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/plugin/dump-1.c > create mode 100644 gcc/testsuite/gcc.dg/plugin/dump_plugin.c > > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c > index d430ea3..d359e41 100644 > --- a/gcc/dumpfile.c > +++ b/gcc/dumpfile.c > @@ -177,12 +177,16 @@ gcc::dump_manager::dump_manager (): > m_next_dump (FIRST_AUTO_NUMBERED_DUMP), > m_extra_dump_files (NULL), > m_extra_dump_files_in_use (0), > - m_extra_dump_files_alloced (0) > + m_extra_dump_files_alloced (0), > + m_optgroup_flags (OPTGROUP_NONE), > + m_optinfo_flags (TDF_NONE), > + m_optinfo_filename (NULL) > { > } > > gcc::dump_manager::~dump_manager () > { > + free (m_optinfo_filename); > for (size_t i = 0; i < m_extra_dump_files_in_use; i++) > { > dump_file_info *dfi = &m_extra_dump_files[i]; > @@ -1512,6 +1516,50 @@ dump_flag_name (int phase) const > return dfi->swtch; > } > > +/* Handle -fdump-* and -fopt-info for a pass added after > + command-line options are parsed (those from plugins and > + those from backends). > + > + Because the registration of plugin/backend passes happens after the > + command-line options are parsed, the options that specify single > + pass dumping (e.g. -fdump-tree-PASSNAME) cannot be used for new > + passes. Therefore we currently can only enable dumping of > + new passes when the 'dump-all' flags (e.g. -fdump-tree-all) > + are specified. This is done here. > + > + Similarly, the saved -fopt-info options are wired up to the new pass. */ > + > +void > +gcc::dump_manager::register_pass (opt_pass *pass) > +{ > + gcc_assert (pass); > + > + register_one_dump_file (pass); > + > + dump_file_info *pass_dfi = get_dump_file_info (pass->static_pass_number); > + gcc_assert (pass_dfi); > + > + enum tree_dump_index tdi; > + if (pass->type == SIMPLE_IPA_PASS > + || pass->type == IPA_PASS) > + tdi = TDI_ipa_all; > + else if (pass->type == GIMPLE_PASS) > + tdi = TDI_tree_all; > + else > + tdi = TDI_rtl_all; > + const dump_file_info *tdi_dfi = get_dump_file_info (tdi); > + gcc_assert (tdi_dfi); > + > + /* Check if dump-all flag is specified. */ > + if (tdi_dfi->pstate) > + { > + pass_dfi->pstate = tdi_dfi->pstate; > + pass_dfi->pflags = tdi_dfi->pflags; > + } > + > + update_dfi_for_opt_info (pass_dfi); > +} > + > /* Finish a tree dump for PHASE. STREAM is the stream created by > dump_begin. */ > > @@ -1587,47 +1635,47 @@ opt_info_enable_passes (optgroup_flags_t > optgroup_flags, dump_flags_t flags, > const char *filename) > { > int n = 0; > - size_t i; > > - for (i = TDI_none + 1; i < (size_t) TDI_end; i++) > - { > - if ((dump_files[i].optgroup_flags & optgroup_flags)) > - { > - const char *old_filename = dump_files[i].alt_filename; > - /* Since this file is shared among different passes, it > - should be opened in append mode. */ > - dump_files[i].alt_state = 1; > - dump_files[i].alt_flags |= flags; > - n++; > - /* Override the existing filename. */ > - if (filename) > - dump_files[i].alt_filename = xstrdup (filename); > - if (old_filename && filename != old_filename) > - free (CONST_CAST (char *, old_filename)); > - } > - } > + m_optgroup_flags = optgroup_flags; > + m_optinfo_flags = flags; > + m_optinfo_filename = xstrdup (filename); > > - for (i = 0; i < m_extra_dump_files_in_use; i++) > - { > - if ((m_extra_dump_files[i].optgroup_flags & optgroup_flags)) > - { > - const char *old_filename = m_extra_dump_files[i].alt_filename; > - /* Since this file is shared among different passes, it > - should be opened in append mode. */ > - m_extra_dump_files[i].alt_state = 1; > - m_extra_dump_files[i].alt_flags |= flags; > - n++; > - /* Override the existing filename. */ > - if (filename) > - m_extra_dump_files[i].alt_filename = xstrdup (filename); > - if (old_filename && filename != old_filename) > - free (CONST_CAST (char *, old_filename)); > - } > - } > + for (size_t i = TDI_none + 1; i < (size_t) TDI_end; i++) > + if (update_dfi_for_opt_info (&dump_files[i])) > + n++; > + > + for (size_t i = 0; i < m_extra_dump_files_in_use; i++) > + if (update_dfi_for_opt_info (&m_extra_dump_files[i])) > + n++; > > return n; > } > > +/* Use the saved -fopt-info options to update DFI. > + Return true if the dump is enabled. */ > + > +bool > +gcc::dump_manager::update_dfi_for_opt_info (dump_file_info *dfi) const > +{ > + gcc_assert (dfi); > + > + if (!(dfi->optgroup_flags & m_optgroup_flags)) > + return false; > + > + const char *old_filename = dfi->alt_filename; > + /* Since this file is shared among different passes, it > + should be opened in append mode. */ > + dfi->alt_state = 1; > + dfi->alt_flags |= m_optinfo_flags; > + /* Override the existing filename. */ > + if (m_optinfo_filename) > + dfi->alt_filename = xstrdup (m_optinfo_filename); > + if (old_filename && m_optinfo_filename != old_filename) > + free (CONST_CAST (char *, old_filename)); > + > + return true; > +} > + > /* Parse ARG as a dump switch. Return nonzero if it is, and store the > relevant details in the dump_files array. */ > > diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h > index 671b7b9..057ca46 100644 > --- a/gcc/dumpfile.h > +++ b/gcc/dumpfile.h > @@ -566,6 +566,8 @@ extern void dump_combine_total_stats (FILE *); > /* In cfghooks.c */ > extern void dump_bb (FILE *, basic_block, int, dump_flags_t); > > +struct opt_pass; > + > namespace gcc { > > /* A class for managing all of the various dump files used by the > @@ -634,6 +636,8 @@ public: > const char * > dump_flag_name (int phase) const; > > + void register_pass (opt_pass *pass); > + > private: > > int > @@ -649,6 +653,8 @@ private: > opt_info_enable_passes (optgroup_flags_t optgroup_flags, dump_flags_t > flags, > const char *filename); > > + bool update_dfi_for_opt_info (dump_file_info *dfi) const; > + > private: > > /* Dynamically registered dump files and switches. */ > @@ -657,6 +663,12 @@ private: > size_t m_extra_dump_files_in_use; > size_t m_extra_dump_files_alloced; > > + /* Stored values from -fopt-info, for handling passes created after > + option-parsing (by backends and by plugins). */ > + optgroup_flags_t m_optgroup_flags; > + dump_flags_t m_optinfo_flags; > + char *m_optinfo_filename; > + > /* Grant access to dump_enable_all. */ > friend bool ::enable_rtl_dump_file (void); > > diff --git a/gcc/passes.c b/gcc/passes.c > index 832f0b3..d838d90 100644 > --- a/gcc/passes.c > +++ b/gcc/passes.c > @@ -1404,7 +1404,6 @@ void > pass_manager::register_pass (struct register_pass_info *pass_info) > { > bool all_instances, success; > - gcc::dump_manager *dumps = m_ctxt->get_dumps (); > > /* The checks below could fail in buggy plugins. Existing GCC > passes should never fail these checks, so we mention plugin in > @@ -1442,33 +1441,16 @@ pass_manager::register_pass (struct > register_pass_info *pass_info) > > /* OK, we have successfully inserted the new pass. We need to register > the dump files for the newly added pass and its duplicates (if any). > - Because the registration of plugin/backend passes happens after the > - command-line options are parsed, the options that specify single > - pass dumping (e.g. -fdump-tree-PASSNAME) cannot be used for new > - passes. Therefore we currently can only enable dumping of > - new passes when the 'dump-all' flags (e.g. -fdump-tree-all) > - are specified. While doing so, we also delete the pass_list_node > + While doing so, we also delete the pass_list_node > objects created during pass positioning. */ > + gcc::dump_manager *dumps = m_ctxt->get_dumps (); > while (added_pass_nodes) > { > struct pass_list_node *next_node = added_pass_nodes->next; > - enum tree_dump_index tdi; > - register_one_dump_file (added_pass_nodes->pass); > - if (added_pass_nodes->pass->type == SIMPLE_IPA_PASS > - || added_pass_nodes->pass->type == IPA_PASS) > - tdi = TDI_ipa_all; > - else if (added_pass_nodes->pass->type == GIMPLE_PASS) > - tdi = TDI_tree_all; > - else > - tdi = TDI_rtl_all; > - /* Check if dump-all flag is specified. */ > - if (dumps->get_dump_file_info (tdi)->pstate) > - { > - dumps->get_dump_file_info (added_pass_nodes->pass->static_pass_number) > - ->pstate = dumps->get_dump_file_info (tdi)->pstate; > - dumps->get_dump_file_info (added_pass_nodes->pass->static_pass_number) > - ->pflags = dumps->get_dump_file_info (tdi)->pflags; > - } > + > + /* Handle -fdump-* and -fopt-info. */ > + dumps->register_pass (added_pass_nodes->pass); > + > XDELETE (added_pass_nodes); > added_pass_nodes = next_node; > } > diff --git a/gcc/testsuite/gcc.dg/plugin/dump-1.c > b/gcc/testsuite/gcc.dg/plugin/dump-1.c > new file mode 100644 > index 0000000..165a9c1 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/plugin/dump-1.c > @@ -0,0 +1,28 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fopt-info-note" } */ > + > +extern void test_string_literal (void); > +extern void test_tree (void); > +extern void test_gimple (int); > +extern void test_cgraph_node (void); > +extern void test_wide_int (void); > +extern void test_poly_int (void); > +extern void test_scopes (void); > + > +void test_remarks (void) > +{ > + test_string_literal (); /* { dg-message "test of dump for > 'test_string_literal'" } */ > + test_tree (); /* { dg-message "test of tree: 0" } */ > + test_gimple (42); /* { dg-message "test of gimple: test_gimple \\(42\\);" > } */ > + test_cgraph_node (); /* { dg-message "test of callgraph node: > test_cgraph_node/\[0-9\]+" } */ > + test_wide_int (); /* { dg-message "test of wide int: 0" } */ > + test_poly_int (); /* { dg-message "test of poly int: 42" } */ > + > + test_scopes (); /* { dg-line test_scopes_line } */ > + /* { dg-message "=== outer scope ===" "" { target *-*-* } test_scopes_line > } */ > + /* { dg-message " at outer scope" "" { target *-*-* } test_scopes_line } */ > + /* { dg-message " === middle scope ===" "" { target *-*-* } > test_scopes_line } */ > + /* { dg-message " at middle scope" "" { target *-*-* } test_scopes_line } > */ > + /* { dg-message " === innermost scope ===" "" { target *-*-* } > test_scopes_line } */ > + /* { dg-message " at innermost scope" "" { target *-*-* } > test_scopes_line } */ > +} > diff --git a/gcc/testsuite/gcc.dg/plugin/dump_plugin.c > b/gcc/testsuite/gcc.dg/plugin/dump_plugin.c > new file mode 100644 > index 0000000..12573d6 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/plugin/dump_plugin.c > @@ -0,0 +1,143 @@ > +/* Plugin for testing dumpfile.c. */ > + > +#include "gcc-plugin.h" > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "tree.h" > +#include "tree-pass.h" > +#include "intl.h" > +#include "plugin-version.h" > +#include "diagnostic.h" > +#include "context.h" > +#include "optinfo.h" > +#include "gimple.h" > +#include "gimple-iterator.h" > +#include "cgraph.h" > + > +int plugin_is_GPL_compatible; > + > +const pass_data pass_data_test_dumping = > +{ > + GIMPLE_PASS, /* type */ > + "test_dumping", /* name */ > + OPTGROUP_LOOP, /* optinfo_flags */ > + TV_NONE, /* tv_id */ > + PROP_ssa, /* properties_required */ > + 0, /* properties_provided */ > + 0, /* properties_destroyed */ > + 0, /* todo_flags_start */ > + 0, /* todo_flags_finish */ > +}; > + > +class pass_test_dumping : public gimple_opt_pass > +{ > +public: > + pass_test_dumping (gcc::context *ctxt) > + : gimple_opt_pass (pass_data_test_dumping, ctxt) > + {} > + > + /* opt_pass methods: */ > + bool gate (function *) { return true; } > + virtual unsigned int execute (function *); > + > +}; // class pass_test_dumping > + > +unsigned int > +pass_test_dumping::execute (function *fun) > +{ > + basic_block bb; > + > + if (!dump_enabled_p ()) > + return 0; > + > + FOR_ALL_BB_FN (bb, fun) > + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); > + !gsi_end_p (gsi); gsi_next (&gsi)) > + { > + gimple *stmt = gsi_stmt (gsi); > + gcall *call = dyn_cast <gcall *> (stmt); > + if (!call) > + continue; > + tree callee_decl = gimple_call_fndecl (call); > + if (!callee_decl) > + continue; > + tree callee_name = DECL_NAME (callee_decl); > + if (!callee_name) > + continue; > + const char *callee = IDENTIFIER_POINTER (callee_name); > + > + /* Various dumping tests, done at callsites, > + controlled by the callee name. */ > + if (strcmp (callee, "test_string_literal") == 0) > + dump_printf_loc (MSG_NOTE, stmt, "test of dump for %qs\n", > + callee); > + else if (strcmp (callee, "test_tree") == 0) > + dump_printf_loc (MSG_NOTE, stmt, "test of tree: %T\n", > + integer_zero_node); > + else if (strcmp (callee, "test_gimple") == 0) > + dump_printf_loc (MSG_NOTE, stmt, "test of gimple: %G", stmt); > + else if (strcmp (callee, "test_cgraph_node") == 0) > + { > + dump_printf_loc (MSG_NOTE, stmt, "test of callgraph node: "); > + dump_symtab_node (MSG_NOTE, cgraph_node::get (callee_decl)); > + dump_printf (MSG_NOTE, "\n"); > + } > + else if (strcmp (callee, "test_wide_int") == 0) > + { > + HOST_WIDE_INT val = 0; > + dump_printf_loc (MSG_NOTE, stmt, > + "test of wide int: " HOST_WIDE_INT_PRINT_DEC "\n", > + val); > + } > + else if (strcmp (callee, "test_poly_int") == 0) > + { > + dump_printf_loc (MSG_NOTE, stmt, "test of poly int: "); > + dump_dec (MSG_NOTE, poly_int64 (42)); > + dump_printf (MSG_NOTE, "\n"); > + } > + else if (strcmp (callee, "test_scopes") == 0) > + { > + AUTO_DUMP_SCOPE ("outer scope", stmt); > + { > + dump_printf_loc (MSG_NOTE, stmt, "at outer scope\n"); > + AUTO_DUMP_SCOPE ("middle scope", stmt); > + { > + dump_printf_loc (MSG_NOTE, stmt, "at middle scope\n"); > + AUTO_DUMP_SCOPE ("innermost scope", stmt); > + dump_printf_loc (MSG_NOTE, stmt, "at innermost scope\n"); > + } > + } > + } > + } > + > + return 0; > +} > + > +static gimple_opt_pass * > +make_pass_test_dumping (gcc::context *ctxt) > +{ > + return new pass_test_dumping (ctxt); > +} > + > +int > +plugin_init (struct plugin_name_args *plugin_info, > + struct plugin_gcc_version *version) > +{ > + struct register_pass_info pass_info; > + const char *plugin_name = plugin_info->base_name; > + int argc = plugin_info->argc; > + struct plugin_argument *argv = plugin_info->argv; > + > + if (!plugin_default_version_check (version, &gcc_version)) > + return 1; > + > + pass_info.pass = make_pass_test_dumping (g); > + pass_info.reference_pass_name = "ssa"; > + pass_info.ref_pass_instance_number = 1; > + pass_info.pos_op = PASS_POS_INSERT_AFTER; > + register_callback (plugin_name, PLUGIN_PASS_MANAGER_SETUP, NULL, > + &pass_info); > + > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp > b/gcc/testsuite/gcc.dg/plugin/plugin.exp > index 46246a2..50db3ae 100644 > --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp > +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp > @@ -101,6 +101,8 @@ set plugin_test_list [list \ > must-tail-call-2.c } \ > { expensive_selftests_plugin.c \ > expensive-selftests-1.c } \ > + { dump_plugin.c \ > + dump-1.c } \ > ] > > foreach plugin_test $plugin_test_list { > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)