Re: [PATCH 2/5] dumpfile.c: eliminate special-casing of dump_file/alt_dump_file

[Richard Biener <richard.guenther@gmail.com>]

On Fri, Jul 27, 2018 at 11:48 PM David Malcolm <dmalcolm@redhat.com> wrote:
>
> With the addition of optinfo, the various dump_* calls had three parts:
> - optionally print to dump_file
> - optionally print to alt_dump_file
> - optionally make an optinfo_item and add it to the pending optinfo,
>   creating it for dump_*_loc calls.
>
> However, this split makes it difficult to implement the formatted dumps
> later in patch kit, so as enabling work towards that, this patch removes
> the above split, so that all dumping within the dump_* API goes through
> optinfo_item.
>
> In order to ensure that the dumps to dump_file and alt_dump_file are
> processed immediately (rather than being buffered within the pending
> optinfo for consolidation), this patch introduces the idea of "immediate"
> optinfo_item destinations vs "non-immediate" destinations.
>
> The patch also adds selftest coverage of what's printed, and of scopes.
>
> This adds two allocations per dump_* call when dumping is enabled.
> I'm assuming that this isn't a problem, as dump_enabled_p is normally
> false.  There are ways of optimizing it if it is an issue (by making
> optinfo_item instances become temporaries that borrow the underlying
> buffer), but they require nontrivial changes, so I'd prefer to leave
> that for another patch kit, if it becomes necessary.

Yeah, I guess that's OK given we can consolidate quite some calls after
your patch anyways.  Using alloca + placement new would be possible
as well I guess?

OK.

Richard.

> gcc/ChangeLog:
>         * dump-context.h: Include "pretty-print.h".
>         (dump_context::refresh_dumps_are_enabled): New decl.
>         (dump_context::emit_item): New decl.
>         (class dump_context): Add fields "m_test_pp" and
>         "m_test_pp_flags".
>         (temp_dump_context::temp_dump_context): Add param "test_pp_flags".
>         (temp_dump_context::get_dumped_text): New decl.
>         (class temp_dump_context): Add field "m_pp".
>         * dumpfile.c (refresh_dumps_are_enabled): Convert to...
>         (dump_context::refresh_dumps_are_enabled): ...and add a test for
>         m_test_pp.
>         (set_dump_file): Update for above change.
>         (set_alt_dump_file): Likewise.
>         (dump_loc): New overload, taking a pretty_printer *.
>         (dump_context::dump_loc): Call end_any_optinfo.  Dump the location
>         to any test pretty-printer.
>         (make_item_for_dump_gimple_stmt): New function, adapted from
>         optinfo::add_gimple_stmt.
>         (dump_context::dump_gimple_stmt): Call it, and use the result,
>         eliminating the direct usage of dump_file and alt_dump_file in
>         favor of indirectly using them via emit_item.
>         (make_item_for_dump_gimple_expr): New function, adapted from
>         optinfo::add_gimple_expr.
>         (dump_context::dump_gimple_expr): Call it, and use the result,
>         eliminating the direct usage of dump_file and alt_dump_file in
>         favor of indirectly using them via emit_item.
>         (make_item_for_dump_generic_expr): New function, adapted from
>         optinfo::add_tree.
>         (dump_context::dump_generic_expr): Call it, and use the result,
>         eliminating the direct usage of dump_file and alt_dump_file in
>         favor of indirectly using them via emit_item.
>         (make_item_for_dump_printf_va): New function, adapted from
>         optinfo::add_printf_va.
>         (make_item_for_dump_printf): New function.
>         (dump_context::dump_printf_va): Call make_item_for_dump_printf_va,
>         and use the result, eliminating the direct usage of dump_file and
>         alt_dump_file in favor of indirectly using them via emit_item.
>         (make_item_for_dump_dec): New function.
>         (dump_context::dump_dec): Call it, and use the result,
>         eliminating the direct usage of dump_file and alt_dump_file in
>         favor of indirectly using them via emit_item.
>         (make_item_for_dump_symtab_node): New function, adapted from
>         optinfo::add_symtab_node.
>         (dump_context::dump_symtab_node): Call it, and use the result,
>         eliminating the direct usage of dump_file and alt_dump_file in
>         favor of indirectly using them via emit_item.
>         (dump_context::begin_scope): Reimplement, avoiding direct usage
>         of dump_file and alt_dump_file in favor of indirectly using them
>         via emit_item.
>         (dump_context::emit_item): New member function.
>         (temp_dump_context::temp_dump_context): Add param "test_pp_flags".
>         Set up test pretty-printer on the underlying context.  Call
>         refresh_dumps_are_enabled.
>         (temp_dump_context::~temp_dump_context): Call
>         refresh_dumps_are_enabled.
>         (temp_dump_context::get_dumped_text): New member function.
>         (selftest::verify_dumped_text): New function.
>         (ASSERT_DUMPED_TEXT_EQ): New macro.
>         (selftest::test_capture_of_dump_calls): Run all tests twice, with
>         and then without optinfo enabled.  Add uses of
>         ASSERT_DUMPED_TEXT_EQ to all tests.  Add test of nested scopes.
>         * dumpfile.h: Update comment for the dump_* API.
>         * optinfo-emit-json.cc
>         (selftest::test_building_json_from_dump_calls): Update for new
>         param for temp_dump_context ctor.
>         * optinfo.cc (optinfo_item::optinfo_item): Remove "owned" param
>         and "m_owned" field.
>         (optinfo_item::~optinfo_item): Likewise.
>         (optinfo::add_item): New member function.
>         (optinfo::emit): Update comment.
>         (optinfo::add_string): Delete.
>         (optinfo::add_printf): Delete.
>         (optinfo::add_printf_va): Delete.
>         (optinfo::add_gimple_stmt): Delete.
>         (optinfo::add_gimple_expr): Delete.
>         (optinfo::add_tree): Delete.
>         (optinfo::add_symtab_node): Delete.
>         (optinfo::add_dec): Delete.
>         * optinfo.h (class dump_context): New forward decl.
>         (optinfo::add_item): New decl.
>         (optinfo::add_string): Delete.
>         (optinfo::add_printf): Delete.
>         (optinfo::add_printf_va): Delete.
>         (optinfo::add_gimple_stmt): Delete.
>         (optinfo::add_gimple_expr): Delete.
>         (optinfo::add_tree): Delete.
>         (optinfo::add_symtab_node): Delete.
>         (optinfo::add_dec): Delete.
>         (optinfo::add_poly_int): Delete.
>         (optinfo_item::optinfo_item): Remove "owned" param.
>         (class optinfo_item): Remove field "m_owned".
> ---
>  gcc/dump-context.h       |  16 +-
>  gcc/dumpfile.c           | 620 ++++++++++++++++++++++++++++++++++-------------
>  gcc/dumpfile.h           |  34 ++-
>  gcc/optinfo-emit-json.cc |   2 +-
>  gcc/optinfo.cc           | 135 ++---------
>  gcc/optinfo.h            |  38 +--
>  6 files changed, 505 insertions(+), 340 deletions(-)
>
> diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> index f6df0b4..f40ea14 100644
> --- a/gcc/dump-context.h
> +++ b/gcc/dump-context.h
> @@ -22,6 +22,8 @@ along with GCC; see the file COPYING3.  If not see
>  #ifndef GCC_DUMP_CONTEXT_H
>  #define GCC_DUMP_CONTEXT_H 1
>
> +#include "pretty-print.h"
> +
>  /* A class for handling the various dump_* calls.
>
>     In particular, this class has responsibility for consolidating
> @@ -39,6 +41,8 @@ class dump_context
>
>    ~dump_context ();
>
> +  void refresh_dumps_are_enabled ();
> +
>    void dump_loc (dump_flags_t dump_kind, const dump_location_t &loc);
>
>    void dump_gimple_stmt (dump_flags_t dump_kind, dump_flags_t extra_dump_flags,
> @@ -93,6 +97,8 @@ class dump_context
>
>    void end_any_optinfo ();
>
> +  void emit_item (optinfo_item *item, dump_flags_t dump_kind);
> +
>   private:
>    optinfo &ensure_pending_optinfo ();
>    optinfo &begin_next_optinfo (const dump_location_t &loc);
> @@ -108,6 +114,11 @@ class dump_context
>       if any.  */
>    optinfo *m_pending;
>
> +  /* For use in selftests: if non-NULL, then items are to be printed
> +     to this, using the given flags.  */
> +  pretty_printer *m_test_pp;
> +  dump_flags_t m_test_pp_flags;
> +
>    /* The currently active dump_context, for use by the dump_* API calls.  */
>    static dump_context *s_current;
>
> @@ -123,13 +134,16 @@ class dump_context
>  class temp_dump_context
>  {
>   public:
> -  temp_dump_context (bool forcibly_enable_optinfo);
> +  temp_dump_context (bool forcibly_enable_optinfo,
> +                    dump_flags_t test_pp_flags);
>    ~temp_dump_context ();
>
>    /* Support for selftests.  */
>    optinfo *get_pending_optinfo () const { return m_context.m_pending; }
> +  const char *get_dumped_text ();
>
>   private:
> +  pretty_printer m_pp;
>    dump_context m_context;
>    dump_context *m_saved;
>    bool m_saved_flag_remarks;
> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> index 3c8bc38..10e9cab 100644
> --- a/gcc/dumpfile.c
> +++ b/gcc/dumpfile.c
> @@ -63,15 +63,6 @@ dump_flags_t dump_flags;
>  bool dumps_are_enabled = false;
>
>
> -/* Update the "dumps_are_enabled" global; to be called whenever dump_file
> -   or alt_dump_file change.  */
> -
> -static void
> -refresh_dumps_are_enabled ()
> -{
> -  dumps_are_enabled = (dump_file || alt_dump_file || optinfo_enabled_p ());
> -}
> -
>  /* Set global "dump_file" to NEW_DUMP_FILE, refreshing the "dumps_are_enabled"
>     global.  */
>
> @@ -80,7 +71,7 @@ set_dump_file (FILE *new_dump_file)
>  {
>    dumpfile_ensure_any_optinfo_are_flushed ();
>    dump_file = new_dump_file;
> -  refresh_dumps_are_enabled ();
> +  dump_context::get ().refresh_dumps_are_enabled ();
>  }
>
>  /* Set "alt_dump_file" to NEW_ALT_DUMP_FILE, refreshing the "dumps_are_enabled"
> @@ -91,7 +82,7 @@ set_alt_dump_file (FILE *new_alt_dump_file)
>  {
>    dumpfile_ensure_any_optinfo_are_flushed ();
>    alt_dump_file = new_alt_dump_file;
> -  refresh_dumps_are_enabled ();
> +  dump_context::get ().refresh_dumps_are_enabled ();
>  }
>
>  #define DUMP_FILE_INFO(suffix, swtch, dkind, num) \
> @@ -465,6 +456,27 @@ dump_loc (dump_flags_t dump_kind, FILE *dfile, source_location loc)
>      }
>  }
>
> +/* Print source location to PP if enabled.  */
> +
> +static void
> +dump_loc (dump_flags_t dump_kind, pretty_printer *pp, source_location loc)
> +{
> +  if (dump_kind)
> +    {
> +      if (LOCATION_LOCUS (loc) > BUILTINS_LOCATION)
> +       pp_printf (pp, "%s:%d:%d: note: ", LOCATION_FILE (loc),
> +                  LOCATION_LINE (loc), LOCATION_COLUMN (loc));
> +      else if (current_function_decl)
> +       pp_printf (pp, "%s:%d:%d: note: ",
> +                  DECL_SOURCE_FILE (current_function_decl),
> +                  DECL_SOURCE_LINE (current_function_decl),
> +                  DECL_SOURCE_COLUMN (current_function_decl));
> +      /* Indentation based on scope depth.  */
> +      for (unsigned i = 0; i < get_dump_scope_depth (); i++)
> +       pp_character (pp, ' ');
> +    }
> +}
> +
>  /* Implementation of dump_context member functions.  */
>
>  /* dump_context's dtor.  */
> @@ -474,12 +486,24 @@ dump_context::~dump_context ()
>    delete m_pending;
>  }
>
> +/* Update the "dumps_are_enabled" global; to be called whenever dump_file
> +   or alt_dump_file change, or when changing dump_context in selftests.  */
> +
> +void
> +dump_context::refresh_dumps_are_enabled ()
> +{
> +  dumps_are_enabled = (dump_file || alt_dump_file || optinfo_enabled_p ()
> +                      || m_test_pp);
> +}
> +
>  /* Print LOC to the appropriate dump destinations, given DUMP_KIND.
>     If optinfos are enabled, begin a new optinfo.  */
>
>  void
>  dump_context::dump_loc (dump_flags_t dump_kind, const dump_location_t &loc)
>  {
> +  end_any_optinfo ();
> +
>    location_t srcloc = loc.get_location_t ();
>
>    if (dump_file && (dump_kind & pflags))
> @@ -488,6 +512,10 @@ dump_context::dump_loc (dump_flags_t dump_kind, const dump_location_t &loc)
>    if (alt_dump_file && (dump_kind & alt_flags))
>      ::dump_loc (dump_kind, alt_dump_file, srcloc);
>
> +  /* Support for temp_dump_context in selftests.  */
> +  if (m_test_pp && (dump_kind & m_test_pp_flags))
> +    ::dump_loc (dump_kind, m_test_pp, srcloc);
> +
>    if (optinfo_enabled_p ())
>      {
>        optinfo &info = begin_next_optinfo (loc);
> @@ -495,6 +523,22 @@ dump_context::dump_loc (dump_flags_t dump_kind, const dump_location_t &loc)
>      }
>  }
>
> +/* Make an item for the given dump call, equivalent to print_gimple_stmt.  */
> +
> +static optinfo_item *
> +make_item_for_dump_gimple_stmt (gimple *stmt, int spc, dump_flags_t dump_flags)
> +{
> +  pretty_printer pp;
> +  pp_needs_newline (&pp) = true;
> +  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> +  pp_newline (&pp);
> +
> +  optinfo_item *item
> +    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location (stmt),
> +                       xstrdup (pp_formatted_text (&pp)));
> +  return item;
> +}
> +
>  /* Dump gimple statement GS with SPC indentation spaces and
>     EXTRA_DUMP_FLAGS on the dump streams if DUMP_KIND is enabled.  */
>
> @@ -503,18 +547,18 @@ dump_context::dump_gimple_stmt (dump_flags_t dump_kind,
>                                 dump_flags_t extra_dump_flags,
>                                 gimple *gs, int spc)
>  {
> -  if (dump_file && (dump_kind & pflags))
> -    print_gimple_stmt (dump_file, gs, spc, dump_flags | extra_dump_flags);
> -
> -  if (alt_dump_file && (dump_kind & alt_flags))
> -    print_gimple_stmt (alt_dump_file, gs, spc, dump_flags | extra_dump_flags);
> +  optinfo_item *item
> +    = make_item_for_dump_gimple_stmt (gs, spc, dump_flags | extra_dump_flags);
> +  emit_item (item, dump_kind);
>
>    if (optinfo_enabled_p ())
>      {
>        optinfo &info = ensure_pending_optinfo ();
>        info.handle_dump_file_kind (dump_kind);
> -      info.add_gimple_stmt (gs, spc, dump_flags | extra_dump_flags);
> +      info.add_item (item);
>      }
> +  else
> +    delete item;
>  }
>
>  /* Similar to dump_gimple_stmt, except additionally print source location.  */
> @@ -529,6 +573,22 @@ dump_context::dump_gimple_stmt_loc (dump_flags_t dump_kind,
>    dump_gimple_stmt (dump_kind, extra_dump_flags, gs, spc);
>  }
>
> +/* Make an item for the given dump call, equivalent to print_gimple_expr.  */
> +
> +static optinfo_item *
> +make_item_for_dump_gimple_expr (gimple *stmt, int spc, dump_flags_t dump_flags)
> +{
> +  dump_flags |= TDF_RHS_ONLY;
> +  pretty_printer pp;
> +  pp_needs_newline (&pp) = true;
> +  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> +
> +  optinfo_item *item
> +    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location (stmt),
> +                       xstrdup (pp_formatted_text (&pp)));
> +  return item;
> +}
> +
>  /* Dump gimple statement GS with SPC indentation spaces and
>     EXTRA_DUMP_FLAGS on the dump streams if DUMP_KIND is enabled.
>     Do not terminate with a newline or semicolon.  */
> @@ -538,18 +598,18 @@ dump_context::dump_gimple_expr (dump_flags_t dump_kind,
>                                 dump_flags_t extra_dump_flags,
>                                 gimple *gs, int spc)
>  {
> -  if (dump_file && (dump_kind & pflags))
> -    print_gimple_expr (dump_file, gs, spc, dump_flags | extra_dump_flags);
> -
> -  if (alt_dump_file && (dump_kind & alt_flags))
> -    print_gimple_expr (alt_dump_file, gs, spc, dump_flags | extra_dump_flags);
> +  optinfo_item *item
> +    = make_item_for_dump_gimple_expr (gs, spc, dump_flags | extra_dump_flags);
> +  emit_item (item, dump_kind);
>
>    if (optinfo_enabled_p ())
>      {
>        optinfo &info = ensure_pending_optinfo ();
>        info.handle_dump_file_kind (dump_kind);
> -      info.add_gimple_expr (gs, spc, dump_flags | extra_dump_flags);
> +      info.add_item (item);
>      }
> +  else
> +    delete item;
>  }
>
>  /* Similar to dump_gimple_expr, except additionally print source location.  */
> @@ -565,6 +625,25 @@ dump_context::dump_gimple_expr_loc (dump_flags_t dump_kind,
>    dump_gimple_expr (dump_kind, extra_dump_flags, gs, spc);
>  }
>
> +/* Make an item for the given dump call, equivalent to print_generic_expr.  */
> +
> +static optinfo_item *
> +make_item_for_dump_generic_expr (tree node, dump_flags_t dump_flags)
> +{
> +  pretty_printer pp;
> +  pp_needs_newline (&pp) = true;
> +  pp_translate_identifiers (&pp) = false;
> +  dump_generic_node (&pp, node, 0, dump_flags, false);
> +
> +  location_t loc = UNKNOWN_LOCATION;
> +  if (EXPR_HAS_LOCATION (node))
> +    loc = EXPR_LOCATION (node);
> +
> +  optinfo_item *item
> +    = new optinfo_item (OPTINFO_ITEM_KIND_TREE, loc,
> +                       xstrdup (pp_formatted_text (&pp)));
> +  return item;
> +}
>
>  /* Dump expression tree T using EXTRA_DUMP_FLAGS on dump streams if
>     DUMP_KIND is enabled.  */
> @@ -574,18 +653,18 @@ dump_context::dump_generic_expr (dump_flags_t dump_kind,
>                                  dump_flags_t extra_dump_flags,
>                                  tree t)
>  {
> -  if (dump_file && (dump_kind & pflags))
> -      print_generic_expr (dump_file, t, dump_flags | extra_dump_flags);
> -
> -  if (alt_dump_file && (dump_kind & alt_flags))
> -      print_generic_expr (alt_dump_file, t, dump_flags | extra_dump_flags);
> +  optinfo_item *item
> +    = make_item_for_dump_generic_expr (t, dump_flags | extra_dump_flags);
> +  emit_item (item, dump_kind);
>
>    if (optinfo_enabled_p ())
>      {
>        optinfo &info = ensure_pending_optinfo ();
>        info.handle_dump_file_kind (dump_kind);
> -      info.add_tree (t, dump_flags | extra_dump_flags);
> +      info.add_item (item);
>      }
> +  else
> +    delete item;
>  }
>
>
> @@ -602,36 +681,56 @@ dump_context::dump_generic_expr_loc (dump_flags_t dump_kind,
>    dump_generic_expr (dump_kind, extra_dump_flags, t);
>  }
>
> +/* Make an item for the given dump call.  */
> +
> +static optinfo_item *
> +make_item_for_dump_printf_va (const char *format, va_list ap)
> +  ATTRIBUTE_PRINTF (1, 0);
> +
> +static optinfo_item *
> +make_item_for_dump_printf_va (const char *format, va_list ap)
> +{
> +  char *formatted_text = xvasprintf (format, ap);
> +  optinfo_item *item
> +    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> +                       formatted_text);
> +  return item;
> +}
> +
> +/* Make an item for the given dump call.  */
> +
> +static optinfo_item *
> +make_item_for_dump_printf (const char *format, ...)
> +  ATTRIBUTE_PRINTF (1, 2);
> +
> +static optinfo_item *
> +make_item_for_dump_printf (const char *format, ...)
> +{
> +  va_list ap;
> +  va_start (ap, format);
> +  optinfo_item *item
> +    = make_item_for_dump_printf_va (format, ap);
> +  va_end (ap);
> +  return item;
> +}
> +
>  /* Output a formatted message using FORMAT on appropriate dump streams.  */
>
>  void
>  dump_context::dump_printf_va (dump_flags_t dump_kind, const char *format,
>                               va_list ap)
>  {
> -  if (dump_file && (dump_kind & pflags))
> -    {
> -      va_list aq;
> -      va_copy (aq, ap);
> -      vfprintf (dump_file, format, aq);
> -      va_end (aq);
> -    }
> -
> -  if (alt_dump_file && (dump_kind & alt_flags))
> -    {
> -      va_list aq;
> -      va_copy (aq, ap);
> -      vfprintf (alt_dump_file, format, aq);
> -      va_end (aq);
> -    }
> +  optinfo_item *item = make_item_for_dump_printf_va (format, ap);
> +  emit_item (item, dump_kind);
>
>    if (optinfo_enabled_p ())
>      {
>        optinfo &info = ensure_pending_optinfo ();
> -      va_list aq;
> -      va_copy (aq, ap);
> -      info.add_printf_va (format, aq);
> -      va_end (aq);
> +      info.handle_dump_file_kind (dump_kind);
> +      info.add_item (item);
>      }
> +  else
> +    delete item;
>  }
>
>  /* Similar to dump_printf, except source location is also printed, and
> @@ -646,26 +745,64 @@ dump_context::dump_printf_loc_va (dump_flags_t dump_kind,
>    dump_printf_va (dump_kind, format, ap);
>  }
>
> -/* Output VALUE in decimal to appropriate dump streams.  */
> +/* Make an item for the given dump call, equivalent to print_dec.  */
>
>  template<unsigned int N, typename C>
> -void
> -dump_context::dump_dec (dump_flags_t dump_kind, const poly_int<N, C> &value)
> +static optinfo_item *
> +make_item_for_dump_dec (const poly_int<N, C> &value)
>  {
>    STATIC_ASSERT (poly_coeff_traits<C>::signedness >= 0);
>    signop sgn = poly_coeff_traits<C>::signedness ? SIGNED : UNSIGNED;
> -  if (dump_file && (dump_kind & pflags))
> -    print_dec (value, dump_file, sgn);
>
> -  if (alt_dump_file && (dump_kind & alt_flags))
> -    print_dec (value, alt_dump_file, sgn);
> +  pretty_printer pp;
> +
> +  if (value.is_constant ())
> +    pp_wide_int (&pp, value.coeffs[0], sgn);
> +  else
> +    {
> +      pp_character (&pp, '[');
> +      for (unsigned int i = 0; i < N; ++i)
> +       {
> +         pp_wide_int (&pp, value.coeffs[i], sgn);
> +         pp_character (&pp, i == N - 1 ? ']' : ',');
> +       }
> +    }
> +
> +  optinfo_item *item
> +    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> +                       xstrdup (pp_formatted_text (&pp)));
> +  return item;
> +}
> +
> +/* Output VALUE in decimal to appropriate dump streams.  */
> +
> +template<unsigned int N, typename C>
> +void
> +dump_context::dump_dec (dump_flags_t dump_kind, const poly_int<N, C> &value)
> +{
> +  optinfo_item *item = make_item_for_dump_dec (value);
> +  emit_item (item, dump_kind);
>
>    if (optinfo_enabled_p ())
>      {
>        optinfo &info = ensure_pending_optinfo ();
>        info.handle_dump_file_kind (dump_kind);
> -      info.add_poly_int<N,C> (value);
> +      info.add_item (item);
>      }
> +  else
> +    delete item;
> +}
> +
> +/* Make an item for the given dump call.  */
> +
> +static optinfo_item *
> +make_item_for_dump_symtab_node (symtab_node *node)
> +{
> +  location_t loc = DECL_SOURCE_LOCATION (node->decl);
> +  optinfo_item *item
> +    = new optinfo_item (OPTINFO_ITEM_KIND_SYMTAB_NODE, loc,
> +                       xstrdup (node->dump_name ()));
> +  return item;
>  }
>
>  /* Output the name of NODE on appropriate dump streams.  */
> @@ -673,18 +810,17 @@ dump_context::dump_dec (dump_flags_t dump_kind, const poly_int<N, C> &value)
>  void
>  dump_context::dump_symtab_node (dump_flags_t dump_kind, symtab_node *node)
>  {
> -  if (dump_file && (dump_kind & pflags))
> -    fprintf (dump_file, "%s", node->dump_name ());
> -
> -  if (alt_dump_file && (dump_kind & alt_flags))
> -    fprintf (alt_dump_file, "%s", node->dump_name ());
> +  optinfo_item *item = make_item_for_dump_symtab_node (node);
> +  emit_item (item, dump_kind);
>
>    if (optinfo_enabled_p ())
>      {
>        optinfo &info = ensure_pending_optinfo ();
>        info.handle_dump_file_kind (dump_kind);
> -      info.add_symtab_node (node);
> +      info.add_item (item);
>      }
> +  else
> +    delete item;
>  }
>
>  /* Get the current dump scope-nesting depth.
> @@ -705,28 +841,28 @@ dump_context::get_scope_depth () const
>  void
>  dump_context::begin_scope (const char *name, const dump_location_t &loc)
>  {
> -  /* Specialcase, to avoid going through dump_printf_loc,
> -     so that we can create a optinfo of kind OPTINFO_KIND_SCOPE.  */
> -
>    if (dump_file)
> -    {
> -      ::dump_loc (MSG_NOTE, dump_file, loc.get_location_t ());
> -      fprintf (dump_file, "=== %s ===\n", name);
> -    }
> +    ::dump_loc (MSG_NOTE, dump_file, loc.get_location_t ());
>
>    if (alt_dump_file)
> -    {
> -      ::dump_loc (MSG_NOTE, alt_dump_file, loc.get_location_t ());
> -      fprintf (alt_dump_file, "=== %s ===\n", name);
> -    }
> +    ::dump_loc (MSG_NOTE, alt_dump_file, loc.get_location_t ());
> +
> +  /* Support for temp_dump_context in selftests.  */
> +  if (m_test_pp)
> +    ::dump_loc (MSG_NOTE, m_test_pp, loc.get_location_t ());
> +
> +  optinfo_item *item = make_item_for_dump_printf ("=== %s ===\n", name);
> +  emit_item (item, MSG_NOTE);
>
>    if (optinfo_enabled_p ())
>      {
> +      optinfo &info = begin_next_optinfo (loc);
> +      info.m_kind = OPTINFO_KIND_SCOPE;
> +      info.add_item (item);
>        end_any_optinfo ();
> -      optinfo info (loc, OPTINFO_KIND_SCOPE, current_pass);
> -      info.add_printf ("=== %s ===", name);
> -      info.emit ();
>      }
> +  else
> +    delete item;
>
>    m_scope_depth++;
>  }
> @@ -776,6 +912,23 @@ dump_context::end_any_optinfo ()
>    m_pending = NULL;
>  }
>
> +/* Emit ITEM to all item destinations (those that don't require
> +   consolidation into optinfo instances).  */
> +
> +void
> +dump_context::emit_item (optinfo_item *item, dump_flags_t dump_kind)
> +{
> +  if (dump_file && (dump_kind & pflags))
> +    fprintf (dump_file, "%s", item->get_text ());
> +
> +  if (alt_dump_file && (dump_kind & alt_flags))
> +    fprintf (alt_dump_file, "%s", item->get_text ());
> +
> +  /* Support for temp_dump_context in selftests.  */
> +  if (m_test_pp && (dump_kind & m_test_pp_flags))
> +    pp_string (m_test_pp, item->get_text ());
> +}
> +
>  /* The current singleton dump_context, and its default.  */
>
>  dump_context *dump_context::s_current = &dump_context::s_default;
> @@ -1510,12 +1663,18 @@ enable_rtl_dump_file (void)
>  /* temp_dump_context's ctor.  Temporarily override the dump_context
>     (to forcibly enable optinfo-generation).  */
>
> -temp_dump_context::temp_dump_context (bool forcibly_enable_optinfo)
> +temp_dump_context::temp_dump_context (bool forcibly_enable_optinfo,
> +                                     dump_flags_t test_pp_flags)
> +
>  : m_context (),
>    m_saved (&dump_context ().get ())
>  {
>    dump_context::s_current = &m_context;
>    m_context.m_forcibly_enable_optinfo = forcibly_enable_optinfo;
> +  m_context.m_test_pp = &m_pp;
> +  m_context.m_test_pp_flags = test_pp_flags;
> +
> +  dump_context::get ().refresh_dumps_are_enabled ();
>  }
>
>  /* temp_dump_context's dtor.  Restore the saved dump_context.  */
> @@ -1523,6 +1682,16 @@ temp_dump_context::temp_dump_context (bool forcibly_enable_optinfo)
>  temp_dump_context::~temp_dump_context ()
>  {
>    dump_context::s_current = m_saved;
> +
> +  dump_context::get ().refresh_dumps_are_enabled ();
> +}
> +
> +/* 0-terminate the text dumped so far, and return it.  */
> +
> +const char *
> +temp_dump_context::get_dumped_text ()
> +{
> +  return pp_formatted_text (&m_pp);
>  }
>
>  namespace selftest {
> @@ -1561,6 +1730,29 @@ test_impl_location ()
>  #endif
>  }
>
> +/* Verify that the text dumped so far in CONTEXT equals
> +   EXPECTED_TEXT, using LOC for the location of any failure.
> +   As a side-effect, the internal buffer is 0-terminated.  */
> +
> +static void
> +verify_dumped_text (const location &loc,
> +                   temp_dump_context *context,
> +                   const char *expected_text)
> +{
> +  gcc_assert (context);
> +  ASSERT_STREQ_AT (loc, context->get_dumped_text (),
> +                  expected_text);
> +}
> +
> +/* Verify that the text dumped so far in CONTEXT equals
> +   EXPECTED_TEXT.
> +   As a side-effect, the internal buffer is 0-terminated.  */
> +
> +#define ASSERT_DUMPED_TEXT_EQ(CONTEXT, EXPECTED_TEXT)                  \
> +  SELFTEST_BEGIN_STMT                                                  \
> +    verify_dumped_text (SELFTEST_LOCATION, &(CONTEXT), (EXPECTED_TEXT)); \
> +  SELFTEST_END_STMT
> +
>  /* Verify that ITEM has the expected values.  */
>
>  static void
> @@ -1611,116 +1803,198 @@ test_capture_of_dump_calls (const line_table_case &case_)
>    linemap_line_start (line_table, 5, 100);
>    linemap_add (line_table, LC_LEAVE, false, NULL, 0);
>    location_t where = linemap_position_for_column (line_table, 10);
> +  if (where > LINE_MAP_MAX_LOCATION_WITH_COLS)
> +    return;
>
>    dump_location_t loc = dump_location_t::from_location_t (where);
>
> -  /* Test of dump_printf.  */
> -  {
> -    temp_dump_context tmp (true);
> -    dump_printf (MSG_NOTE, "int: %i str: %s", 42, "foo");
> -
> -    optinfo *info = tmp.get_pending_optinfo ();
> -    ASSERT_TRUE (info != NULL);
> -    ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> -    ASSERT_EQ (info->num_items (), 1);
> -    ASSERT_IS_TEXT (info->get_item (0), "int: 42 str: foo");
> -  }
> -
> -  /* Tree, via dump_generic_expr.  */
> -  {
> -    temp_dump_context tmp (true);
> -    dump_printf_loc (MSG_NOTE, loc, "test of tree: ");
> -    dump_generic_expr (MSG_NOTE, TDF_SLIM, integer_zero_node);
> -
> -    optinfo *info = tmp.get_pending_optinfo ();
> -    ASSERT_TRUE (info != NULL);
> -    ASSERT_EQ (info->get_location_t (), where);
> -    ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> -    ASSERT_EQ (info->num_items (), 2);
> -    ASSERT_IS_TEXT (info->get_item (0), "test of tree: ");
> -    ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION, "0");
> -  }
> -
> -  /* Tree, via dump_generic_expr_loc.  */
> -  {
> -    temp_dump_context tmp (true);
> -    dump_generic_expr_loc (MSG_NOTE, loc, TDF_SLIM, integer_one_node);
> -
> -    optinfo *info = tmp.get_pending_optinfo ();
> -    ASSERT_TRUE (info != NULL);
> -    ASSERT_EQ (info->get_location_t (), where);
> -    ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> -    ASSERT_EQ (info->num_items (), 1);
> -    ASSERT_IS_TREE (info->get_item (0), UNKNOWN_LOCATION, "1");
> -  }
> -
> -  /* Gimple.  */
> -  {
> -    greturn *stmt = gimple_build_return (NULL);
> -    gimple_set_location (stmt, where);
> -
> -    /* dump_gimple_stmt_loc.  */
> -    {
> -      temp_dump_context tmp (true);
> -      dump_gimple_stmt_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> -
> -      optinfo *info = tmp.get_pending_optinfo ();
> -      ASSERT_TRUE (info != NULL);
> -      ASSERT_EQ (info->num_items (), 1);
> -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> -    }
> -
> -    /* dump_gimple_stmt.  */
> -    {
> -      temp_dump_context tmp (true);
> -      dump_gimple_stmt (MSG_NOTE, TDF_SLIM, stmt, 2);
> +  greturn *stmt = gimple_build_return (NULL);
> +  gimple_set_location (stmt, where);
>
> -      optinfo *info = tmp.get_pending_optinfo ();
> -      ASSERT_TRUE (info != NULL);
> -      ASSERT_EQ (info->num_items (), 1);
> -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> -    }
> -
> -    /* dump_gimple_expr_loc.  */
> +  /* Run all tests twice, with and then without optinfo enabled, to ensure
> +     that immediate destinations vs optinfo-based destinations both
> +     work, independently of each other, with no leaks.  */
> +  for (int i = 0 ; i < 2; i++)
>      {
> -      temp_dump_context tmp (true);
> -      dump_gimple_expr_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> +      bool with_optinfo = (i == 0);
> +
> +      /* Test of dump_printf.  */
> +      {
> +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> +       dump_printf (MSG_NOTE, "int: %i str: %s", 42, "foo");
> +
> +       ASSERT_DUMPED_TEXT_EQ (tmp, "int: 42 str: foo");
> +       if (with_optinfo)
> +         {
> +           optinfo *info = tmp.get_pending_optinfo ();
> +           ASSERT_TRUE (info != NULL);
> +           ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> +           ASSERT_EQ (info->num_items (), 1);
> +           ASSERT_IS_TEXT (info->get_item (0), "int: 42 str: foo");
> +         }
> +      }
> +
> +      /* Tree, via dump_generic_expr.  */
> +      {
> +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> +       dump_printf_loc (MSG_NOTE, loc, "test of tree: ");
> +       dump_generic_expr (MSG_NOTE, TDF_SLIM, integer_zero_node);
> +
> +       ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note: test of tree: 0");
> +       if (with_optinfo)
> +         {
> +           optinfo *info = tmp.get_pending_optinfo ();
> +           ASSERT_TRUE (info != NULL);
> +           ASSERT_EQ (info->get_location_t (), where);
> +           ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> +           ASSERT_EQ (info->num_items (), 2);
> +           ASSERT_IS_TEXT (info->get_item (0), "test of tree: ");
> +           ASSERT_IS_TREE (info->get_item (1), UNKNOWN_LOCATION, "0");
> +         }
> +      }
> +
> +      /* Tree, via dump_generic_expr_loc.  */
> +      {
> +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> +       dump_generic_expr_loc (MSG_NOTE, loc, TDF_SLIM, integer_one_node);
> +
> +       ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note: 1");
> +       if (with_optinfo)
> +         {
> +           optinfo *info = tmp.get_pending_optinfo ();
> +           ASSERT_TRUE (info != NULL);
> +           ASSERT_EQ (info->get_location_t (), where);
> +           ASSERT_EQ (info->get_kind (), OPTINFO_KIND_NOTE);
> +           ASSERT_EQ (info->num_items (), 1);
> +           ASSERT_IS_TREE (info->get_item (0), UNKNOWN_LOCATION, "1");
> +         }
> +      }
> +
> +      /* Gimple.  */
> +      {
> +       /* dump_gimple_stmt_loc.  */
> +       {
> +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> +         dump_gimple_stmt_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> +
> +         ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note: return;\n");
> +         if (with_optinfo)
> +           {
> +             optinfo *info = tmp.get_pending_optinfo ();
> +             ASSERT_TRUE (info != NULL);
> +             ASSERT_EQ (info->num_items (), 1);
> +             ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> +           }
> +       }
>
> -      optinfo *info = tmp.get_pending_optinfo ();
> -      ASSERT_TRUE (info != NULL);
> -      ASSERT_EQ (info->num_items (), 1);
> -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> -    }
> +       /* dump_gimple_stmt.  */
> +       {
> +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> +         dump_gimple_stmt (MSG_NOTE, TDF_SLIM, stmt, 2);
> +
> +         ASSERT_DUMPED_TEXT_EQ (tmp, "return;\n");
> +         if (with_optinfo)
> +           {
> +             optinfo *info = tmp.get_pending_optinfo ();
> +             ASSERT_TRUE (info != NULL);
> +             ASSERT_EQ (info->num_items (), 1);
> +             ASSERT_IS_GIMPLE (info->get_item (0), where, "return;\n");
> +           }
> +       }
>
> -    /* dump_gimple_expr.  */
> -    {
> -      temp_dump_context tmp (true);
> -      dump_gimple_expr (MSG_NOTE, TDF_SLIM, stmt, 2);
> +       /* dump_gimple_expr_loc.  */
> +       {
> +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> +         dump_gimple_expr_loc (MSG_NOTE, loc, TDF_SLIM, stmt, 2);
> +
> +         ASSERT_DUMPED_TEXT_EQ (tmp, "test.txt:5:10: note: return;");
> +         if (with_optinfo)
> +           {
> +             optinfo *info = tmp.get_pending_optinfo ();
> +             ASSERT_TRUE (info != NULL);
> +             ASSERT_EQ (info->num_items (), 1);
> +             ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> +           }
> +       }
>
> -      optinfo *info = tmp.get_pending_optinfo ();
> -      ASSERT_TRUE (info != NULL);
> -      ASSERT_EQ (info->num_items (), 1);
> -      ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> +       /* dump_gimple_expr.  */
> +       {
> +         temp_dump_context tmp (with_optinfo, MSG_ALL);
> +         dump_gimple_expr (MSG_NOTE, TDF_SLIM, stmt, 2);
> +
> +         ASSERT_DUMPED_TEXT_EQ (tmp, "return;");
> +         if (with_optinfo)
> +           {
> +             optinfo *info = tmp.get_pending_optinfo ();
> +             ASSERT_TRUE (info != NULL);
> +             ASSERT_EQ (info->num_items (), 1);
> +             ASSERT_IS_GIMPLE (info->get_item (0), where, "return;");
> +           }
> +       }
> +      }
> +
> +      /* poly_int.  */
> +      {
> +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> +       dump_dec (MSG_NOTE, poly_int64 (42));
> +
> +       ASSERT_DUMPED_TEXT_EQ (tmp, "42");
> +       if (with_optinfo)
> +         {
> +           optinfo *info = tmp.get_pending_optinfo ();
> +           ASSERT_TRUE (info != NULL);
> +           ASSERT_EQ (info->num_items (), 1);
> +           ASSERT_IS_TEXT (info->get_item (0), "42");
> +         }
> +      }
> +
> +      /* scopes.  */
> +      {
> +       temp_dump_context tmp (with_optinfo, MSG_ALL);
> +       dump_printf_loc (MSG_NOTE, stmt, "msg 1\n");
> +       {
> +         AUTO_DUMP_SCOPE ("outer scope", stmt);
> +         dump_printf_loc (MSG_NOTE, stmt, "msg 2\n");
> +         {
> +           AUTO_DUMP_SCOPE ("middle scope", stmt);
> +           dump_printf_loc (MSG_NOTE, stmt, "msg 3\n");
> +           {
> +             AUTO_DUMP_SCOPE ("inner scope", stmt);
> +             dump_printf_loc (MSG_NOTE, stmt, "msg 4\n");
> +           }
> +           dump_printf_loc (MSG_NOTE, stmt, "msg 5\n");
> +         }
> +         dump_printf_loc (MSG_NOTE, stmt, "msg 6\n");
> +       }
> +       dump_printf_loc (MSG_NOTE, stmt, "msg 7\n");
> +
> +       ASSERT_DUMPED_TEXT_EQ (tmp,
> +                              "test.txt:5:10: note: msg 1\n"
> +                              "test.txt:5:10: note: === outer scope ===\n"
> +                              "test.txt:5:10: note:  msg 2\n"
> +                              "test.txt:5:10: note:  === middle scope ===\n"
> +                              "test.txt:5:10: note:   msg 3\n"
> +                              "test.txt:5:10: note:   === inner scope ===\n"
> +                              "test.txt:5:10: note:    msg 4\n"
> +                              "test.txt:5:10: note:   msg 5\n"
> +                              "test.txt:5:10: note:  msg 6\n"
> +                              "test.txt:5:10: note: msg 7\n");
> +       if (with_optinfo)
> +         {
> +           optinfo *info = tmp.get_pending_optinfo ();
> +           ASSERT_TRUE (info != NULL);
> +           ASSERT_EQ (info->num_items (), 1);
> +           ASSERT_IS_TEXT (info->get_item (0), "msg 7\n");
> +         }
> +      }
>      }
> -  }
> -
> -  /* poly_int.  */
> -  {
> -    temp_dump_context tmp (true);
> -    dump_dec (MSG_NOTE, poly_int64 (42));
> -
> -    optinfo *info = tmp.get_pending_optinfo ();
> -    ASSERT_TRUE (info != NULL);
> -    ASSERT_EQ (info->num_items (), 1);
> -    ASSERT_IS_TEXT (info->get_item (0), "42");
> -  }
>
>    /* Verify that MSG_* affects optinfo->get_kind (); we tested MSG_NOTE
>       above.  */
>    {
>      /* MSG_OPTIMIZED_LOCATIONS.  */
>      {
> -      temp_dump_context tmp (true);
> +      temp_dump_context tmp (true, MSG_ALL);
>        dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, loc, "test");
>        ASSERT_EQ (tmp.get_pending_optinfo ()->get_kind (),
>                  OPTINFO_KIND_SUCCESS);
> @@ -1728,7 +2002,7 @@ test_capture_of_dump_calls (const line_table_case &case_)
>
>      /* MSG_MISSED_OPTIMIZATION.  */
>      {
> -      temp_dump_context tmp (true);
> +      temp_dump_context tmp (true, MSG_ALL);
>        dump_printf_loc (MSG_MISSED_OPTIMIZATION, loc, "test");
>        ASSERT_EQ (tmp.get_pending_optinfo ()->get_kind (),
>                  OPTINFO_KIND_FAILURE);
> diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
> index 1dbe3b8..2b174e5 100644
> --- a/gcc/dumpfile.h
> +++ b/gcc/dumpfile.h
> @@ -442,19 +442,27 @@ dump_enabled_p (void)
>  }
>
>  /* The following API calls (which *don't* take a "FILE *")
> -   write the output to zero or more locations:
> -   (a) the active dump_file, if any
> -   (b) the -fopt-info destination, if any
> -   (c) to the "optinfo" destinations, if any:
> -       (c.1) as optimization records
> -
> -   dump_* (MSG_*) --> dumpfile.c --+--> (a) dump_file
> -                                   |
> -                                   +--> (b) alt_dump_file
> -                                   |
> -                                   `--> (c) optinfo
> -                                            `---> optinfo destinations
> -                                                  (c.1) optimization records
> +   write the output to zero or more locations.
> +
> +   Some destinations are written to immediately as dump_* calls
> +   are made; for others, the output is consolidated into an "optinfo"
> +   instance (with its own metadata), and only emitted once the optinfo
> +   is complete.
> +
> +   The destinations are:
> +
> +   (a) the "immediate" destinations:
> +       (a.1) the active dump_file, if any
> +       (a.2) the -fopt-info destination, if any
> +   (b) the "optinfo" destinations, if any:
> +       (b.1) as optimization records
> +
> +   dump_* (MSG_*) --> dumpfile.c --> items --> (a.1) dump_file
> +                                       |   `-> (a.2) alt_dump_file
> +                                       |
> +                                       `--> (b) optinfo
> +                                                `---> optinfo destinations
> +                                                      (b.1) optimization records
>
>     For optinfos, the dump_*_loc mark the beginning of an optinfo
>     instance: all subsequent dump_* calls are consolidated into
> diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc
> index 2199d52..992960e 100644
> --- a/gcc/optinfo-emit-json.cc
> +++ b/gcc/optinfo-emit-json.cc
> @@ -537,7 +537,7 @@ namespace selftest {
>  static void
>  test_building_json_from_dump_calls ()
>  {
> -  temp_dump_context tmp (true);
> +  temp_dump_context tmp (true, MSG_NOTE);
>    dump_location_t loc;
>    dump_printf_loc (MSG_NOTE, loc, "test of tree: ");
>    dump_generic_expr (MSG_NOTE, TDF_SLIM, integer_zero_node);
> diff --git a/gcc/optinfo.cc b/gcc/optinfo.cc
> index 93de9d9..b858c3c 100644
> --- a/gcc/optinfo.cc
> +++ b/gcc/optinfo.cc
> @@ -34,11 +34,11 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cgraph.h"
>  #include "selftest.h"
>
> -/* optinfo_item's ctor.  */
> +/* optinfo_item's ctor.  Takes ownership of TEXT.  */
>
>  optinfo_item::optinfo_item (enum optinfo_item_kind kind, location_t location,
> -                           char *text, bool owned)
> -: m_kind (kind), m_location (location), m_text (text), m_owned (owned)
> +                           char *text)
> +: m_kind (kind), m_location (location), m_text (text)
>  {
>  }
>
> @@ -46,8 +46,7 @@ optinfo_item::optinfo_item (enum optinfo_item_kind kind, location_t location,
>
>  optinfo_item::~optinfo_item ()
>  {
> -  if (m_owned)
> -    free (m_text);
> +  free (m_text);
>  }
>
>  /* Get a string from KIND.  */
> @@ -81,7 +80,17 @@ optinfo::~optinfo ()
>      delete item;
>  }
>
> -/* Emit the optinfo to all of the active destinations.  */
> +/* Add ITEM to this optinfo.  */
> +
> +void
> +optinfo::add_item (optinfo_item *item)
> +{
> +  gcc_assert (item);
> +  m_items.safe_push (item);
> +}
> +
> +/* Emit the optinfo to all of the "non-immediate" destinations
> +   (emission to "immediate" destinations is done by emit_item).  */
>
>  void
>  optinfo::emit ()
> @@ -103,120 +112,6 @@ optinfo::handle_dump_file_kind (dump_flags_t dump_kind)
>      m_kind = OPTINFO_KIND_NOTE;
>  }
>
> -/* Append a string literal to this optinfo.  */
> -
> -void
> -optinfo::add_string (const char *str)
> -{
> -  optinfo_item *item
> -    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> -                       const_cast <char *> (str), false);
> -  m_items.safe_push (item);
> -}
> -
> -/* Append printf-formatted text to this optinfo.  */
> -
> -void
> -optinfo::add_printf (const char *format, ...)
> -{
> -  va_list ap;
> -  va_start (ap, format);
> -  add_printf_va (format, ap);
> -  va_end (ap);
> -}
> -
> -/* Append printf-formatted text to this optinfo.  */
> -
> -void
> -optinfo::add_printf_va (const char *format, va_list ap)
> -{
> -  char *formatted_text = xvasprintf (format, ap);
> -  optinfo_item *item
> -    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> -                       formatted_text, true);
> -  m_items.safe_push (item);
> -}
> -
> -/* Append a gimple statement to this optinfo, equivalent to
> -   print_gimple_stmt.  */
> -
> -void
> -optinfo::add_gimple_stmt (gimple *stmt, int spc, dump_flags_t dump_flags)
> -{
> -  pretty_printer pp;
> -  pp_needs_newline (&pp) = true;
> -  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> -  pp_newline (&pp);
> -
> -  optinfo_item *item
> -    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location (stmt),
> -                       xstrdup (pp_formatted_text (&pp)), true);
> -  m_items.safe_push (item);
> -}
> -
> -/* Append a gimple statement to this optinfo, equivalent to
> -   print_gimple_expr.  */
> -
> -void
> -optinfo::add_gimple_expr (gimple *stmt, int spc, dump_flags_t dump_flags)
> -{
> -  dump_flags |= TDF_RHS_ONLY;
> -  pretty_printer pp;
> -  pp_needs_newline (&pp) = true;
> -  pp_gimple_stmt_1 (&pp, stmt, spc, dump_flags);
> -
> -  optinfo_item *item
> -    = new optinfo_item (OPTINFO_ITEM_KIND_GIMPLE, gimple_location (stmt),
> -                       xstrdup (pp_formatted_text (&pp)), true);
> -  m_items.safe_push (item);
> -}
> -
> -/* Append a tree node to this optinfo, equivalent to print_generic_expr.  */
> -
> -void
> -optinfo::add_tree (tree node, dump_flags_t dump_flags)
> -{
> -  pretty_printer pp;
> -  pp_needs_newline (&pp) = true;
> -  pp_translate_identifiers (&pp) = false;
> -  dump_generic_node (&pp, node, 0, dump_flags, false);
> -
> -  location_t loc = UNKNOWN_LOCATION;
> -  if (EXPR_HAS_LOCATION (node))
> -    loc = EXPR_LOCATION (node);
> -
> -  optinfo_item *item
> -    = new optinfo_item (OPTINFO_ITEM_KIND_TREE, loc,
> -                       xstrdup (pp_formatted_text (&pp)), true);
> -  m_items.safe_push (item);
> -}
> -
> -/* Append a symbol table node to this optinfo.  */
> -
> -void
> -optinfo::add_symtab_node (symtab_node *node)
> -{
> -  location_t loc = DECL_SOURCE_LOCATION (node->decl);
> -  optinfo_item *item
> -    = new optinfo_item (OPTINFO_ITEM_KIND_SYMTAB_NODE, loc,
> -                       xstrdup (node->dump_name ()), true);
> -  m_items.safe_push (item);
> -}
> -
> -/* Append the decimal represenation of a wide_int_ref to this
> -   optinfo.  */
> -
> -void
> -optinfo::add_dec (const wide_int_ref &wi, signop sgn)
> -{
> -  char buf[WIDE_INT_PRINT_BUFFER_SIZE];
> -  print_dec (wi, buf, sgn);
> -  optinfo_item *item
> -    = new optinfo_item (OPTINFO_ITEM_KIND_TEXT, UNKNOWN_LOCATION,
> -                       xstrdup (buf), true);
> -  m_items.safe_push (item);
> -}
> -
>  /* Should optinfo instances be created?
>     All creation of optinfos should be guarded by this predicate.
>     Return true if any optinfo destinations are active.  */
> diff --git a/gcc/optinfo.h b/gcc/optinfo.h
> index c4cf8ad..8ac961c 100644
> --- a/gcc/optinfo.h
> +++ b/gcc/optinfo.h
> @@ -92,6 +92,8 @@ enum optinfo_kind
>
>  extern const char *optinfo_kind_to_string (enum optinfo_kind kind);
>
> +class dump_context;
> +
>  /* A bundle of information describing part of an optimization.  */
>
>  class optinfo
> @@ -120,41 +122,14 @@ class optinfo
>    location_t get_location_t () const { return m_loc.get_location_t (); }
>    profile_count get_count () const { return m_loc.get_count (); }
>
> +  void add_item (optinfo_item *item);
> +
>   private:
>    void emit ();
>
>    /* Pre-canned ways of manipulating the optinfo, for use by friend class
>       dump_context.  */
>    void handle_dump_file_kind (dump_flags_t);
> -  void add_string (const char *str);
> -  void add_printf (const char *format, ...) ATTRIBUTE_PRINTF_2;
> -  void add_printf_va (const char *format, va_list ap) ATTRIBUTE_PRINTF (2, 0);
> -  void add_gimple_stmt (gimple *stmt, int spc, dump_flags_t dump_flags);
> -  void add_gimple_expr (gimple *stmt, int spc, dump_flags_t dump_flags);
> -  void add_tree (tree node, dump_flags_t dump_flags);
> -  void add_symtab_node (symtab_node *node);
> -  void add_dec (const wide_int_ref &wi, signop sgn);
> -
> -  template<unsigned int N, typename C>
> -  void add_poly_int (const poly_int<N, C> &value)
> -  {
> -    /* Compare with dump_dec (MSG_NOTE, ).  */
> -
> -    STATIC_ASSERT (poly_coeff_traits<C>::signedness >= 0);
> -    signop sgn = poly_coeff_traits<C>::signedness ? SIGNED : UNSIGNED;
> -
> -    if (value.is_constant ())
> -      add_dec (value.coeffs[0], sgn);
> -    else
> -      {
> -       add_string ("[");
> -       for (unsigned int i = 0; i < N; ++i)
> -         {
> -           add_dec (value.coeffs[i], sgn);
> -           add_string (i == N - 1 ? "]" : ",");
> -         }
> -      }
> -  }
>
>   private:
>    dump_location_t m_loc;
> @@ -179,7 +154,7 @@ class optinfo_item
>  {
>   public:
>    optinfo_item (enum optinfo_item_kind kind, location_t location,
> -               char *text, bool owned);
> +               char *text);
>    ~optinfo_item ();
>
>    enum optinfo_item_kind get_kind () const { return m_kind; }
> @@ -191,9 +166,8 @@ class optinfo_item
>    enum optinfo_item_kind m_kind;
>    location_t m_location;
>
> -  /* The textual form of the item.  */
> +  /* The textual form of the item, owned by the item.  */
>    char *m_text;
> -  bool m_owned;
>  };
>
>  #endif /* #ifndef GCC_OPTINFO_H */
> --
> 1.8.5.3
>