On Sat, Nov 17, 2018 at 1:12 AM David Malcolm <dmalc...@redhat.com> wrote:
>
> As work towards fixing PR tree-optimization/87025, this patch
> eliminates global state from optinfo-emit-json.cc in favor
> of adding an optional m_json_writer field to dump_context,
> replacing the m_forcibly_enable_optinfo flag.
>
> This allows for writing selftests for the interaction of the
> JSON-building code with the dumpfile.c code.
> In particular, the existing selftest that created optinfo
> instances now exercise the JSON-building code (although no
> JSON is actually written out).
>
> The patch also simplifies the layering by replacing optinfo::emit ()
> with dump_context::emit_optinfo, so that dump_context has
> responsibility for keeping track of dump destinations.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
> (in conjuction with the followup patch)
>
> OK for trunk?
OK.
Richard.
> gcc/ChangeLog:
> PR tree-optimization/87025
> * dump-context.h: Include "optinfo.h".
> (class optrecord_json_writer): New forward decl.
> (dump_context::forcibly_enable_optinfo_p): Delete.
> (dump_context::optinfo_enabled_p): New member function.
> (dump_context::optimization_records_enabled_p): New member
> function.
> (dump_context::set_json_writer): New member function.
> (dump_context::emit_optinfo): New member function.
> (dump_context::m_forcibly_enable_optinfo): Delete.
> (dump_context::m_json_writer): New member data.
> * dumpfile.c (dump_context::set_json_writer): New member function.
> (dump_context::finish_any_json_writer): New member function.
> (dump_context::end_scope): Replace call to
> optimization_records_maybe_pop_dump_scope with call to
> m_json_writer->pop_scope.
> (dump_context::optinfo_enabled_p): New member function.
> (dump_context::end_any_optinfo): Replace call to optinfo::emit with
> call
> to dump_context::emit_optinfo.
> (dump_context::emit_optinfo): New member function.
> (temp_dump_context::temp_dump_context): Replace
> m_forcibly_enable_optinfo with call to set_json_writer.
> (temp_dump_context::~temp_dump_context): Clean up any json writer.
> * optinfo-emit-json.cc (class optrecord_json_writer): Move to
> optinfo-emit-json.h
> (the_json_writer): Delete.
> (optimization_records_start): Delete.
> (optimization_records_finish): Delete.
> (optimization_records_enabled_p): Delete, in favor of
> dump_context::optimization_records_enabled_p.
> (optimization_records_maybe_record_optinfo): Delete.
> (optimization_records_maybe_pop_dump_scope): Delete.
> * optinfo-emit-json.h: Include "json.h". Delete forward
> decl of opt_pass.
> (optimization_records_start): Delete.
> (optimization_records_finish): Delete.
> (optimization_records_enabled_p): Delete.
> (optimization_records_maybe_record_optinfo): Delete.
> (optimization_records_maybe_pop_dump_scope): Delete.
> (class optrecord_json_writer): Move here from
> optinfo-emit-json.cc.
> * optinfo.cc (optinfo::emit_for_opt_problem): Replace call
> to optinfo::emit with call to dump_context::emit_optinfo.
> (optinfo::emit): Delete, in favor of dump_context::emit_optinfo.
> (optinfo_enabled_p): Delete, in favor of
> dump_context::optinfo_enabled_p.
> (optinfo_wants_inlining_info_p): Update for conversion o
> optimization_records_enabled_p to a member function of
> dump_context.
> * optinfo.h (optinfo_enabled_p): Delete, in favor of
> dump_context::optinfo_enabled_p.
> (optinfo::emit): Delete, in favor of dump_context::emit_optinfo.
> * toplev.c: Include "dump-context.h".
> (compile_file): Replace call to optimization_records_finish with
> dump_context::finish_any_json_writer.
> (do_compile): Replace call to optimization_records_start with
> conditionally creating a optrecord_json_writer for the
> dump_context.
> ---
> gcc/dump-context.h | 22 ++++++++---
> gcc/dumpfile.c | 55 ++++++++++++++++++++++++--
> gcc/optinfo-emit-json.cc | 101
> -----------------------------------------------
> gcc/optinfo-emit-json.h | 42 +++++++++++++++-----
> gcc/optinfo.cc | 25 +-----------
> gcc/optinfo.h | 8 ----
> gcc/toplev.c | 8 +++-
> 7 files changed, 109 insertions(+), 152 deletions(-)
>
> diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> index ace139c..2016ce7 100644
> --- a/gcc/dump-context.h
> +++ b/gcc/dump-context.h
> @@ -25,7 +25,9 @@ along with GCC; see the file COPYING3. If not see
> #include "dumpfile.h"
> #include "pretty-print.h"
> #include "selftest.h"
> +#include "optinfo.h"
>
> +class optrecord_json_writer;
> namespace selftest { class temp_dump_context; }
>
> /* A class for handling the various dump_* calls.
> @@ -95,14 +97,21 @@ class dump_context
> void begin_scope (const char *name, const dump_location_t &loc);
> void end_scope ();
>
> - /* For use in selftests; if true then optinfo_enabled_p is true. */
> - bool forcibly_enable_optinfo_p () const
> + /* Should optinfo instances be created?
> + All creation of optinfos should be guarded by this predicate.
> + Return true if any optinfo destinations are active. */
> + bool optinfo_enabled_p () const;
> +
> + bool optimization_records_enabled_p () const
> {
> - return m_forcibly_enable_optinfo;
> + return m_json_writer != NULL;
> }
> + void set_json_writer (optrecord_json_writer *writer);
> + void finish_any_json_writer ();
>
> void end_any_optinfo ();
>
> + void emit_optinfo (const optinfo *info);
> void emit_item (optinfo_item *item, dump_flags_t dump_kind);
>
> bool apply_dump_filter_p (dump_flags_t dump_kind, dump_flags_t filter)
> const;
> @@ -111,9 +120,6 @@ class dump_context
> optinfo &ensure_pending_optinfo ();
> optinfo &begin_next_optinfo (const dump_location_t &loc);
>
> - /* For use in selftests; if true then optinfo_enabled_p is true. */
> - bool m_forcibly_enable_optinfo;
> -
> /* The current nesting depth of dump scopes, for showing nesting
> via indentation). */
> unsigned int m_scope_depth;
> @@ -122,6 +128,10 @@ class dump_context
> if any. */
> optinfo *m_pending;
>
> + /* If -fsave-optimization-record is enabled, the heap-allocated JSON writer
> + instance, otherwise NULL. */
> + optrecord_json_writer *m_json_writer;
> +
> /* For use in selftests: if non-NULL, then items are to be printed
> to this, using the given flags. */
> pretty_printer *m_test_pp;
> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> index 86651df..014acf1 100644
> --- a/gcc/dumpfile.c
> +++ b/gcc/dumpfile.c
> @@ -515,6 +515,28 @@ dump_context::~dump_context ()
> delete m_pending;
> }
>
> +void
> +dump_context::set_json_writer (optrecord_json_writer *writer)
> +{
> + delete m_json_writer;
> + m_json_writer = writer;
> +}
> +
> +/* Perform cleanup activity for -fsave-optimization-record.
> + Currently, the file is written out here in one go, before cleaning
> + up. */
> +
> +void
> +dump_context::finish_any_json_writer ()
> +{
> + if (!m_json_writer)
> + return;
> +
> + m_json_writer->write ();
> + delete m_json_writer;
> + m_json_writer = NULL;
> +}
> +
> /* Update the "dumps_are_enabled" global; to be called whenever dump_file
> or alt_dump_file change, or when changing dump_context in selftests. */
>
> @@ -1121,7 +1143,19 @@ dump_context::end_scope ()
> {
> end_any_optinfo ();
> m_scope_depth--;
> - optimization_records_maybe_pop_dump_scope ();
> +
> + if (m_json_writer)
> + m_json_writer->pop_scope ();
> +}
> +
> +/* Should optinfo instances be created?
> + All creation of optinfos should be guarded by this predicate.
> + Return true if any optinfo destinations are active. */
> +
> +bool
> +dump_context::optinfo_enabled_p () const
> +{
> + return (optimization_records_enabled_p ());
> }
>
> /* Return the optinfo currently being accumulated, creating one if
> @@ -1154,11 +1188,23 @@ void
> dump_context::end_any_optinfo ()
> {
> if (m_pending)
> - m_pending->emit ();
> + emit_optinfo (m_pending);
> delete m_pending;
> m_pending = NULL;
> }
>
> +/* Emit the optinfo to all of the "non-immediate" destinations
> + (emission to "immediate" destinations is done by
> + dump_context::emit_item). */
> +
> +void
> +dump_context::emit_optinfo (const optinfo *info)
> +{
> + /* -fsave-optimization-record. */
> + if (m_json_writer)
> + m_json_writer->add_record (info);
> +}
> +
> /* Emit ITEM to all item destinations (those that don't require
> consolidation into optinfo instances). */
>
> @@ -2004,7 +2050,8 @@ temp_dump_context::temp_dump_context (bool
> forcibly_enable_optinfo,
> m_saved (&dump_context ().get ())
> {
> dump_context::s_current = &m_context;
> - m_context.m_forcibly_enable_optinfo = forcibly_enable_optinfo;
> + if (forcibly_enable_optinfo)
> + m_context.set_json_writer (new optrecord_json_writer ());
> /* Conditionally enable the test dump, so that we can verify both the
> dump_enabled_p and the !dump_enabled_p cases in selftests. */
> if (forcibly_enable_dumping)
> @@ -2020,6 +2067,8 @@ temp_dump_context::temp_dump_context (bool
> forcibly_enable_optinfo,
>
> temp_dump_context::~temp_dump_context ()
> {
> + m_context.set_json_writer (NULL);
> +
> dump_context::s_current = m_saved;
>
> dump_context::get ().refresh_dumps_are_enabled ();
> diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc
> index 6d4502c..4fa6708 100644
> --- a/gcc/optinfo-emit-json.cc
> +++ b/gcc/optinfo-emit-json.cc
> @@ -47,38 +47,6 @@ along with GCC; see the file COPYING3. If not see
> #include "dump-context.h"
> #include <zlib.h>
>
> -/* A class for writing out optimization records in JSON format. */
> -
> -class optrecord_json_writer
> -{
> -public:
> - optrecord_json_writer ();
> - ~optrecord_json_writer ();
> - void write () const;
> - void add_record (const optinfo *optinfo);
> - void pop_scope ();
> -
> - void add_record (json::object *obj);
> - json::object *impl_location_to_json (dump_impl_location_t loc);
> - json::object *location_to_json (location_t loc);
> - json::object *profile_count_to_json (profile_count count);
> - json::string *get_id_value_for_pass (opt_pass *pass);
> - json::object *pass_to_json (opt_pass *pass);
> - json::value *inlining_chain_to_json (location_t loc);
> - json::object *optinfo_to_json (const optinfo *optinfo);
> - void add_pass_list (json::array *arr, opt_pass *pass);
> -
> -private:
> - /* The root value for the JSON file.
> - Currently the JSON values are stored in memory, and flushed when the
> - compiler exits. It would probably be better to simply write out
> - the JSON as we go. */
> - json::array *m_root_tuple;
> -
> - /* The currently open scopes, for expressing nested optimization records.
> */
> - auto_vec<json::array *> m_scopes;
> -};
> -
> /* optrecord_json_writer's ctor. Populate the top-level parts of the
> in-memory JSON representation. */
>
> @@ -471,75 +439,6 @@ optrecord_json_writer::add_pass_list (json::array *arr,
> opt_pass *pass)
> while (pass);
> }
>
> -/* File-level interface to rest of compiler (to avoid exposing
> - class optrecord_json_writer outside of this file). */
> -
> -static optrecord_json_writer *the_json_writer;
> -
> -/* Perform startup activity for -fsave-optimization-record. */
> -
> -void
> -optimization_records_start ()
> -{
> - /* Bail if recording not enabled. */
> - if (!flag_save_optimization_record)
> - return;
> -
> - the_json_writer = new optrecord_json_writer ();
> -}
> -
> -/* Perform cleanup activity for -fsave-optimization-record.
> -
> - Currently, the file is written out here in one go, before cleaning
> - up. */
> -
> -void
> -optimization_records_finish ()
> -{
> - /* Bail if recording not enabled. */
> - if (!the_json_writer)
> - return;
> -
> - the_json_writer->write ();
> -
> - delete the_json_writer;
> - the_json_writer = NULL;
> -}
> -
> -/* Did the user request optimization records to be written out? */
> -
> -bool
> -optimization_records_enabled_p ()
> -{
> - return the_json_writer != NULL;
> -}
> -
> -/* If optimization records were requested, then add a record for OPTINFO
> - to the queue of records to be written. */
> -
> -void
> -optimization_records_maybe_record_optinfo (const optinfo *optinfo)
> -{
> - /* Bail if recording not enabled. */
> - if (!the_json_writer)
> - return;
> -
> - the_json_writer->add_record (optinfo);
> -}
> -
> -/* Handling for the end of a dump scope for the
> - optimization records sink. */
> -
> -void
> -optimization_records_maybe_pop_dump_scope ()
> -{
> - /* Bail if recording not enabled. */
> - if (!the_json_writer)
> - return;
> -
> - the_json_writer->pop_scope ();
> -}
> -
> #if CHECKING_P
>
> namespace selftest {
> diff --git a/gcc/optinfo-emit-json.h b/gcc/optinfo-emit-json.h
> index 3628d56..2185c08 100644
> --- a/gcc/optinfo-emit-json.h
> +++ b/gcc/optinfo-emit-json.h
> @@ -21,16 +21,40 @@ along with GCC; see the file COPYING3. If not see
> #ifndef GCC_OPTINFO_EMIT_JSON_H
> #define GCC_OPTINFO_EMIT_JSON_H
>
> -class optinfo;
> -struct opt_pass;
> -
> -extern void optimization_records_start ();
> -extern void optimization_records_finish ();
> +#include "json.h"
>
> -extern bool optimization_records_enabled_p ();
> -
> -extern void optimization_records_maybe_record_optinfo (const optinfo *);
> -extern void optimization_records_maybe_pop_dump_scope ();
> +class optinfo;
>
> +/* A class for writing out optimization records in JSON format. */
> +
> +class optrecord_json_writer
> +{
> +public:
> + optrecord_json_writer ();
> + ~optrecord_json_writer ();
> + void write () const;
> + void add_record (const optinfo *optinfo);
> + void pop_scope ();
> +
> + void add_record (json::object *obj);
> + json::object *impl_location_to_json (dump_impl_location_t loc);
> + json::object *location_to_json (location_t loc);
> + json::object *profile_count_to_json (profile_count count);
> + json::string *get_id_value_for_pass (opt_pass *pass);
> + json::object *pass_to_json (opt_pass *pass);
> + json::value *inlining_chain_to_json (location_t loc);
> + json::object *optinfo_to_json (const optinfo *optinfo);
> + void add_pass_list (json::array *arr, opt_pass *pass);
> +
> + private:
> + /* The root value for the JSON file.
> + Currently the JSON values are stored in memory, and flushed when the
> + compiler exits. It would probably be better to simply write out
> + the JSON as we go. */
> + json::array *m_root_tuple;
> +
> + /* The currently open scopes, for expressing nested optimization records.
> */
> + auto_vec<json::array *> m_scopes;
> +};
>
> #endif /* #ifndef GCC_OPTINFO_EMIT_JSON_H */
> diff --git a/gcc/optinfo.cc b/gcc/optinfo.cc
> index de781a5..f8e08de 100644
> --- a/gcc/optinfo.cc
> +++ b/gcc/optinfo.cc
> @@ -125,18 +125,7 @@ optinfo::emit_for_opt_problem () const
> dump_context::get ().emit_item (item, dump_kind);
>
> /* Re-emit to "non-immediate" destinations. */
> - emit ();
> -}
> -
> -/* Emit the optinfo to all of the "non-immediate" destinations
> - (emission to "immediate" destinations is done by
> - dump_context::emit_item). */
> -
> -void
> -optinfo::emit () const
> -{
> - /* -fsave-optimization-record. */
> - optimization_records_maybe_record_optinfo (this);
> + dump_context::get ().emit_optinfo (this);
> }
>
> /* Update the optinfo's kind based on DUMP_KIND. */
> @@ -152,21 +141,11 @@ optinfo::handle_dump_file_kind (dump_flags_t dump_kind)
> m_kind = OPTINFO_KIND_NOTE;
> }
>
> -/* Should optinfo instances be created?
> - All creation of optinfos should be guarded by this predicate.
> - Return true if any optinfo destinations are active. */
> -
> -bool optinfo_enabled_p ()
> -{
> - return (dump_context::get ().forcibly_enable_optinfo_p ()
> - || optimization_records_enabled_p ());
> -}
> -
> /* Return true if any of the active optinfo destinations make use
> of inlining information.
> (if true, then the information is preserved). */
>
> bool optinfo_wants_inlining_info_p ()
> {
> - return optimization_records_enabled_p ();
> + return dump_context::get ().optimization_records_enabled_p ();
> }
> diff --git a/gcc/optinfo.h b/gcc/optinfo.h
> index 99bd22c..4a678ff 100644
> --- a/gcc/optinfo.h
> +++ b/gcc/optinfo.h
> @@ -68,12 +68,6 @@ along with GCC; see the file COPYING3. If not see
> struct opt_pass;
> class optinfo_item;
>
> -/* Should optinfo instances be created?
> - All creation of optinfos should be guarded by this predicate.
> - Return true if any optinfo destinations are active. */
> -
> -extern bool optinfo_enabled_p ();
> -
> /* Return true if any of the active optinfo destinations make use
> of inlining information.
> (if true, then the information is preserved). */
> @@ -130,8 +124,6 @@ class optinfo
> void emit_for_opt_problem () const;
>
> private:
> - void emit () const;
> -
> /* Pre-canned ways of manipulating the optinfo, for use by friend class
> dump_context. */
> void handle_dump_file_kind (dump_flags_t);
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index 3fd4ec4..ab20cd9 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -83,6 +83,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-pass.h"
> #include "dumpfile.h"
> #include "ipa-fnsummary.h"
> +#include "dump-context.h"
> #include "optinfo-emit-json.h"
>
> #if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
> @@ -488,7 +489,7 @@ compile_file (void)
> if (lang_hooks.decls.post_compilation_parsing_cleanups)
> lang_hooks.decls.post_compilation_parsing_cleanups ();
>
> - optimization_records_finish ();
> + dump_context::get ().finish_any_json_writer ();
>
> if (seen_error ())
> return;
> @@ -2131,7 +2132,10 @@ do_compile ()
>
> timevar_start (TV_PHASE_SETUP);
>
> - optimization_records_start ();
> + if (flag_save_optimization_record)
> + {
> + dump_context::get ().set_json_writer (new optrecord_json_writer ());
> + }
>
> /* This must be run always, because it is needed to compute the FP
> predefined macros, such as __LDBL_MAX__, for targets using non
> --
> 1.8.5.3
>
